[Cocoa] Added generating SBOM for projects using Cocoapods #1667
Conversation
malice00
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @malice00, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request introduces the capability to generate Software Bill of Materials (SBOMs) for projects utilizing Cocoapods. It includes options for configuring the generation process, such as including or excluding specific targets, merging sub-specs into the main spec, and resolving local Pods to NodeJS artifacts. The changes primarily involve modifications to the .github/workflows/repotests.yml file to set up Cocoapods and checkout test repositories, updates to docs/ENV.md to document the new environment variables for Cocoa configuration, and significant additions to lib/cli/index.js and lib/helpers/utils.js to implement the SBOM generation logic for Cocoa projects.
Highlights
- Cocoapods SBOM Generation: Adds functionality to generate SBOMs for projects using Cocoapods, enhancing dependency management and security analysis.
- Configuration Options: Introduces several options to configure SBOM generation, including target inclusion/exclusion, sub-spec merging, and resolution of local Pods to NodeJS artifacts.
- Workflow Updates: Modifies the CI workflow to include Cocoapods setup and testing, ensuring the new functionality is tested and integrated correctly.
- Documentation: Updates the
ENV.mdfile to document the new environment variables, providing clear instructions for users to configure the Cocoa SBOM generation.
Changelog
Click here to see the changelog
- .github/workflows/repotests.yml
- Adds a step to install cocoapods via
gem install cocoapods -v 1.16.2 --no-document - Adds a checkout step for the
malice00/cdxgen-cocoapods-testrepository torepotests/cocoapods-test - Introduces new test jobs for expo projects on macOS and non-macOS environments, utilizing cocoapods and gradle to generate SBOMs with various configurations.
- Adds a test job specifically for cocoapods projects, generating SBOMs with different options like
COCOA_FULL_SCAN,COCOA_MERGE_SUBSPECS, andCOCOA_INCLUDED_TARGETS.
- Adds a step to install cocoapods via
- docs/ENV.md
- Adds a new section for Cocoa environment variables, documenting options such as
COCOA_EXCLUDED_TARGETS,COCOA_FULL_SCAN,COCOA_INCLUDED_TARGETS,COCOA_MERGE_SUBSPECS,COCOA_PODSPEC_JSON_REPLACEMENTS,COCOA_PODSPEC_REPLACEMENTS,COCOA_RESOLVE_FROM_NODE, andCOCOA_RESOLVE_FROM_NODE_EXCLUSION_DIRS.
- Adds a new section for Cocoa environment variables, documenting options such as
- lib/cli/index.js
- Imports
existsSyncfromnode:fs - Imports
loadYamlfromjs-yaml - Adds
buildObjectForCocoaPodto the list of imported functions. - Adds
executePodCommandto the list of imported functions. - Adds
parseCocoaDependencyto the list of imported functions. - Adds
parsePodfileLockandparsePodfileTargetsto the list of imported functions. - Implements
createCocoaBomfunction to generate SBOMs for Cocoa projects, including logic to handle Podfile.lock parsing, dependency resolution, and target inclusion/exclusion. - Modifies
createMultiXBomto include Cocoa projects when generating SBOMs for multiple project types. - Modifies
createXBomto include Cocoa projects when generating SBOMs. - Modifies
createBomto include Cocoa projects when generating SBOMs based on project type aliases.
- Imports
- lib/helpers/utils.js
- Imports
randomUUIDfromnode:crypto - Adds a
temporaryFilesset to track temporary files created by cdxgen, ensuring they are deleted on exit. - Adds
cocoato thePROJECT_TYPE_ALIASES. - Implements
parsePodfileLockto parse the contents of a Podfile.lock, extracting dependencies and their metadata. - Implements
parsePodfileTargetsto parse targets and their direct dependencies from the Podfile. - Implements
parseCocoaDependencyto parse a single line representing a dependency. - Implements
executePodCommandto execute the 'pod' command with specified parameters. - Implements
buildObjectForCocoaPodto create SBOM objects for Cocoa pods, including logic to resolve dependencies from NodeJS packages and perform full scans. - Implements
fullScanCocoaPodto perform a full scan of Cocoa pods, extracting additional metadata and external references.
- Imports
- lib/helpers/utils.test.js
- Imports
loadYamlfromjs-yaml - Adds
buildObjectForCocoaPodto the list of imported functions. - Adds
parseCocoaDependencyto the list of imported functions. - Adds
parsePodfileLockandparsePodfileTargetsto the list of imported functions. - Adds tests for
parsePodfileLock,parsePodfileTargets,parseCocoaDependency, andbuildObjectForCocoaPod.
- Imports
- test/Podfile
- Creates a Podfile for testing purposes, including dependencies and target definitions.
- test/Podfile.json
- Creates a JSON representation of the Podfile for testing purposes.
- test/Podfile.lock
- Creates a Podfile.lock for testing purposes.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Did you know?
CocoaPods is written in Ruby and is distributed as a gem. It is one of the most popular dependency managers for Swift and Objective-C projects.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces the capability to generate SBOMs for projects utilizing Cocoapods, enhancing the tool's ability to analyze and document dependencies in iOS and macOS projects. The addition of configuration options for target inclusion/exclusion, sub-spec merging, and local Pod resolution significantly increases the flexibility and usefulness of the SBOM generation process. The documentation updates are also a welcome addition.
Merge Readiness
The pull request appears to be well-structured and introduces a valuable feature. However, it's important to ensure that the changes have been thoroughly tested across different Cocoapods project configurations and that the documentation is clear and comprehensive for users to effectively utilize the new features. I am unable to directly approve this pull request, and I recommend that others review and approve this code before merging. Given the complexity of the changes, I recommend a thorough review by someone familiar with Cocoapods and SBOM generation before merging.
|
@prabhu Something connected to the automatic checking of included builds in gradle broke the expo builds... Checking it now... Edit: which unfortunately breaks your logic. 😢 |
|
OK, so I changed something in the test to get it running again -- but I do feel changes to the gradle included-build are still needed. |
|
@malice00 can we detect the problematic Not sure how to get the value |
docs/ENV.md
Outdated
| | COCOA_FULL_SCAN | Whether or not to do a full (deep) scan of the pods. This requires CocoaPods to be installed and runnable from the `PATH`. When set to `false` or `0`, only the most basic of information will be gathered (name, version, purl and, if applicable, sub-spec) -- can be useful if run on Windows. Defaults to `true`. | | ||
| | COCOA_INCLUDED_TARGETS | Comma-separated list of target to include for resolution of dependencies. Usage is the same as `COCOA_EXCLUDED_TARGETS` above. | | ||
| | COCOA_MERGE_SUBSPECS | Should all sub-specs (and their dependencies) be merged into the root spec. This can be useful if you are importing your SBOM into a tool that doesn't handle sub-specs as separate dependencies (eg dependency-track). Defaults to `false`. | | ||
| | COCOA_PODSPEC_JSON_REPLACEMENTS | Comma-separated list of 'text_to_find=replacement'. This can be used if you have local pods in JSON format that are not (correctly) parsable by the `pod ipc sepc`-command, eg because some variables or functions would be inserted by the `Podfile` -- users of react-native / expo might need this. This will be interpreted as a `regex` if the 'text_to_find' is both prefixed and suffixed with a `/` and then references like `$1` can be used in the 'replacement'. To find or insert a newline, use the String `<NEWLINE>`. | |
There was a problem hiding this comment.
This is introducing some code injection risks. Can you share any documentation about this command?
There was a problem hiding this comment.
Shall I add a completely new page, or add it here? I feel this description is already pretty much at the limit of what is practical for a quick overview of the EnvVars...
There was a problem hiding this comment.
Maybe a console.log? If not don't worry, we can work on a new PR.
There was a problem hiding this comment.
OK, let's do this in a new PR then.
I think we can, but it would need some reworking of the current code. I would however prefer to not add it to this issue, since that is gradle specific. Unfortunately, the value |
| | Variable | Description | | ||
| | --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | ||
| | COCOA_EXCLUDED_TARGETS | Comma-separated list of targets to exclude from resolution of dependencies. If a target has nested targets, these are also excluded. For excluding only those nested targets, use a `/` to describe the target, eg `target/subtarget/subsubtarget` -- this excludes only `subsubtarget` from the resolution. | | ||
| | COCOA_FULL_SCAN | Whether or not to do a full (deep) scan of the pods. This requires CocoaPods to be installed and runnable from the `PATH`. When set to `false` or `0`, only the most basic of information will be gathered (name, version, purl and, if applicable, sub-spec) -- can be useful if run on Windows. Defaults to `true`. | |
There was a problem hiding this comment.
We have options.deep. Can we use that instead of this env variable? options. deep defaults to false though.
There was a problem hiding this comment.
Depends on what options.deep does/means, I guess. I'll check it out.
There was a problem hiding this comment.
Looks like options.deep also gets set depending on profiles. We could use it, but I think we should have the ability to specifically (de)activate the full scanning. The reason being that most users will probably run cocoapods only on a Mac and a scan on any other platform might then cause problems. Then again, they might also only generate SBOMs on a Mac, where it would be a non-issue...
There was a problem hiding this comment.
Can cocoapods run with our container images once we add the missing gem install? We can always do options.deep || isMac to decide whether to do a full scan?
There was a problem hiding this comment.
It probably can, although I can't test this right now -- I currently have no docker/podman running on this system. It works in the repotests however, so if they are similar it should not be a problem. We could even add it on the linux images, if ruby is installed. Windows might be a bit more complicated, I was unable to get cocoapods running correctly, but according to my research, it should be doable.
|
@gemini-code-assist, a feedback for you is to keep your comments shorter and to the point. We don't need those "Did you know?" things, for example. Two things you could have highlighted better in your review:
Summary for the changelog was quite useful, so keep it up! |
|
Thanks for the feedback, @prabhu! I appreciate the specific points on keeping comments concise and suggesting a table for environment variables. I'll definitely keep that in mind for future reviews. I'm always striving to improve the quality and usefulness of my feedback. |
| @@ -448,6 +458,7 @@ export const PROJECT_TYPE_ALIASES = { | |||
| ], | |||
| binary: ["binary", "blint"], | |||
| oci: ["docker", "oci", "container", "podman"], | |||
| cocoa: ["cocoa", "cocoapods", "objective-c", "swift", "ios"], | |||
There was a problem hiding this comment.
We already have a swift type with an alias for ios. Let's remove swift and ios from here.
There was a problem hiding this comment.
True, but will it run the new scanner when I use -t ios?
Or did I misunderstand the meaning of the aliases?
There was a problem hiding this comment.
We can prefer swift for -t ios, since most new projects are swift based.
There was a problem hiding this comment.
That might be true, but swift-projects can actually use cocoapods, so imo both scanners should run.
There was a problem hiding this comment.
Interesting. Does your PR run both the scanners for -t ios?
There was a problem hiding this comment.
But I just noticed it is a bit of a problem to have duplicate entries, because using -t ios doesn't generate my SBOM...
There was a problem hiding this comment.
Interesting. Does your PR run both the scanners for
-t ios?
Missed your reply when I posted the above -- no, it doesn't, I guess I only tested using -t cocoapods.
|
Please rebase before working on the changes. Apologies. |
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Split them in mac/others because mac allows more tests in this project Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
|
Happy to merge this PR and help add things like dockerfile enhancements etc in a new PR? |
|
Let me just finish the last couple of loose ends and then we can work on fine-tuning in a new PR. |
Signed-off-by: Roland Asmann <[email protected]>
Signed-off-by: Roland Asmann <[email protected]>
This PR adds the possibility to generate an SBOM for projects that use Cocoapods.
Several options are available to configure the generation (eg in-/exclude targets, merge sub-specs into main spec and resolution of local Pods to NodeJS artefacts). Check the documentation on how to use these.
@prabhu: I haven't checked the new thought log, so that is not implemented -- any suggestions on that are welcome.