Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add the origin field to the output format of syftjson #1327

Merged
merged 5 commits into from
Jan 12, 2023
Merged

feat: Add the origin field to the output format of syftjson #1327

merged 5 commits into from
Jan 12, 2023

Conversation

TupleType
Copy link
Contributor

This PR adds the origin field to the output format of syftjson when parsing “package-lock.json” or “Pipfile.lock”. The origin field can be useful in tracking the origin of downloaded packages in case of a security incident. The origin field contains the URL used to download an artifact according to the appropriate lock file. For example, package-lock.json has the resolved field to indicate the url used to download the package.

I wasn’t able to find a way to update the golden files of the integration tests similarly to the argument in the unit test’s golden files. Could you please guide me on how to update them?

Changes to the golden files were done using the appropriate parameters of the test files.

Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @asi-cider! It looks like this Origin field is specific to NPM packages and as such should not be added to the main pkg.Package but rather the appropriate metadata type. In this case we probably need a new metadata type, something like PackageLockPackageMetadata, which is set when creating the package, which contains the Origin field. An example metadata type is here.

I think this change would also reduce some of the conflicts, as it stands there are a number of conflicts to resolve if you are able to.

@TupleType
Copy link
Contributor Author

Thanks for the contribution @asi-cider! It looks like this Origin field is specific to NPM packages and as such should not be added to the main pkg.Package but rather the appropriate metadata type. In this case we probably need a new metadata type, something like PackageLockPackageMetadata, which is set when creating the package, which contains the Origin field. An example metadata type is here.

I think this change would also reduce some of the conflicts, as it stands there are a number of conflicts to resolve if you are able to.

Hey @kzantow, thanks for the response. This field exists in some form or another in all kinds of lock files. Currently I've add this for npm and pipfile but I intend to add this for more ecosystems. Do you still think it should be added as metadata field?

@wagoodman
Copy link
Contributor

@asi-cider I agree with @kzantow on this one. Though there are some similarities between different package managers on this field, at this time I think keeping this kind of field in the pkg.Metadata makes the most sense.

@TupleType
Copy link
Contributor Author

@asi-cider I agree with @kzantow on this one. Though there are some similarities between different package managers on this field, at this time I think keeping this kind of field in the pkg.Metadata makes the most sense.

Sure i'll add that in pkg.Metadata

@TupleType
Copy link
Contributor Author

Hey @kzantow and @wagoodman,
I've made the changes I would appreciate your approval.

kzantow
kzantow previously requested changes Dec 6, 2022
syft/pkg/cataloger/javascript/package.go Outdated Show resolved Hide resolved
@TupleType
Copy link
Contributor Author

Hey @kzantow,

Could you guide me on how to fix the unit tests?
Regarding the CLI test, to what version should I bump the schema?

@TupleType TupleType requested a review from kzantow January 2, 2023 09:09
@TupleType
Copy link
Contributor Author

Hey @kzantow @wagoodman, could you please help?

@kzantow
Copy link
Contributor

kzantow commented Jan 12, 2023

@asi-cider I'll have a look today, but there's a conflict that needs to be dealt with -- are you able to merge the latest main changes and take care of this?

@kzantow kzantow closed this Jan 12, 2023
@kzantow kzantow reopened this Jan 12, 2023
@wagoodman
Copy link
Contributor

wagoodman commented Jan 12, 2023

I can help out with the git conflicts and other updates

@wagoodman
Copy link
Contributor

I rebased on main and made the following changes:

  • make the metadata struct names as specific as possible relative to the parsed data sources they represent
  • move the metadata structs to the syft.pkg package
  • regenerated the JSON schema to account for the new fields. (since 6.1.0 hasn't been released yet, it can safely be regenerated)

Copy link
Contributor

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvements @asi-cider !

GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
…1327)

* moved the relevant fields to the Metadata field

Signed-off-by: Asaf Greenholts <[email protected]>

* added metadata types

Signed-off-by: Asaf Greenholts <[email protected]>

* Added hashes to metadata of packge-lock.json and Pipfile.lock

Signed-off-by: Asaf Greenholts <[email protected]>

* move package metadata types to "pkg" package

Signed-off-by: Alex Goodman <[email protected]>

* re-generate json schema to include new npm, python, and binary metadatas

Signed-off-by: Alex Goodman <[email protected]>

Signed-off-by: Asaf Greenholts <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Co-authored-by: Alex Goodman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants