Repository navigation
fix: lock and verify Steady mock tooling - #95
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 953f831260
ℹ️ 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.
Pull request overview
This PR hardens and makes reproducible the OpenAPI mock-server tooling used by the CLI integration test workflow by pinning Steady tooling via a dedicated scripts/mock-tools npm lockfile, updating the mock/test scripts to use that locked install, and documenting the new workflow for contributors.
Changes:
- Replace dynamic
npm exec --package=@stdy/cli...execution with a pinned@stdy/cli@0.22.2install vianpm ci --ignore-scriptsunderscripts/mock-tools/. - Add integrity-locked npm manifests for Steady (including platform-native optional packages) and ignore the resulting
node_modules/. - Add contributor documentation covering local setup, mock-server integrity, and common workflows.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/test | Updates test failure guidance to point to ./scripts/mock instead of npm exec. |
| scripts/mock-tools/package.json | Adds a dedicated, private tooling manifest pinning @stdy/cli@0.22.2. |
| scripts/mock-tools/package-lock.json | Adds a lockfile with resolved tarball URLs and SHA-512 integrity for Steady tooling. |
| scripts/mock | Switches mock-server startup to npm ci + locked steady binary and runs npm audit signatures in GitHub Actions. |
| CONTRIBUTING.md | Adds a contributor guide documenting setup, testing, and mock-server integrity expectations. |
| .gitignore | Ignores scripts/mock-tools/node_modules/ produced by the locked install. |
Files not reviewed (1)
- scripts/mock-tools/package-lock.json: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 6 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- scripts/mock-tools/package-lock.json: Generated file
Suppressed comments (1)
CONTRIBUTING.md:6
- The integration tests rely on
@stdy/cli(Steady), which declaresengines.node >=14in the checked-in lockfile. The contributor guide should document this minimum Node.js version so contributors don’t hit runtime failures with older Node installs.
Install [Go 1.25 or later](https://go.dev/doc/install). The integration tests
also require Node.js, npm, `curl`, and `lsof`.
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Validated three supply-chain integrity failures against the exact PR head; the optional-native execution bypass was also reproduced against the base commit. Reproduction details and concrete remediation are in the inline comments.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f41a414d6e
ℹ️ 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.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- scripts/mock-tools/package-lock.json: Generated file
Suppressed comments (3)
scripts/mock-tools/verify-provenance.js:15
lockedPackages()assumes thenode_modules/@stdy/clilockfile entry (and itsoptionalDependencies) always exists. If the lockfile is missing/tampered, this throws a generic TypeError (e.g., readingwrapper.version) rather than a clear fail-closed error explaining what’s wrong with the lock data.
const lockfile = JSON.parse(fs.readFileSync(path.join(toolsDirectory, "package-lock.json"), "utf8"));
const wrapper = lockfile.packages["node_modules/@stdy/cli"];
const expectedVersions = {
"@stdy/cli": wrapper.version,
...wrapper.optionalDependencies,
};
scripts/mock-tools/resolve-native.js:18
resolveNativeBinary()can throw a generic TypeError when the wrapper lock entry is missing/invalid (e.g.,wrapper.optionalDependencies[packageName]) or whenlockedPackage.os/cpuaren’t present. Since this code is meant to hard-fail on unexpected installs, it should validate these lockfile fields explicitly and throw a clear error message instead.
const lockfile = JSON.parse(fs.readFileSync(path.join(trustedDirectory, "package-lock.json"), "utf8"));
const packageName = `@stdy/cli-${platform}-${architecture}`;
const lockfilePath = `node_modules/${packageName}`;
const lockedPackage = lockfile.packages[lockfilePath];
const wrapper = lockfile.packages["node_modules/@stdy/cli"];
if (!lockedPackage || wrapper.optionalDependencies[packageName] !== lockedPackage.version) {
throw new Error(`No locked Steady native package exists for ${platform}-${architecture}`);
}
if (!lockedPackage.os.includes(platform) || !lockedPackage.cpu.includes(architecture)) {
throw new Error(`Locked Steady package ${packageName} does not match the current platform`);
}
scripts/mock-tools/security.test.js:63
withNativeFixture()assumes the currentprocess.platform/process.archcombination is always present in the locked package list. On unsupported platforms (e.g., win32-arm64),packages.find(...)returns undefined and the test crashes with a TypeError instead of a clear, actionable message.
const packageName = `@stdy/cli-${process.platform}-${process.arch}`;
const packageVersion = packages.find(({ name }) => name === packageName).version;
const packageDirectory = path.join(toolsDirectory, "node_modules", packageName);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- scripts/mock-tools/package-lock.json: Generated file
Suppressed comments (3)
CONTRIBUTING.md:124
node scripts/mock-tools/verify-provenance.jsrelies onnpm audit signatures, which is not available in all npm versions. It would help to call this out explicitly so contributors can understand failures like “Unknown command: audit signatures”.
If your configured npm registry supports provenance verification, you can run
the same cross-platform check locally:
scripts/mock-tools/verify-provenance.js:15
lockedPackages()assumes the@stdy/cliwrapper lock entry exists and hasoptionalDependencies. If the lockfile is missing/corrupt, this will currently throw a TypeError (“Cannot read properties of undefined”) rather than a clear, actionable error message.
const wrapper = lockfile.packages["node_modules/@stdy/cli"];
const expectedVersions = {
"@stdy/cli": wrapper.version,
...wrapper.optionalDependencies,
};
CONTRIBUTING.md:6
- The contributor prerequisites say Node.js 14+, but the checked-in mock tooling lockfile uses lockfileVersion 3 (requires newer npm) and the mock scripts/tests use APIs not available in the very earliest Node 14 releases. Please document the minimum Node/npm versions required to run
./scripts/test/./scripts/mocksuccessfully.
Install [Go 1.25 or later](https://go.dev/doc/install). The integration tests
also require Node.js 14.18 or later, npm, `curl`, and `lsof`.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b5733d075
ℹ️ 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.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- scripts/mock-tools/package-lock.json: Generated file
Suppressed comments (2)
scripts/mock:29
scripts/mocknow relies on an npm lockfile withlockfileVersion: 3(npm >= 9) and usesnpm ci --include=.... On older npm versions, this will fail with a somewhat opaque lockfile/flag error; adding an explicit version check here would make failures actionable for contributors.
# Install the reviewed, integrity-locked native package without lifecycle scripts.
npm ci --prefix scripts/mock-tools --include=dev --include=optional \
--ignore-scripts --no-audit --no-fund
STEADY="$(node scripts/mock-tools/resolve-native.js)"
CONTRIBUTING.md:6
- The contributor guide says Node.js 14.18+ is sufficient, but the checked-in
scripts/mock-tools/package-lock.jsonuseslockfileVersion: 3(requires npm >= 9) and./scripts/mockrunsnpm ciagainst it. The documented minimums should be updated to match what the scripts actually require.
Install [Go 1.25 or later](https://go.dev/doc/install). The integration tests
also require Node.js 14.18 or later, npm, `curl`, and `lsof`.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7494ddd42e
ℹ️ 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.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- scripts/mock-tools/package-lock.json: Generated file
Suppressed comments (1)
scripts/mock-tools/verify-provenance.js:93
workflowis optional (it’s read via optional chaining), but the condition then unconditionally accessesworkflow.ref. If the provenance statement lacks the expected workflow metadata, this will throw a TypeError instead of producing the intended verification failure message.
const workflow = statement.predicate?.buildDefinition?.externalParameters?.workflow;
if (workflow?.repository !== STEADY_REPOSITORY || workflow.ref !== `refs/tags/v${expected.version}`) {
throw new Error(`Verified provenance does not match the expected Steady release for ${expected.name}`);
7494ddd to
ef53336
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- scripts/mock-tools/package-lock.json: Generated file
Suppressed comments (1)
scripts/mock-tools/verify-provenance.js:89
verifyAuditReportassumes the expected artifact digest is always instatement.subject[0]. The in-toto Statement schema allows multiple subjects and does not guarantee ordering, so this can fail closed even when a later subject matches the lockfile integrity (e.g., if npm changes subject ordering). Consider accepting any subject entry whose sha512 digest matches the locked integrity.
const expectedDigest = Buffer.from(expected.integrity.slice("sha512-".length), "base64").toString("hex");
if (statement.subject?.[0]?.digest?.sha512 !== expectedDigest) {
throw new Error(`Verified provenance does not match locked SHA-512 integrity for ${expected.name}`);
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- scripts/mock-tools/package-lock.json: Generated file
Suppressed comments (1)
scripts/mock-tools/security.test.js:33
packages.find(...)can return undefined on unsupported platform/arch combinations (e.g., win32-arm64), causing this test script to crash with aTypeErrorrather than failing with a clear assertion. Add an explicit assertion/message before reading.versionso failures are actionable.
const packageName = `@stdy/cli-${process.platform}-${process.arch}`;
const packageVersion = packages.find(({ name }) => name === packageName).version;
const wrapperDirectory = path.join(toolsDirectory, "node_modules", "@stdy", "cli");
e50a24d to
2c1e8bb
Compare
apcha-oai
left a comment
There was a problem hiding this comment.
[sdk codex review] Approved after two independent full reviews of 2c1e8bb05cbdcc1c204d2b232a4f28ab6decdb2c. No new actionable findings across all eight changed files, supporting code, correctness, security boundaries, compatibility, maintainability, tests, CI and all 58 reviews / 26 resolved feedback threads.
Verified the reviewed-lockfile → fresh private npm installation → contained exact-version native-executable boundary, configuration preservation, and removal of shared installation publication. The final implementation does not claim independent runtime publisher-provenance verification.
Immediately before submission: open, non-draft, Justin-authored and MERGEABLE; PR base, actual merge base and live main all 3fe27b9a8f6f6df3d8fec5dee0e28badf5d41fb5. All 13 contexts are terminal: 12 successful and one expected skip. CI checked out 9eaaf80b12fa796e34e2404f840909015146bf55, whose parents are this exact head and current base and whose tree matches HEAD; 24 mock-tooling regressions, mock-backed Go suites and Windows cross-compilation passed.
Source analysis and existing hosted CI only; no PR code or tests executed locally. Native Windows/macOS and the full minimum-version/install-layout matrix were not independently run.
Automated Release PR --- ## [1.9.0](openai/openai-cli@v1.8.0...v1.9.0) (2026-08-26) ### Features * **api:** Add residency flag and deprecate geography ([openai#136](openai#136)) ([e460582](openai@e460582)) ### Bug Fixes * Add provenance-aware untrusted stdin mode ([openai#137](openai#137)) ([a133707](openai@a133707)) * build CLI releases with patched Go toolchains ([openai#93](openai#93)) ([a58f1cc](openai@a58f1cc)) * discard fish completion diagnostics ([openai#109](openai#109)) ([4f35fc0](openai@4f35fc0)) * escape raw output controls on interactive terminals ([openai#112](openai#112)) ([7601afb](openai@7601afb)) * fail closed when bootstrapping Go dependencies ([openai#94](openai#94)) ([4ebe610](openai@4ebe610)) * lock and verify Steady mock tooling ([openai#95](openai#95)) ([681d595](openai@681d595)) * prepare release artifacts before publishing ([openai#111](openai#111)) ([4193619](openai@4193619)) * redact sensitive response headers in debug logs ([openai#106](openai#106)) ([4931572](openai@4931572)) * reject invalid multipart MIME header controls ([openai#105](openai#105)) ([fad98b4](openai@fad98b4)) * restrict downloaded file permissions ([openai#107](openai#107)) ([7a3a7d1](openai@7a3a7d1)) * safely construct bash file completions ([openai#110](openai#110)) ([b3e0946](openai@b3e0946)) * stream binary download responses ([openai#108](openai#108)) ([1f3ba6c](openai@1f3ba6c)) * stream multipart uploads without buffering ([openai#65](openai#65)) ([bcf0ecd](openai@bcf0ecd)) * verify GoReleaser before privileged release execution ([openai#96](openai#96)) ([b3729ea](openai@b3729ea)) ### Chores * **api:** clarify image background and audio delta docs ([openai#124](openai#124)) ([056f306](openai@056f306)) * **api:** No customer-visible SDK or public API changes ([openai#128](openai#128)) ([12e0d28](openai@12e0d28)) * **api:** refresh image model API reference ([openai#115](openai#115)) ([a771913](openai@a771913)) * **api:** Refresh the bundled OpenAPI reference ([openai#66](openai#66)) ([d14387a](openai@d14387a)) * **api:** Report no customer-visible SDK or public API changes ([openai#113](openai#113)) ([8da2d53](openai@8da2d53)) * **api:** update generation metadata only ([openai#138](openai#138)) ([4244a62](openai@4244a62)) * configure Dependabot for Go and GitHub Actions ([openai#69](openai#69)) ([f495366](openai@f495366)) * **deps:** bump actions/attest-build-provenance from 4.1.0 to 4.2.2 ([openai#102](openai#102)) ([6594c73](openai@6594c73)) * **deps:** bump actions/checkout from 6.0.2 to 7.0.1 ([openai#103](openai#103)) ([ce6a6d0](openai@ce6a6d0)) * **deps:** bump actions/checkout from 6.0.2 to 7.0.1 ([openai#134](openai#134)) ([f40d5f2](openai@f40d5f2)) * **deps:** bump actions/download-artifact from 4.3.0 to 8.0.1 ([openai#133](openai#133)) ([98ed029](openai@98ed029)) * **deps:** bump actions/github-script from 7.1.0 to 9.0.0 ([openai#132](openai#132)) ([978ce47](openai@978ce47)) * **deps:** bump actions/setup-go in / ([openai#101](openai#101)) ([5564982](openai@5564982)) * **deps:** bump actions/upload-artifact from 4.6.2 to 7.0.1 ([openai#104](openai#104)) ([b3ac15a](openai@b3ac15a)) * **deps:** bump github.com/charmbracelet/bubbles from 0.21.0 to 1.0.0 ([openai#99](openai#99)) ([7ac7035](openai@7ac7035)) * **deps:** bump github.com/urfave/cli/v3 from 3.4.1 to 3.10.1 in the go-minor-and-patch group across 1 directory ([openai#130](openai#130)) ([ddc612e](openai@ddc612e)) * **deps:** bump the charmbracelet group across 1 directory with 2 updates ([openai#97](openai#97)) ([2137b80](openai@2137b80)) * **deps:** bump the codeql group across 1 directory with 2 updates ([openai#100](openai#100)) ([c95ac0a](openai@c95ac0a)) * **deps:** bump the codeql group across 1 directory with 2 updates ([openai#131](openai#131)) ([58e3bdd](openai@58e3bdd)) * **deps:** bump the go-minor-and-patch group across 1 directory with 5 updates ([openai#98](openai#98)) ([74822be](openai@74822be)) * **deps:** update openai-go to v3.51.0 ([openai#63](openai#63)) ([68e3e70](openai@68e3e70)) * **deps:** update openai-go to v3.52.0 ([openai#67](openai#67)) ([7d87ee2](openai@7d87ee2)) * set a 1,000-line custom-code budget ([openai#126](openai#126)) ([016bfb2](openai@016bfb2)) ### Documentation * add secure CLI agent and contributor guidance ([openai#72](openai#72)) ([09ec087](openai@09ec087)) * standardize CLI vulnerability disclosure policy ([openai#71](openai#71)) ([d082a01](openai@d082a01)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: openai-sdks[bot] <284451331+openai-sdks[bot]@users.noreply.github.com>
Summary
@stdy/cli@0.22.1with reviewed, MIT-licensed Steady0.22.2.npm ci --package-lock=true --include=dev --include=optional --ignore-scriptsin a fresh workspace seeded with the reviewed manifests and trusted repository.npmrc; never vendor native binaries.node_modulesdirectory.Addresses the P1 Steady/npm test-tooling finding in SDK-310.
Trust boundary
The repository-reviewed
package-lock.jsonis the trust root. Every invocation creates an empty installation workspace containing copies of the reviewedpackage.jsonandpackage-lock.jsonplus the repository's trusted.npmrc, when present; prior forged installations cannot survive dry-run mode, an unreviewed siblingnpm-shrinkwrap.jsoncannot override the selected lock, and project-scoped registry/authentication settings remain intact. Explicit--package-lock=trueprevents npm 7 from silently disabling integrity through inherited configuration. npm verifies downloaded archives against the exact reviewed SHA-512 entries, including when an authorized registry mirror supplies the same bytes. Lifecycle scripts are disabled. The fresh installation is validated before it is published: normal Node package resolution must find the exact-version wrapper/native packages inside its npm-managednode_modules, and the executable's real path must remain inside its package. Missing optional natives,NODE_PATH/global fallbacks, ancestor fallbacks, incorrect versions, and external symlinks fail closed.Earlier revisions attempted synthetic provenance verification and inspected npm's private installed lockfile and registry URLs. Those implementation-specific layers could not establish the proposed provenance invariant and rejected supported npm layouts and registry configuration. They have been removed. Publisher provenance remains dependency-update review evidence when available; mock-server startup does not claim independent publisher-provenance verification.
Validation
node scripts/mock-tools/security.test.js— 17 focused cases cover the six exact MIT-licensed/SHA-512-locked packages, all four installation layouts, missing optional natives,NODE_PATH/ancestor fallbacks, incorrect versions, external symlinks, real-npm dry-run reuse, counterfeit same-version shrinkwrap archives, forced npm lockfile enforcement, and repository-only registry settings. All cases pass under npm 7, 10, and 11.package-lock=false, but the exact same archive fails withEINTEGRITYwhen--package-lock=trueis present.NODE_ENV=production,npm_config_omit=optional,npm_config_package_lock=false, omitted private-lock URLs, an unreachable user-level default registry, and a repository-only configured mirror; npm 10 and 11 cover hoisted, nested, shallow, and linked strategies.resolvedvalues. A mirror serving a valid but incorrect native archive leaves the optional package unavailable, and the resolver refuses execution.TEST_API_BASE_URL=http://127.0.0.1:14010 ./scripts/test -count=1 -skip TestFilesCreateCLICancelClosesStalledFIFO— uncached mock-backed Go integration suite and Windows test cross-compilation using an isolated, linked-install Steady 0.22.2 server../scripts/test-goreleaser-installer,go test ./scripts,./scripts/lint, and shell syntax checks pass.TestFilesCreateCLICancelClosesStalledFIFOfails independently on this macOS baseline, so only the local invocation excludes it; Linux CI runs the complete unfiltered suite.