chore(openshell): trust v0.0.101 release identities - #8616
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe installer trust checks now use allowlisted Homebrew formulas and trusted sandbox-build identities. Tests cover OpenShell 0.0.101, malformed trust data, mismatched digests, duplicate entries, alternate releases, and parser mutations. ChangesOpenShell release trust
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Installer as Installer
participant Checker as check-installer-hash.sh
participant Formula as Homebrew formula
participant Parser as extract-installer-pins.mts
Installer->>Checker: Submit release and installer pins
Checker->>Formula: Download allowlisted formula
Formula-->>Checker: Return formula content
Checker->>Checker: Verify upstream digest and formula pin
Checker->>Parser: Validate extracted sandbox-build pins
Parser-->>Checker: Return trusted identity result
Checker-->>Installer: Accept or reject release identities
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 414de88 in the TypeScript / code-coverage/cliThe overall coverage in commit 414de88 in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/installer-hash-check.test.ts (1)
639-691: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that anchored mutations actually change the source.
CHECKER_MUTATIONSandPARSER_MUTATIONSuseString.prototype.replacewith long literal anchors, for example"readonly -a OPENSHELL_RELEASE_FORMULA_ALLOWLIST=(\n"at line 651 and"const TRUSTED_SANDBOX_BUILD_PINS: readonly TrustedSandboxBuildPin[] = [\n"at line 682. If a later refactor changes those anchors,replacereturns the source unchanged and the mutation becomes a silent no-op.A silent no-op does not make the negative tests error. It makes them pass for the wrong reason. For example,
trusted-sandbox-alternate-versionat line 1146 expects failure. IftrustAlternateSandboxBuildsno-ops, the run still fails, but because the sandbox identities were never trusted, not because the release lacks an allowlist entry.
test/installer-sandbox-build-trust.test.tsalready guards this at lines 49-53. Apply the same guard here.♻️ Proposed guard for anchored replacements
+const replaceAnchored = (source: string, anchor: string, replacement: string): string => { + const mutated = source.replace(anchor, replacement); + if (mutated === source) { + throw new Error(`fixture mutation anchor no longer matches: ${anchor}`); + } + return mutated; +};Then route the anchored mutations through it, for example:
-const trustAlternateSandboxBuilds = (source: string): string => { - const digests = SYNTHETIC_SANDBOX_BUILD_DIGESTS; - return source.replace( - "const TRUSTED_SANDBOX_BUILD_PINS: readonly TrustedSandboxBuildPin[] = [\n", - `const TRUSTED_SANDBOX_BUILD_PINS: readonly TrustedSandboxBuildPin[] = [ - { required: false, sha256: "${digests[0]}", version: "9.9.9" }, - { required: false, sha256: "${digests[1]}", version: "9.9.9" }, -`, - ); -}; +const trustAlternateSandboxBuilds = (source: string): string => { + const digests = SYNTHETIC_SANDBOX_BUILD_DIGESTS; + return replaceAnchored( + source, + "const TRUSTED_SANDBOX_BUILD_PINS: readonly TrustedSandboxBuildPin[] = [\n", + `const TRUSTED_SANDBOX_BUILD_PINS: readonly TrustedSandboxBuildPin[] = [ + { required: false, sha256: "${digests[0]}", version: "9.9.9" }, + { required: false, sha256: "${digests[1]}", version: "9.9.9" }, +`, + ); +};Apply the same treatment to the
allowlisted-alternate-version,duplicate-trusted-formula,malformed-trusted-formula,mismatched-trusted-formula-url, andtrusted-formula-mismatchentries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/installer-hash-check.test.ts` around lines 639 - 691, Add a shared checked-replacement helper in the installer hash-check tests that asserts each anchor is found and the resulting source changes, then use it for the anchored replacements in CHECKER_MUTATIONS and trustAlternateSandboxBuilds, including the allowlisted, duplicate, malformed, URL-mismatch, and digest-mismatch formula mutations. Preserve each mutation’s existing replacement behavior while preventing silent no-ops.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/installer-hash-check.test.ts`:
- Around line 639-691: Add a shared checked-replacement helper in the installer
hash-check tests that asserts each anchor is found and the resulting source
changes, then use it for the anchored replacements in CHECKER_MUTATIONS and
trustAlternateSandboxBuilds, including the allowlisted, duplicate, malformed,
URL-mismatch, and digest-mismatch formula mutations. Preserve each mutation’s
existing replacement behavior while preventing silent no-ops.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 856b1b40-dbb4-438c-9b26-ee3384dc64dc
📒 Files selected for processing (4)
scripts/check-installer-hash.shscripts/checks/extract-installer-pins.mtstest/installer-hash-check.test.tstest/installer-sandbox-build-trust.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
2 participants
Summary
Establish base-trusted identities for the published OpenShell v0.0.101 release without changing NemoClaw's active v0.0.99 selection. The checker now binds checksum manifests, the Homebrew formula, and standalone sandbox fallback binaries to reviewed release identities so a later selector PR cannot authorize its own artifacts.
Related Issue
Fixes #8598
Changes
Type of Change
Quality Gates
6ace3b8d657d3d65589d9ce61fe4672400f30955returned PASS with no blocker. It independently recomputed the v0.0.101 artifacts and confirmed the base-owned formula tuple and standalone sandbox version/digest sets close both candidate self-authorization paths.Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablescripts/check-installer-hash.shpassed on the current v0.0.99 selection.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to dormant trust data and its verifier boundary.npm run validate:pr,npm run checks:repository,npm run typecheck:cli,npm run test:changed, test-size/source-shape checks, shfmt, and ShellCheck passed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Security
Release Support
Testing