fix(onboard): rebuild on messaging credential drift - #7853
Conversation
Signed-off-by: Ho Lim <subhoya@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSandbox resume compares stored messaging credential hashes with environment credentials for active channels. Detected changes prevent reuse, validate replacement credentials, recreate the sandbox when required, and leave disabled channels unchanged. ChangesMessaging credential drift
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Onboard
participant SandboxResume
participant CredentialDrift
participant SandboxRegistry
participant SandboxRuntime
Onboard->>SandboxResume: resume existing sandbox
SandboxResume->>SandboxRegistry: load registry messaging plan
SandboxResume->>CredentialDrift: compare active-channel hashes with environment
CredentialDrift-->>SandboxResume: messaging credential drift
SandboxResume->>SandboxRuntime: validate credentials and recreate sandbox
SandboxRuntime->>SandboxRegistry: store updated messaging plan
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/onboard/machine/handlers/sandbox-credential-drift.test.ts (2)
18-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the hoisted module mock.
This import-time
vi.mocksetup has no explanation. Add a short comment stating that messaging discovery is mocked to isolate credential-drift resume behavior. As per coding guidelines, import-time stubs must be documented.🤖 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 `@src/lib/onboard/machine/handlers/sandbox-credential-drift.test.ts` around lines 18 - 20, Add a short explanatory comment immediately above the import-time vi.mock for detectMessagingChannelsFromEnv, stating that messaging discovery is mocked to isolate credential-drift resume behavior.Source: Coding guidelines
64-72: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the durable registry result after recreation.
These assertions verify the session hash and mocked remove/create calls, but not the PR’s durable contract: one registry entry for
savedcontaining only the replacement hash and no raw credential. Add a public-boundary assertion against the resulting registry state.
As per path instructions, tests should prove observable behavior through the public boundary.🤖 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 `@src/lib/onboard/machine/handlers/sandbox-credential-drift.test.ts` around lines 64 - 72, The test currently verifies session state and mocked sandbox calls but not the durable registry contract. Extend the test around the recreation flow to inspect the resulting registry through its public boundary, asserting exactly one entry for “saved” containing only the replacement credential hash and no raw credential; keep the existing session and call assertions unchanged.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 `@src/lib/onboard/machine/handlers/sandbox-credential-drift.test.ts`:
- Around line 18-20: Add a short explanatory comment immediately above the
import-time vi.mock for detectMessagingChannelsFromEnv, stating that messaging
discovery is mocked to isolate credential-drift resume behavior.
- Around line 64-72: The test currently verifies session state and mocked
sandbox calls but not the durable registry contract. Extend the test around the
recreation flow to inspect the resulting registry through its public boundary,
asserting exactly one entry for “saved” containing only the replacement
credential hash and no raw credential; keep the existing session and call
assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 60303fae-3b03-4bc2-beb8-8a5ceb5a50cd
📒 Files selected for processing (6)
src/lib/onboard/machine/handlers/sandbox-credential-drift.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.tssrc/lib/onboard/machine/handlers/sandbox-resume.test.tssrc/lib/onboard/machine/handlers/sandbox-resume.tssrc/lib/onboard/machine/handlers/sandbox.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Maintainer security review — exact head f17c8894aff2552c08add35d4d1e674b7ca395aa against base 795de982e26b400009bf1fc0f3826845d79acc4d: PASS. No security findings. Diff fingerprint: 12d36c86f39aef3a93fe3f5a2771338c2dac662ce3c166e698e78f69a7184900.
- Secrets and credentials — PASS. The change compares only SHA-256 credential hashes. Raw token values are neither persisted nor logged, and disabled-channel bindings are excluded from drift-triggered recreation.
- Input validation and sanitization — PASS. Drift is bounded to canonical active channel IDs from the durable messaging plan; missing credentials remain non-events, and explicitly changed active credentials enter the existing validation flow.
- Authentication and authorization — PASS. Replacement credentials are validated through the existing channel setup before any sandbox deletion or recreation. No role, permission, or pairing boundary changes.
- Dependencies and third-party code — PASS. No dependency, lockfile, registry, image-source, or license changes.
- Error handling and logging — PASS. Validation failures occur before the destructive recreation edge; existing rollback behavior remains intact. Messages disclose only that drift occurred, never credential content.
- Cryptography and data protection — PASS. The existing credential-hash primitive is reused without algorithm or storage changes.
- Configuration and infrastructure — PASS. No new command, option, environment variable, network policy, capability, or filesystem-permission surface. Paused channels retain their current sandbox without unrelated churn.
- Security testing — PASS. Focused tests cover changed active credentials, unchanged/missing credentials, disabled-channel replacement credentials, and resume decision behavior. Three focused files passed 38/38; CLI typecheck, Biome, diff checks, secret scan, and
npm run check:diffpassed. - Holistic security posture — PASS. The patch strengthens the fail-closed credential-rotation path by coupling the active-channel drift decision to validation before recreation while preventing unneeded destructive work for inactive channels.
Documentation writer result: docs-updated; npm run docs passed with 0 errors and 2 pre-existing warnings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@docs/reference/troubleshooting.mdx`:
- Around line 726-728: Rewrite the new troubleshooting text around NemoClaw so
it addresses the reader directly in active voice and present tense, replacing
third-person references with second-person wording while preserving the existing
credential validation, backup, recreation, restore, and paused-channel behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e0864382-415a-4fc2-a2ab-c57490c8738b
📒 Files selected for processing (5)
docs/reference/troubleshooting.mdxsrc/lib/onboard/machine/handlers/sandbox-credential-drift.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.tssrc/lib/onboard/machine/handlers/sandbox.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/onboard/machine/handlers/sandbox-messaging.test.ts
Signed-off-by: Ho Lim <subhoya@gmail.com>
wscurran
added
area: messaging
|
✨ Thanks for the PR. This fixes credential drift detection by comparing explicitly supplied messaging credential hashes against the durable sandbox plan and entering the reconciliation path when they differ. Maintainers will review the hash comparison logic, registry cleanup, and test coverage for missing, unchanged, and replacement credentials. Related open issues: |
|
Current-head CI note for |
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Approved exact head 8d144240adabbbe36901a32dd633af8312203a27 against base 795de982e26b400009bf1fc0f3826845d79acc4d (diff fingerprint faba59c22c4ae37d4b3f2500598c449c83a2b3cd4c876db845d3977aad811520). The credential-drift rebuild path was reviewed end to end: reconciliation validates messaging inputs before destructive recreation, completed checkpoints still detect drift before recreation, and rejected placeholder handling preserves the existing hash contract. Final security verdict is PASS; the last delta after that receipt is documentation only. All four commits are GitHub-verified, DCO/docs receipt/primary advisor/current threads are clean, normal CI passed after one unrelated temporary-lock retry, and protected E2E run 30503257538 passed cloud-onboard, onboard-repair, and onboard-resume with controller verification 30503239150.
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Approved exact head 0e10adf14b9238a7f85592a40a4339b0ab6dd8f5 against current base d52d4599a18490e7f8efc6e8062296fffcbea4a7 (diff fingerprint faba59c22c4ae37d4b3f2500598c449c83a2b3cd4c876db845d3977aad811520). The only delta after the previously reviewed 8d144240a head is the current-main merge; the credential-drift behavior and prior PASS security verdict are unchanged. The current-main merge gate reports all 41 checks green, clean/mergeable state, no unresolved major CodeRabbit findings, risky-path test coverage, DCO compliance, and all five commits GitHub-verified. Protected E2E child run 30505198308 passed cloud-onboard, onboard-repair, and onboard-resume; controller run 30505176269 and the native exact-head gate both succeeded. Contributor/approver overlap for @cjagwani is surfaced as the repository's non-blocking advisory.
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Exact-head approval after refresh onto current main. Deterministic maintainer gate passes with all 41 current checks green, clean merge state, verified history, and no unresolved major findings. Protected cloud-onboard, onboard-repair, and onboard-resume all passed on this head.
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Addressed the remaining documentation-review gap in commit
Validation passed: the documentation build completed with 0 errors (2 existing warnings), the focused credential-rotation documentation contract passed 6/6, and the diff check passed. The earlier rotation lifecycle and terminology requests remain resolved. Fresh repository CI and protected E2E are still pending; no check is being waived. |
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Security review — exact head 0e6176d28cbe41397a341a5114147f2851e18bdd against base c176af5f08595ed748b4eeac8988c554e180183e: PASS with no findings.
- Secrets and credentials — PASS. Raw messaging credentials are neither persisted nor logged; comparisons use recorded hashes and replacement values remain confined to the existing validation and apply path.
- Input validation and injection resistance — PASS. Only explicitly supplied active-channel credentials are compared. Changed values are validated before backup, deletion, or recreation; missing and unchanged values preserve reuse.
- Authentication and authorization — PASS. Manifest and registry authority, channel ownership, and stopped or disabled channel state remain authoritative. No identity or permission surface is widened.
- Dependencies and supply chain — PASS. No dependency, lockfile, image source, download, or external repository change occurs in this effective patch.
- Error handling and information exposure — PASS. Validation failure leaves the existing sandbox unchanged. Recreation errors remain explicit and recoverable; credential values are not exposed.
- Cryptography and data protection — PASS. Hash comparison semantics are unchanged. The latest child commit narrows the documentation to supported workspace and manifest-declared state and explicitly states that files outside those paths are not preserved.
- Configuration and infrastructure — PASS. Credential drift uses the existing backup, recreation, and restore workflow. Provider-effect recovery, sandbox intent boundaries, and channel configuration authority remain intact.
- Security testing — PASS. The implementation revision passed the prior exact nine-category review. At this exact head, the credential-rotation documentation contract passes 6/6 and strict docs validation passes with 0 errors and 2 existing warnings. Fresh GitHub Actions remains authoritative for the full revision.
- System security — PASS. Destructive work occurs only after validation through the owned-sandbox lifecycle. Cleanup targets and preserved paths are not broadened, stopped integrations are not revived, and the documentation now matches the actual preservation boundary.
The only change after the prior exact security review is the verified, signed-off documentation commit 0e6176d28cbe41397a341a5114147f2851e18bdd. It does not change executable behavior and resolves the preservation-boundary ambiguity without expanding the supported state surface. The exact effective patch has raw SHA-256 c39c00fb381bca3cdfebab5d145d7f1db62f753b8a5d548ab57f5483161fd515 and stable patch ID b8d0a2851a22171d859475206abeb49d44aabca9.
Fresh CI, protected E2E, original-reviewer resolution, and independent exact-head approval remain mandatory; this review waives none of those gates.
|
The OpenClaw managed-startup failure is classified as transient startup timing, not a PR-related code failure:
I requested the single bounded failed-job retry for this branch revision: https://github.com/NVIDIA/NemoClaw/actions/runs/31033638864. The earlier |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security review — exact head 303e92e1511eac489bb3c4fe7870d6609b8f344a against base 3b208d79e5d3bda4183704145ee5c28d79876ae1: PASS with no findings. The effective patch is safe from the reviewed security perspective, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. Raw messaging credentials are neither persisted nor logged; comparisons use recorded hashes and replacement values remain confined to the existing validation and apply path.
-
Input Validation and Data Sanitization — PASS. Only explicitly supplied active-channel credentials are compared. Changed values are validated before backup, deletion, or recreation; missing and unchanged values preserve reuse.
-
Authentication and Authorization — PASS. Manifest and registry authority, channel ownership, and stopped or disabled channel state remain authoritative. No identity or permission surface is widened.
-
Dependencies and Third-Party Libraries — PASS. No dependency, lockfile, image source, download, or external repository change occurs in this effective patch.
-
Error Handling and Logging — PASS. Validation failure leaves the existing sandbox unchanged. Recreation errors remain explicit and recoverable; credential values are not exposed.
-
Cryptography and Data Protection — PASS. Hash comparison semantics are unchanged. The latest child commit narrows the documentation to supported workspace and manifest-declared state and explicitly states that files outside those paths are not preserved.
-
Configuration and Security Headers — PASS. Credential drift uses the existing backup, recreation, and restore workflow. Provider-effect recovery, sandbox intent boundaries, and channel configuration authority remain intact.
-
Security Testing — PASS. The implementation revision passed the prior exact nine-category review. At this exact head, the credential-rotation documentation contract passes 6/6 and strict docs validation passes with 0 errors and 2 existing warnings. Fresh GitHub Actions remains authoritative for the full revision.
-
System Security — PASS. Destructive work occurs only after validation through the owned-sandbox lifecycle. Cleanup targets and preserved paths are not broadened, stopped integrations are not revived, and the documentation now matches the actual preservation boundary.
Files Reviewed
docs/reference/troubleshooting.mdxdocs/security/credential-rotation.mdxsrc/lib/messaging/applier/host-state-applier.tssrc/lib/onboard/machine/handlers/sandbox-create-intent-boundary.test.tssrc/lib/onboard/machine/handlers/sandbox-credential-drift.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.tssrc/lib/onboard/machine/handlers/sandbox-resume.test.tssrc/lib/onboard/machine/handlers/sandbox-resume.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/messaging-channel-setup.test.tssrc/lib/onboard/messaging-channel-setup.tstest/credential-rotation-docs.test.ts
Provenance and Required Gates
The signed refresh merge is conflict-free and preserves the effective 13-file patch byte-for-byte (raw SHA-256 c39c00fb381bca3cdfebab5d145d7f1db62f753b8a5d548ab57f5483161fd515; stable patch ID b8d0a2851a22171d859475206abeb49d44aabca9). Base PR #8372 changes the canonical security rubric and PR Review Advisor machinery only; it does not alter any effective PR file or runtime trust boundary. This review independently applies the new rubric.
Fresh CI, protected E2E, documentation receipt, unresolved-review resolution, and independent exact-head approval remain mandatory. This security review waives none of those gates.
|
Current revision
The remaining human gates are the explicitly unchecked independent security receipt, clearance of the historical changes-requested review, and authorization/execution of the selected protected E2E plan. No gate bypass, reviewer request, or additional retry is being attempted, and the merge freeze remains active. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security review — exact head 9a17d24874bb7cc5efb9453ccea6f8042f3ac231 against base c6713c9dd17a9682654713ccb6829b8714f1b307: PASS with no findings. The effective patch is safe from the reviewed security perspective, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. Raw messaging credentials are neither persisted nor logged; comparisons use recorded hashes and replacement values remain confined to the existing validation and apply path.
-
Input Validation and Data Sanitization — PASS. Only explicitly supplied active-channel credentials are compared. Changed values are validated before backup, deletion, or recreation; missing and unchanged values preserve reuse.
-
Authentication and Authorization — PASS. Manifest and registry authority, channel ownership, and stopped or disabled channel state remain authoritative. No identity or permission surface is widened.
-
Dependencies and Third-Party Libraries — PASS. No dependency, lockfile, image source, download, or external repository change occurs in this effective patch.
-
Error Handling and Logging — PASS. Validation failure leaves the existing sandbox unchanged. Recreation errors remain explicit and recoverable; credential values are not exposed.
-
Cryptography and Data Protection — PASS. Hash comparison semantics are unchanged. The latest child commit narrows the documentation to supported workspace and manifest-declared state and explicitly states that files outside those paths are not preserved.
-
Configuration and Security Headers — PASS. Credential drift uses the existing backup, recreation, and restore workflow. Provider-effect recovery, sandbox intent boundaries, and channel configuration authority remain intact.
-
Security Testing — PASS. The implementation revision passed the prior exact nine-category review. At this exact head, the credential-rotation documentation contract passes 6/6 and strict docs validation passes with 0 errors and 2 existing warnings. Fresh GitHub Actions remains authoritative for the full revision.
-
System Security — PASS. Destructive work occurs only after validation through the owned-sandbox lifecycle. Cleanup targets and preserved paths are not broadened, stopped integrations are not revived, and the documentation now matches the actual preservation boundary.
Files Reviewed
docs/reference/troubleshooting.mdxdocs/security/credential-rotation.mdxsrc/lib/messaging/applier/host-state-applier.tssrc/lib/onboard/machine/handlers/sandbox-create-intent-boundary.test.tssrc/lib/onboard/machine/handlers/sandbox-credential-drift.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.tssrc/lib/onboard/machine/handlers/sandbox-resume.test.tssrc/lib/onboard/machine/handlers/sandbox-resume.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/messaging-channel-setup.test.tssrc/lib/onboard/messaging-channel-setup.tstest/credential-rotation-docs.test.ts
Provenance and Required Gates
The signed refresh merge is conflict-free and preserves the effective 13-file patch byte-for-byte (raw SHA-256 c39c00fb381bca3cdfebab5d145d7f1db62f753b8a5d548ab57f5483161fd515; stable patch ID b8d0a2851a22171d859475206abeb49d44aabca9). Base PR #8383 adds only the v0.0.103 release-note page and does not alter any effective PR file or reviewed runtime trust boundary. Base PR #8372 changes the canonical security rubric and PR Review Advisor machinery only; it does not alter any effective PR file or runtime trust boundary. This review independently applies the new rubric.
Fresh CI, protected E2E, documentation receipt, unresolved-review resolution, and independent exact-head approval remain mandatory. This security review waives none of those gates.
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security revalidation — exact head 34fcbe1feefc5ccdc7ee0e2b4801f47f64089818 against current base 1608281462923ff8282151c7be1a109ee262b4e9: PASS with no findings, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. The effective PR patch is unchanged from the prior exact review. The base refresh adds no overlapping secret, credential, environment, or sensitive-logging path.
-
Input Validation and Data Sanitization — PASS. The reviewed validation and command/path construction are unchanged. The new base delta does not alter an effective PR file.
-
Authentication and Authorization — PASS. No identity, ownership, permission, or authorization boundary changed in the PR patch or its integration with the refreshed base.
-
Dependencies and Third-Party Libraries — PASS. The PR's reviewed dependency and artifact identities are unchanged. Base PR #8387 adds verified llama.cpp GGUF acquisition in eight disjoint files and does not change this PR's dependency surface.
-
Error Handling and Logging — PASS. The reviewed fail-closed behavior and diagnostics are unchanged; the base refresh introduces no overlapping error or logging path.
-
Cryptography and Data Protection — PASS. The PR's reviewed cryptographic and protected-data behavior is unchanged. No new cross-patch data flow exists.
-
Configuration and Security Headers — PASS. The effective PR patch preserves its reviewed configuration, network, privilege, capability, port, and header boundaries.
-
Security Testing — PASS. Raw patch SHA-256 remains
c39c00fb381bca3cdfebab5d145d7f1db62f753b8a5d548ab57f5483161fd515and stable patch ID remainsb8d0a2851a22171d859475206abeb49d44aabca9. Documentation writer revalidation passed at this exact head; the existing PR documentation remains accurate. Prior targeted test evidence remains applicable; fresh exact-head GitHub Actions is authoritative. -
System Security — PASS. The signed refresh is conflict-free, preserves the reviewed patch byte-for-byte, and introduces no new runtime trust-boundary interaction with the disjoint base delta.
Provenance and Required Gates
This exact-head revalidation supplements the full prior nine-category review at #7853 (review). It applies the current rubric from .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa.
Fresh required CI, protected E2E, the exact documentation receipt, unresolved-review resolution, GitHub commit verification, and an independent exact-head approval remain mandatory. This review waives none of those gates.
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security review — exact head ebd14c8d4fbbce1153f780b3c2d64db215807793 against current base b148bc13ee53b7a1f612d07e42275ec732e71877: PASS with no findings, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. The effective PR patch is unchanged and adds no secret, credential source, environment exposure, or sensitive logging. The refreshed base introduces no overlapping credential path.
-
Input Validation and Data Sanitization — PASS. Reviewed validation, command construction, and path handling remain unchanged. The portable-profile base delta has no exact file overlap with this PR.
-
Authentication and Authorization — PASS. No identity, ownership, permission, or authorization boundary is widened by the PR patch or its integration with the refreshed base.
-
Dependencies and Third-Party Libraries — PASS. The PR's reviewed dependency and artifact identities are unchanged. Base PRs #8333 and #8376 add disjoint E2E artifact restoration and portable experimental onboarding behavior.
-
Error Handling and Logging — PASS. Reviewed fail-closed behavior and diagnostics remain unchanged; the signed integrations add no overlapping error or logging path.
-
Cryptography and Data Protection — PASS. Reviewed cryptographic and protected-data behavior remains unchanged, with no new cross-patch data flow.
-
Configuration and Security Headers — PASS. The effective PR patch preserves its reviewed configuration, network, privilege, capability, port, and header boundaries. The hidden portable profile does not bypass this PR's authority checks.
-
Security Testing — PASS. Raw patch SHA-256 remains
c39c00fb381bca3cdfebab5d145d7f1db62f753b8a5d548ab57f5483161fd515and stable patch ID remainsb8d0a2851a22171d859475206abeb49d44aabca9. Adjacent portable-profile integration was rechecked locally: 7 focused files and 163 tests pass, including credential drift, messaging, resume, intent-boundary, and documentation contracts. Exact-head documentation writer revalidation passed; fresh GitHub Actions is authoritative. -
System Security — PASS. Both signed refreshes are conflict-free, preserve the effective patch byte-for-byte, and introduce no new runtime trust-boundary interaction. For #7853 specifically, the portable profile supplies onboarding defaults but neither stages messaging credentials nor bypasses registry-driven credential-drift validation.
Provenance and Required Gates
The exact head preserves contributor history through signed merge commits and applies the current rubric from .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa. This supplements the full prior nine-category review at #7853 (review).
Fresh required CI, protected E2E, the exact documentation receipt, unresolved-review resolution, GitHub commit verification, and an independent exact-head approval remain mandatory. This review waives none of those gates.
|
Review follow-up for revision
No additional code or documentation change is indicated. The |
|
Current blocker disposition: ordinary CI, documentation review, security scanning, DCO, and the complete test suite are green. Both advisor lanes failed only because their inference services returned HTTP 429 during analysis; the publisher preserved zero findings, and the already-used attempt will not be retried again. After confirming the fork branch still has the reviewed five-file patch and changes no workflow or credential-exposure path, I authorized the first protected E2E run through the repository controller: https://github.com/NVIDIA/NemoClaw/actions/runs/31057671147. The prior actionable documentation feedback remains addressed. A fresh human review is still required because the earlier change request remains recorded. |
|
Maintainer gate disposition for revision
The release freeze is lifted, but GitHub still records CHANGES_REQUESTED. This PR must not merge until an independent human reviewer approves the addressed changes. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Maintainer follow-up for the current branch revision
Fresh repository CI and protected E2E are starting. GitHub still records |
Summary
Detect an explicitly supplied messaging credential whose hash differs from the durable sandbox plan before a ready sandbox is reused. The resume flow validates the replacement credential and enters sandbox recreation instead of retaining the old credential binding.
Replacement credentials are validated before backup, deletion, or recreation. Missing or unchanged credentials preserve reuse, and channels stopped or disabled in durable state remain inactive.
Related Issue
Fixes #3631
Changes
Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/security/credential-rotation.mdxanddocs/reference/troubleshooting.mdxdescribe validation-before-mutation, sandbox recreation, active credential bindings, and the supported-state preservation boundary. The independent final review found the lifecycle, data-safety boundary, credential-family coverage, OpenClaw/Hermes scope, and Deep Agents exclusion complete and accurate, with no edits needed. The focused documentation contract passed 6/6 and the strict documentation build completed with 0 errors and 2 existing warnings.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every previously published commit appears asVerifiedin GitHub; the current-main merge commit is signed and will be rechecked after pushnpm run docsbuilds without warnings (doc changes only) — 2 existing warnings remainGitHub Actions
Fresh repository CI, protected E2E evidence, review clearance, and independent approval are required for the current branch revision. No prior result is waived or carried forward as a merge gate.
Signed-off-by: Ho Lim subhoya@gmail.com
Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com