Add Swift Package Manager parser for Package.swift/resolved#139
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 64ac7f1 | Docs | Datadog PR Page | Give us feedback! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4b98f13d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be6d78e1ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5ab3c8f59
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
anderruiz
force-pushed
the
ander/shift-package-manager
branch
from
a5ab3c8 to
8ef14af
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1601d3127e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if !inBlock { | ||
| // Look for .package( to start a dependency block | ||
| idx := strings.Index(line, ".package(") |
There was a problem hiding this comment.
When Package.swift contains a commented-out .package(url: ...) for a package that is still present in Package.resolved (for example because it is transitive, or the lockfile is stale), this unconditional substring search records the comment as a real manifest entry. Match then marks that package IsDirect=true and reports the comment as the manifest location, which can skew direct-dependency metadata and reachability inputs; skip Swift line/block comments before looking for dependency declarations.
Useful? React with 👍 / 👎.
|
|
||
| if !inBlock { | ||
| // Look for .package( to start a dependency block | ||
| idx := strings.Index(line, ".package(") |
There was a problem hiding this comment.
When dependencies are formatted compactly on one line, such as dependencies: [.package(...), .package(...)], this only finds the first .package( and then treats the rest of the line as one block; extractEntryFromBlock extracts only the first url: from that block. Any later direct dependencies on the same line are left out of entryMap, so they remain marked as transitive and lose their manifest location.
Useful? React with 👍 / 👎.
anderruiz
changed the title
piloulacdog
left a comment
piloulacdog
left a comment
There was a problem hiding this comment.
something we might be missing is to know: "how good is our parser compared to competitor"? have we done some analysis about this?
We have the tools to do so. I didn't spend much time on trying to find the right repository to test, but randomly picking for example: https://github.com/muh-nee/sca-testing-Signal-iOS (which has no Packages.resolved - only has Packgaes.swift) and https://github.com/muh-nee/sca-testing-firefox-ios (has both) we can already tests them and see some results here and there and it could guide us on identifying potential gaps / or prove that we are better at detecting libraries.
| models.EcosystemPub: "pub", | ||
| models.EcosystemHex: packageurl.TypeHex, | ||
| models.EcosystemCRAN: packageurl.TypeCran, | ||
| models.EcosystemSwiftURL: "swift", |
There was a problem hiding this comment.
should we do like the other ecosystem (except pub) and have an actual: packageurl enum?
| tests := []struct { | ||
| name string | ||
| packageName string | ||
| wantNamespace string |
There was a problem hiding this comment.
nitpick: but in tests usually this kind of variables are using the keyword "expected" over "want" (which I almost never seen - doesn't mean it doesn't exist - just that's pretty uncommon to me)
| // Build a lookup map from normalized name to entry | ||
| entryMap := make(map[string]packageEntry, len(entries)) | ||
| for _, entry := range entries { | ||
| entryMap[entry.name] = entry |
There was a problem hiding this comment.
I think that because GitHub URLs are case-insensitive, so a Package.resolved with https://github.com/alamofire/alamofire.git paired with a Package.swift referencing https://github.com/Alamofire/Alamofire.git (or vice-versa) won't match, and the package stays IsDirect=false
| entryMap[entry.name] = entry | |
| entryMap[strings.ToLower(entry.name)] = entry |
There was a problem hiding this comment.
we would need to validate it with a test
There was a problem hiding this comment.
But do you think this is a real case given resolved is autogenerated?
There was a problem hiding this comment.
checked locally and:
when in Package.swift
.package(url: "https://github.com/Alamofire/Alamofire.git", from: "5.9.0")
...
.product(name: "Alamofire", package: "Alamofire")then Package.resolved
{
"identity" : "alamofire",
"kind" : "remoteSourceControl",
"location" : "https://github.com/Alamofire/Alamofire.git",
"state" : {
"revision" : "e938f8c66708e7352fc7e3512647fa54255b267a",
"version" : "5.11.2"
}
}and when:
.package(url: "https://github.com/alamofire/alamofire.git", from: "5.9.0"),
...
.product(name: "Alamofire", package: "alamofire"), # name should match the manifest of the library - cannot be lower case then Package.resolved
{
"identity" : "alamofire",
"kind" : "remoteSourceControl",
"location" : "https://github.com/Alamofire/Alamofire.git",
"state" : {
"revision" : "e938f8c66708e7352fc7e3512647fa54255b267a",
"version" : "5.11.2"
}
},|
|
||
| // Enrich matching packages | ||
| for i := range packages { | ||
| entry, ok := entryMap[packages[i].Name] |
There was a problem hiding this comment.
same as above
| entry, ok := entryMap[packages[i].Name] | |
| entry, ok := entryMap[strings.ToLower(packages[i].Name)] |
| // direct-dependency information from Package.swift. | ||
| type PackageSwiftMatcher struct{} | ||
|
|
||
| var urlRegexp = cachedregexp.MustCompile(`url:\s*"([^"]+)"`) |
There was a problem hiding this comment.
From reading a bit the specs of swift, it seems that the Swift Package Registry .package(id: ...) is never detected as direct
This regex only matches the URL-based declaration form. When Swift 5.7+ registry deps use .package(id: "scope.name", from: ...) with no url: field, so a registry-kind pin paired with a matching .package(id:) in Package.swift stays IsDirect=false.
A complete fix probably needs two changes:
- Here, also extract
id:\s*"([^"]+)"so registry-style declarations are picked up. - In
parse-package-resolved.goaround line 90, key registry pins (pin.Kind == "registry") bypin.Identityinstead ofnameFromRepoURL(pin.Location), so the matcher key actually lines up with theid:value.
If we want to defer registry support to a follow-up, that's fine: a code comment / TODO here calling out the limitation would be enough so it isn't silently incorrect + explaining the limitation in the README.md (just like we do for other languages where we explain some limitations)
@piloulacdog I know for sure that Package.resolved is missing in several repos, because I started with that approach for the crawlers and it was not working, but I am assuming that as a reasonable limitation that we can address in a similar way to what we will do in NPM to allow lockless reporting. I would however start with the normal approach of assuming both needs to be present |
Pass LocationRoleManifest to NewPackageLocations in the scanner's vulnerability result builder. Update test fixture literals and snapshots to include the new Role field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds SwiftPM PackageManager constant, Swift language, SwiftFilePath
("Package.resolved"), and a pkg:swift purl extractor that derives
namespace/name from the repository URL (strips .git suffix, lowercases
the last two path segments).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ackage.swift matcher Extractor parses all three Package.resolved formats: - v1 (Xcode 11): object.pins[] with repositoryURL - v2 (Xcode 13): pins[] with location - v3 (Xcode 15.3): same as v2 plus originHash localSourceControl pins are skipped (no meaningful purl URL). All packages get LocationRole=lockfile and IsDirect=false by default. The Package.swift Matcher enriches packages found in the sibling Package.swift manifest: sets IsDirect=true, LocationRole=manifest, and BlockLocation pointing to the .package(url:...) declaration line that the developer actually edits to update a dependency. Registers via init() into pkg/lockfile/parsers/parsers.go. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The BlockLocation was using Column{0,0} which failed
IsFilePositionExtractedSuccessfully and caused locations to be silently
dropped. Use GetFirstNonEmptyCharacterIndexInLine / GetLastNonEmpty to
extract real column bounds from the .package( source line, matching the
pattern used by all other matchers (e.g. Cargo.toml).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Registry pins (kind: "registry") have an empty location and store
the package identifier in the identity field. Previously they were
silently dropped because nameFromRepoURL("") returns "".
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
git@github.com:org/repo.git style URLs were silently dropped because url.Parse does not recognise them as having a host. A regex handles this format before falling back to url.Parse. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A line-comment stripper removes // ... (excluding :// in URLs) before searching for .package(, so transitive packages that coincidentally match a commented-out declaration are not incorrectly marked IsDirect. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…wift Previously only the first .package( per line was detected. The parser now tracks the closing paren position and continues scanning the remainder of the line for additional declarations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GitHub URLs are case-insensitive. If Package.swift and Package.resolved use different casings for the same repository (e.g. after a manual URL edit without re-resolving), the lookup now still matches correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When a dependency is pinned to a branch instead of a version tag, State.Version is empty and State.Branch holds the branch name. Previously these packages were emitted with an empty version; now the branch name is used, matching Trivy's behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
anderruiz
force-pushed
the
ander/shift-package-manager
branch
from
1601d31 to
1a39688
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Use integer range (intrange) in stripLineComment loop - Add blank lines before break statements (nlreturn) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65f75ca5df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if len(nameParts) < 2 || len(packageInfo.Name) == 0 { | ||
| err = fmt.Errorf("invalid swift package_name (%s)", packageInfo.Name) |
There was a problem hiding this comment.
When a registry pin is extracted, the parser now emits the dotted pin.identity as the package name (for example apple.swift-argument-parser), but sanitizeScannedPackages still sends that name through this splitter, which requires a slash and returns an error. That causes every registry-resolved Swift dependency to be dropped before SBOM/vulnerability output; the fresh evidence beyond the earlier registry-parser comment is that the identity is now preserved by extraction but rejected during PURL conversion.
Useful? React with 👍 / 👎.
| if resolved.Version == 1 { | ||
| if isLocalURL(pin.repoURL) { | ||
| continue |
There was a problem hiding this comment.
For v1 Package.resolved files that store a private repository as repositoryURL: "git@github.com:org/repo.git", this v1-only guard calls isLocalURL; net/url.Parse errors on scp-style URLs and isLocalURL treats that as local, so the pin is skipped before the scp handler in nameFromRepoURL can run. The fresh evidence beyond the earlier SSH comment is that the v2 path now handles scp URLs, but this v1 guard still drops the same URL form.
Useful? React with 👍 / 👎.
Adds an idRegexp to extract the id: "scope.name" argument from .package(id: ...) blocks. Registry pins in Package.resolved already use pin.Identity as the package name, so the two values align and IsDirect is correctly set to true. URL resolution from scope.name to a repository URL is left to the backend. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64ac7f1e66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Use branch as version when there is no version tag (branch-pinned dependency). | ||
| version := pin.version | ||
| if version == "" { | ||
| version = pin.branch | ||
| } |
There was a problem hiding this comment.
When a Swift dependency is pinned by branch or exact revision, state.version is empty and the lockfile's only exact resolved identifier is state.revision; this fallback stores the branch name (or leaves Version empty for .revision(...)) even though Commit is populated. sanitizeScannedPackages then creates a Swift PURL from only name/version, and groupBySource takes the ecosystem/name path before the commit-only fallback, so vulnerability output for those pins carries pkg:swift/...@main or no version instead of the resolved commit. Please use the revision as the version for unversioned pins, or propagate Commit into PackageInfo for Swift packages.
Useful? React with 👍 / 👎.
2 participants
Summary
Adds a Swift Package Manager parser that handles all three
Package.resolvedlockfile formats (v1, v2, v3) and aPackage.swiftmatcher for direct-dependency enrichment.Package.resolvedformats:object.pins[]withrepositoryURLpins[]withlocationoriginHashlocalSourceControlpins (no meaningful purl URL)IsDirect=false,LocationRole=lockfilePackage.swiftmatcher enriches direct dependencies: setsIsDirect=true,LocationRole=manifest, andBlockLocationpointing to the.package(url:...)declaration line.package(...)declarationspkg:swift/github.com/{owner}/{repo}@{version}New files
pkg/lockfile/swift/types.gopkg/lockfile/swift/parse-package-resolved.goinit()registrationpkg/lockfile/swift/match-package-swift.goPackage.swiftinternal/utility/purl/swift.goTest plan
pkg/lockfile/parsers/parsers.go🤖 Generated with Claude Code