Skip to content

fix(security): harden local credential capture#6439

Merged
cv merged 22 commits into
mainfrom
codex/harden-local-credential-helper
Jul 8, 2026
Merged

fix(security): harden local credential capture#6439
cv merged 22 commits into
mainfrom
codex/harden-local-credential-helper

Conversation

@cv

@cv cv commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

This follow-up to #6423 replaces prose-generated credential capture with a checked-in, authenticated one-shot loopback helper and a hardened local form. It binds the reviewed helper/form bytes to immutable commit URLs and SHA-256 digests so coding agents fail closed before collecting secrets. The helper now requires an explicit isolated or account-home execution profile so stateless commands use private config roots while persistent install/onboard commands use only an explicitly approved absolute working directory and the OS account home.

Changes

  • Add a Node.js 22 helper that validates the approved field schema, form digest, loopback request boundary, one-shot capability, and exact child argv without invoking a shell.
  • Require explicit isolated and account-home profiles, with private per-session config/temp roots for isolated work and reconstructed account-home roots plus an approved absolute cwd for persistent work.
  • Deny ambient NEMOCLAW_*, OPENSHELL_*, config-root, loader, package-manager, container, crypto-provider, and other process-control selectors while permitting only explicitly approved collected fields.
  • Harden the local form with a no-network preview/edit/confirm flow, redacted summaries, strict response locking, and no retry after an ambiguous submission.
  • Pin both reviewed artifacts in the starter prompt and add a repository check for digest, package, executable-bit, and helper/form safety-rule parity.
  • Update user-facing starter-prompt summaries and add integration coverage for malformed requests, races, abandoned responses, CSP, form behavior, and immutable pins.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent nine-category repository security review completed with final PASS after the CodeQL filesystem-race, CodeRabbit memory-semantics/parity, and GPT-advisor ambient-environment findings were addressed; the helper/form boundary was reviewed for secret handling, input validation, authentication, error behavior, data protection, headers, security tests, and race safety.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project integration test/local-credential-helper.test.ts test/local-credential-helper-pin.test.ts test/starter-prompt-docs.test.ts passed 163/163 tests.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: umask 022 && npm test passed 14,323 tests across 1,267 passing files, with 39 expected skips and 1 todo.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — not a doc-only PR; the build completed with 0 errors and 2 pre-existing warnings.
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Advisor-required live E2E: run 28920925588 passed credential-sanitization and cloud-onboard at head a413ba3.

Additional checks: npm run checks, npm run typecheck:cli, npm run source-shape:check, npm run test-size:check, package dry-run contents, post-push raw artifact SHA-256 verification, and git diff --check all passed.


Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • New Features
    • Added a stricter “preview then confirm” credential-capture workflow before any credentialed commands are generated or executed.
    • Improved non-interactive guidance to support secure curl | bash execution with approved-command enforcement.
  • Bug Fixes
    • Hardened the local credential form UI and submission behavior with stronger input validation, loopback-only access, and safer handling of ambiguous/failed outcomes.
  • Documentation
    • Updated quickstarts and starter prompt text to match the new approve-then-capture flow.
  • Tests
    • Expanded integration and security tests for the local helper, integrity pinning, and credential/processing policy checks.
  • Chores
    • Packaged the hardened credential-form asset and added automated integrity verification.

cv added 2 commits July 7, 2026 20:02
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv added security area: docs Documentation, examples, guides, or docs build area: install Install, setup, prerequisites, or uninstall flow area: security Security controls, permissions, secrets, or hardening bug-fix PR fixes a bug or regression v0.0.77 Release target labels Jul 8, 2026
@cv cv self-assigned this Jul 8, 2026
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR tightens the local credential helper flow, hardens the checked-in credential form, adds pinning and policy checks, and updates docs and tests for the new capability-gated submit path.

Changes

Local credential helper flow

Layer / File(s) Summary
Prompt and quickstart docs
docs/_components/StarterPrompt.tsx, docs/get-started/quickstart*.mdx, docs/index.mdx, docs/resources/agent-skills.mdx
Updates the starter prompt and related quickstart text to reference the checked-in helper and form after exact command approval.
Helper and credential form
scripts/local-credential-helper.mts, docs/resources/local-credential-form.html, package.json
Implements the helper CLI and request handling, and reworks the form into preview/edit/confirm behavior with stricter validation and capability gating.
Integrity pinning and security policy
scripts/checks/local-credential-helper-pin.ts, scripts/checks/run.ts, src/lib/security/*
Adds repository checks, credential-shape policy, and process-control policy modules with parity tests and packaging wiring.
Pin check extraction tests
test/local-credential-helper-pin.test.ts
Adds parser coverage for the pin-check extraction helpers and decoy rejection cases.
Local credential helper integration tests
test/local-credential-helper.test.ts
Adds end-to-end coverage for helper startup, rejection paths, concurrency, and abandoned submissions.
Starter prompt docs test updates
test/starter-prompt-docs.test.ts
Updates fixtures, harness behavior, and flow assertions for the pinned helper and form.

Estimated code review effort: 5 (Critical) | ~120 minutes

Suggested labels: area: e2e, refactor

Suggested reviewers: ericksoa

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main security hardening of the local credential capture flow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/harden-local-credential-helper

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the branch.

TypeScript / code-coverage/cli

The overall coverage in the branch remains at 76%, unchanged from the branch.

Show a code coverage summary of the most impacted files.
File 5ddf9a1 ec50830 +/-
src/lib/messagi...ate-resolver.ts 94% 71% -23%
src/lib/messagi...ate-resolver.ts 100% 95% -5%
src/lib/inference/config.ts 99% 97% -2%
src/lib/security/redact.ts 97% 98% +1%
src/lib/adapters/http/probe.ts 89% 91% +2%
src/lib/onboard...ure-contract.ts 89% 91% +2%
src/lib/runner.ts 80% 83% +3%
src/lib/core/url-utils.ts 97% 100% +3%
src/lib/adapters/docker/run.ts 67% 100% +33%
src/lib/securit...-control-env.ts 0% 100% +100%

Updated July 08, 2026 20:22 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: credential-sanitization, cloud-onboard, inference-routing
Optional E2E: security-posture, docs-validation, full-e2e

Dispatch hint: credential-sanitization,cloud-onboard,inference-routing

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • credential-sanitization (medium): Required because credential filtering and credential-shaped environment detection changed. This live E2E validates credential stripping and sandbox credential-boundary behavior after real install/onboard.
  • cloud-onboard (medium): Required because the PR changes assistant-facing install/onboarding credential handoff guidance and ships the helper/form as package artifacts. A real hosted onboarding run gives merge-blocking confidence that the primary onboarding path still works with provider credentials.
  • inference-routing (medium): Required because credential-env changes can affect the environment inherited by HTTP/curl probes and managed inference routing. This catches regressions in the live inference.local routing path.

Optional E2E

  • security-posture (medium): Useful additional confidence for the broader security-boundary changes, especially because the new helper/form process-control policy is intended to reduce child-process and credential exposure risk across agent modes.
  • docs-validation (low): Useful because the starter prompt and quickstarts changed substantially; this validates the live docs E2E docs contract, but it does not replace credential/security live coverage.
  • full-e2e (medium): Optional broad smoke for the complete OpenClaw install, onboard, sandbox, and live inference user journey after package/docs helper shipping changes.

New E2E recommendations

  • local-credential-helper (high): No existing live E2E appears to exercise the new standalone local credential helper and browser form as a real loopback flow. Add an E2E that runs the shipped helper with the shipped form, opens/submits to the one-time 127.0.0.1 URL with a synthetic secret, verifies capability one-shot behavior, validates isolated and account-home execution profiles, asserts forbidden process-control field rejection, and confirms the approved child command receives only the intended redacted environment without leaking secrets to logs/artifacts.
    • Suggested test: local-credential-helper-live
  • assistant-install-credential-flow (medium): The docs starter prompt now defines a security-sensitive assistant-mediated credential collection flow, but existing E2E coverage does not validate that an assistant can follow the documented helper/form command shape during install or onboarding. Add a workflow-dispatched E2E that follows the prompt path with synthetic credentials and asserts the approved install/onboard command shape and redaction behavior.
    • Suggested test: starter-prompt-credential-onboard-flow

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: credential-sanitization,cloud-onboard,inference-routing

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: ubuntu-repo-cloud-openclaw
Optional E2E targets: ubuntu-repo-cloud-langchain-deepagents-code

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • ubuntu-repo-cloud-openclaw: The PR changes shared credential-shaped environment detection, process-control environment policy, and the shipped local credential helper/form surface. The Ubuntu OpenClaw live target is the smallest live-supported typed target that runs repo-current onboarding with hosted credentials and includes the credentials suite.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw

Optional E2E targets

  • ubuntu-repo-cloud-langchain-deepagents-code: Optional adjacent coverage for the same credential/security environment changes through the LangChain Deep Agents Code onboarding and terminal-agent path, which has additional credential/proxy redaction assertions.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Relevant changed files

  • docs/resources/local-credential-form.html
  • package.json
  • scripts/local-credential-helper.mts
  • src/lib/security/credential-env.ts
  • src/lib/security/process-control-env.ts

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Blocked

Merge posture: Do not merge until addressed
Primary next action: Fix PRA-5: No canonical security-standalone module for duplicated security predicates; then add or justify PRA-T1.
Open items: 6 required · 10 warnings · 6 suggestions · 8 test follow-ups
Since last review: 1 prior item resolved · 13 still apply · 0 new items found

Action checklist

  • PRA-5 Fix: No canonical security-standalone module for duplicated security predicates in scripts/local-credential-helper.mts:147
  • PRA-6 Fix: SANITIZED_PRIVILEGED_ENV hardcoded list diverges from canonical process-control policy in src/lib/sandbox/privileged-exec.ts:56
  • PRA-7 Fix: Helper requires Node.js 22.16+ but has no runtime version check in scripts/local-credential-helper.mts:1010
  • PRA-8 Fix: NEMOCLAW_* prefix not blocked in forbidden environment names in scripts/local-credential-helper.mts:223
  • PRA-9 Fix: Isolated execution root created under repo scripts/ directory instead of system temp in scripts/local-credential-helper.mts:258
  • PRA-10 Fix: sanitizeInheritedChildEnvironment exists only in standalone helper — no canonical source in scripts/local-credential-helper.mts:235
  • PRA-1 Resolve or justify: Source-of-truth review needed: sanitizeInheritedChildEnvironment in helper only
  • PRA-2 Resolve or justify: Source-of-truth review needed: XDG_CONFIG_DIRS/XDG_DATA_DIRS as single paths
  • PRA-3 Resolve or justify: Source-of-truth review needed: NEMOCLAW_* exact names only, no prefix rule
  • PRA-4 Resolve or justify: Source-of-truth review needed: Execution root in repo scripts/ directory
  • PRA-11 Resolve or justify: Pin check missing sanitizeInheritedChildEnvironment source-text parity test in test/local-credential-helper-pin.test.ts:1
  • PRA-12 Resolve or justify: Starter prompt missing account-home profile warning about ~/.ssh, ~/.config, ~/.npmrc access in docs/_components/StarterPrompt.tsx:102
  • PRA-13 Resolve or justify: clearCredentialReferences called after spawn — best-effort only, strings not zeroed in scripts/local-credential-helper.mts:860
  • PRA-14 Resolve or justify: Missing test for Windows absolute executable path in test/local-credential-helper.test.ts:615
  • PRA-15 Resolve or justify: XDG_CONFIG_DIRS and XDG_DATA_DIRS set as single paths, not colon-separated lists per XDG spec in scripts/local-credential-helper.mts:278
  • PRA-16 Resolve or justify: sanitizeInheritedChildEnvironment misnamed — returns {} for isolated, not used for account-home in scripts/local-credential-helper.mts:235
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Helper requires Node.js 22.16+ but has no runtime version check
  • PRA-T7 Add or justify test follow-up: Pin check missing sanitizeInheritedChildEnvironment source-text parity test
  • PRA-T8 Add or justify test follow-up: Missing test for Windows absolute executable path
  • PRA-17 In-scope improvement: CSP hashes computed at runtime via buildCredentialFormCsp() instead of precomputed at build time in scripts/local-credential-helper.mts:691
  • PRA-18 In-scope improvement: Non-absolute executable test uses toContain instead of exact error message in test/local-credential-helper.test.ts:615
  • PRA-19 In-scope improvement: Pin check uses TypeScript compiler API — document as intentional design in scripts/checks/local-credential-helper-pin.ts:1
  • PRA-20 In-scope improvement: Refactor privileged-exec.ts to import canonical process-control policy in src/lib/sandbox/privileged-exec.ts:22
  • PRA-21 In-scope improvement: Pin check TypeScript compiler API usage — document in test file in test/local-credential-helper-pin.test.ts:1
  • PRA-22 In-scope improvement: Create shared security-standalone.ts module to deduplicate 5+ embedded security predicate copies in scripts/security-standalone.ts:1

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-4 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-5 Required architecture scripts/local-credential-helper.mts:147 Create scripts/security-standalone.ts exporting CREDENTIAL_SHAPED_NAME_PATTERN, PROCESS_CONTROL_ENV_NAMES, isCredentialShapedName, isProcessControlEnvName, and sanitizeInheritedChildEnvironment (renamed to createIsolatedChildEnvironment). Update helper to import with --experimental-strip-types. Update form HTML to fetch module and eval (or generate embedded copy from module at build). Update pin check to verify module is source of truth.
PRA-6 Required security src/lib/sandbox/privileged-exec.ts:56 Update privileged-exec.ts to import isProcessControlEnvName from process-control-env.ts and generate sanitization list dynamically at runtime. Add parity test in pin check verifying every canonical process-control name is covered.
PRA-7 Required tests scripts/local-credential-helper.mts:1010 Add runtime version check at top of main(): parse process.version, verify >= 22.16.0, exit with clear error message if not. Add test 'rejects Node.js < 22.16 with clear error' in test/local-credential-helper.test.ts (mock process.version).
PRA-8 Required security scripts/local-credential-helper.mts:223 Add name.startsWith('NEMOCLAW_') to isForbiddenChildEnvName in helper.mts and isProcessControlEnvName in process-control-env.ts. Update FORBIDDEN_CHILD_ENV_NAMES and PROCESS_CONTROL_ENV_NAMES accordingly. Update pin check.
PRA-9 Required security scripts/local-credential-helper.mts:258 Change createPrivateExecutionRoot to use mkdtempSync(path.join(os.tmpdir(), 'nemoclaw-credential-child-')). Ensure 0o700 permissions on root and all subdirectories (already done at line 262).
PRA-10 Required architecture scripts/local-credential-helper.mts:235 Rename to createIsolatedChildEnvironment and move to scripts/security-standalone.ts. Update helper and form to import/use it. Document: 'Returns empty object for isolated profile — child receives only profile environment + submitted fields. For account-home, ambient env is not sanitized (uses accountHomeEnvironment directly).'
PRA-11 Resolve/justify tests test/local-credential-helper-pin.test.ts:1 Add test in test/local-credential-helper-pin.test.ts that extracts the function from helper using TS compiler API (like extractCredentialPattern) and compares against expected source snapshot. Once security-standalone.ts exists, verify helper imports it.
PRA-12 Resolve/justify docs docs/_components/StarterPrompt.tsx:102 Add bullet: '⚠️ The account-home profile grants the command access to your user configuration directories (e.g., ~/.ssh, ~/.config, ~/.npmrc). Only use this profile for commands that must persist account state, such as the initial NemoClaw install. Prefer isolated profile for one-shot credential capture.'
PRA-13 Resolve/justify correctness scripts/local-credential-helper.mts:860 Move clearCredentialReferences calls before spawn for values object (childEnv is newly created). Add explicit comment: 'Best-effort exposure minimization only. JavaScript strings and child process environment copies cannot be reliably zeroed.'
PRA-14 Resolve/justify tests test/local-credential-helper.test.ts:615 Add test case 'accepts Windows absolute executable path' using path.resolve('C:', 'Windows', 'System32', 'cmd.exe') or similar. Verify helper spawns successfully with such path.
PRA-15 Resolve/justify correctness scripts/local-credential-helper.mts:278 Set XDG_CONFIG_DIRS and XDG_DATA_DIRS to colon-separated lists with the private directory as the only entry: path.join(root, 'config-dirs') + ':' + path.join(root, 'config-dirs').
PRA-16 Resolve/justify security scripts/local-credential-helper.mts:235 Rename to createIsolatedChildEnvironment to match behavior. Add comment: 'Returns empty object for isolated profile — child receives only profile environment + submitted fields. For account-home, ambient env is not sanitized (uses accountHomeEnvironment directly).'
PRA-17 Improvement architecture scripts/local-credential-helper.mts:691 Add build step (e.g., in package.json prepare or scripts/checks) to compute CSP hashes from form HTML and embed as constants EXPECTED_FORM_SCRIPT_CSP_HASH and EXPECTED_FORM_STYLE_CSP_HASH in helper.mts. Remove buildCredentialFormCsp runtime call at line 787.
PRA-18 Improvement tests test/local-credential-helper.test.ts:615 Update test to use toBe() with exact error message string: 'The approved command executable must use an absolute path'.
PRA-19 Improvement architecture scripts/checks/local-credential-helper-pin.ts:1 Add comment in pin check header: 'Uses TypeScript compiler API for structural source-text parity verification. This ignores comments, formatting, and string-embedded decoys while verifying executable code structure.'
PRA-20 Improvement security src/lib/sandbox/privileged-exec.ts:22 Replace SANITIZED_PRIVILEGED_ENV const with dynamic generation using imported isProcessControlEnvName from process-control-env.ts. Filter process.env at runtime or generate --env args dynamically.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-5 Required — No canonical security-standalone module for duplicated security predicates

  • Location: scripts/local-credential-helper.mts:147
  • Category: architecture
  • Problem: The security-critical predicates (CREDENTIAL_SHAPED_NAME_PATTERN, PROCESS_CONTROL_ENV_NAMES, isCredentialShapedName, isProcessControlEnvName, sanitizeInheritedChildEnvironment) are duplicated across credential-env.ts, process-control-env.ts, helper.mts, and form HTML. No scripts/security-standalone.ts module exists as single source of truth.
  • Impact: Future divergence risk: changes to canonical policy may not propagate to helper/form, creating security gaps. Pin check enforces parity but cannot prevent drift if canonical source doesn't exist.
  • Required action: Create scripts/security-standalone.ts exporting CREDENTIAL_SHAPED_NAME_PATTERN, PROCESS_CONTROL_ENV_NAMES, isCredentialShapedName, isProcessControlEnvName, and sanitizeInheritedChildEnvironment (renamed to createIsolatedChildEnvironment). Update helper to import with --experimental-strip-types. Update form HTML to fetch module and eval (or generate embedded copy from module at build). Update pin check to verify module is source of truth.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check if scripts/security-standalone.ts exists and is imported by helper.mts and form HTML
  • Missing regression test: Pin check test verifying helper imports security-standalone.ts and form HTML embeds generated copy
  • Done when: The required change is committed and verification passes: Check if scripts/security-standalone.ts exists and is imported by helper.mts and form HTML.
  • Evidence: Helper and form embed identical literal copies of credential pattern, process-control names, and predicates. Pin check verifies parity but has no canonical module to reference.

PRA-6 Required — SANITIZED_PRIVILEGED_ENV hardcoded list diverges from canonical process-control policy

  • Location: src/lib/sandbox/privileged-exec.ts:56
  • Category: security
  • Problem: privileged-exec.ts defines SANITIZED_PRIVILEGED_ENV as 18 hardcoded entries. The canonical process-control-env.ts defines 100+ exact names plus prefix rules (BASH_FUNC_, LD_, DYLD_, GIT_CONFIG_, GIT_TRACE, NPM_CONFIG_, OPENSHELL_, PIP_). Privileged sandbox exec could inherit process-control variables that the credential helper blocks.
  • Impact: Security gap: privileged container exec may leak proxy settings, loader hooks, config paths, or Git/SSL credentials that are stripped in credential handoff.
  • Required action: Update privileged-exec.ts to import isProcessControlEnvName from process-control-env.ts and generate sanitization list dynamically at runtime. Add parity test in pin check verifying every canonical process-control name is covered.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read src/lib/sandbox/privileged-exec.ts:56-69 and verify it imports from process-control-env.ts
  • Missing regression test: Test in test/local-credential-helper-pin.test.ts verifying privileged-exec.ts sanitization list equals canonical policy
  • Done when: The required change is committed and verification passes: Read src/lib/sandbox/privileged-exec.ts:56-69 and verify it imports from process-control-env.ts.
  • Evidence: SANITIZED_PRIVILEGED_ENV has 18 entries vs 100+ in PROCESS_CONTROL_ENV_NAMES plus 8 prefix rules. No import from canonical module.

PRA-7 Required — Helper requires Node.js 22.16+ but has no runtime version check

  • Location: scripts/local-credential-helper.mts:1010
  • Category: tests
  • Problem: The helper uses --experimental-strip-types which requires Node.js >= 22.16.0. If run on older Node, it fails with cryptic syntax errors instead of a clear message.
  • Impact: Poor developer experience: coding agents running helper on unsupported Node versions get unactionable errors, potentially falling back to insecure credential handling.
  • Required action: Add runtime version check at top of main(): parse process.version, verify >= 22.16.0, exit with clear error message if not. Add test 'rejects Node.js < 22.16 with clear error' in test/local-credential-helper.test.ts (mock process.version).
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check main() function in helper.mts for process.version check before any other logic
  • Missing regression test: Test mocking process.version to verify clear error on old Node
  • Done when: The required change is committed and verification passes: Check main() function in helper.mts for process.version check before any other logic.
  • Evidence: main() function at line 1010 starts parsing CLI args immediately without version check. package.json engines specifies >=22.16.0 but no runtime enforcement.

PRA-8 Required — NEMOCLAW_* prefix not blocked in forbidden environment names

  • Location: scripts/local-credential-helper.mts:223
  • Category: security
  • Problem: isForbiddenChildEnvName has prefix rules for OPENSHELL_, NPM_CONFIG_, PIP_, LD_, DYLD_, BASH_FUNC_, GIT_CONFIG_, GIT_TRACE but NOT NEMOCLAW_*. FORBIDDEN_CHILD_ENV_NAMES contains only exact NEMOCLAW_* names. An ambient NEMOCLAW_CUSTOM_VAR could leak into the isolated child.
  • Impact: Credential/environment leakage: attacker-controlled ambient NEMOCLAW_* variables could influence child command behavior, bypassing the isolated profile's intent.
  • Required action: Add name.startsWith('NEMOCLAW_') to isForbiddenChildEnvName in helper.mts and isProcessControlEnvName in process-control-env.ts. Update FORBIDDEN_CHILD_ENV_NAMES and PROCESS_CONTROL_ENV_NAMES accordingly. Update pin check.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for NEMOCLAW_ prefix in isForbiddenChildEnvName (helper.mts:223) and isProcessControlEnvName (process-control-env.ts:140)
  • Missing regression test: Test verifying NEMOCLAW_* names are blocked in both helper and canonical policy
  • Done when: The required change is committed and verification passes: Search for NEMOCLAW_ prefix in isForbiddenChildEnvName (helper.mts:223) and isProcessControlEnvName (process-control-env.ts:140).
  • Evidence: isForbiddenChildEnvName has 8 prefix rules but not NEMOCLAW_. FORBIDDEN_CHILD_ENV_NAMES has 7 exact NEMOCLAW_* entries. PR description claims 'Deny ambient NEMOCLAW_*' but prefix rule missing.

PRA-9 Required — Isolated execution root created under repo scripts/ directory instead of system temp

  • Location: scripts/local-credential-helper.mts:258
  • Category: security
  • Problem: createPrivateExecutionRoot() uses path.join(path.dirname(fileURLToPath(import.meta.url)), '.credential-child-') which places the private root in the repository checkout. This violates isolation and could leave artifacts in the repo.
  • Impact: Isolation violation: temporary credential-handling artifacts (config, cache, data dirs) created in source tree. Could be committed accidentally or inspected by other processes.
  • Required action: Change createPrivateExecutionRoot to use mkdtempSync(path.join(os.tmpdir(), 'nemoclaw-credential-child-')). Ensure 0o700 permissions on root and all subdirectories (already done at line 262).
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check createPrivateExecutionRoot function for os.tmpdir() usage instead of repo-relative path
  • Missing regression test: Test verifying isolated execution root is under system temp directory (os.tmpdir())
  • Done when: The required change is committed and verification passes: Check createPrivateExecutionRoot function for os.tmpdir() usage instead of repo-relative path.
  • Evidence: Line 258: root = mkdtempSync(path.join(path.dirname(fileURLToPath(import.meta.url)), '.credential-child-')) — uses repo scripts/ directory.

PRA-10 Required — sanitizeInheritedChildEnvironment exists only in standalone helper — no canonical source

  • Location: scripts/local-credential-helper.mts:235
  • Category: architecture
  • Problem: The function sanitizeInheritedChildEnvironment (line 235) is defined only in helper.mts and embedded in form HTML. It returns {} for isolated profile (child gets only profile env + submitted fields) and is not used for account-home. No canonical module exports it.
  • Impact: Duplicated logic across 4 locations; function name misrepresents behavior (returns empty object, doesn't sanitize). Pin check cannot verify function body parity.
  • Required action: Rename to createIsolatedChildEnvironment and move to scripts/security-standalone.ts. Update helper and form to import/use it. Document: 'Returns empty object for isolated profile — child receives only profile environment + submitted fields. For account-home, ambient env is not sanitized (uses accountHomeEnvironment directly).'
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check if scripts/security-standalone.ts exists and exports createIsolatedChildEnvironment
  • Missing regression test: Pin check test extracting createIsolatedChildEnvironment source text and comparing to canonical
  • Done when: The required change is committed and verification passes: Check if scripts/security-standalone.ts exists and exports createIsolatedChildEnvironment.
  • Evidence: Function at line 235 returns {} with comment 'Unknown variables can be tool-specific execution controls. The child gets only the selected profile environment and explicitly submitted fields.' Name suggests sanitization but behavior is isolation.
Review findings by urgency: 6 required fixes, 10 items to resolve/justify, 6 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: sanitizeInheritedChildEnvironment in helper only

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Pin check verifying parity across all 4 locations
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Function exists only in helper.mts:235 and form HTML embedded script. Pin check verifies parity but has no canonical source to reference.

PRA-2 Resolve/justify — Source-of-truth review needed: XDG_CONFIG_DIRS/XDG_DATA_DIRS as single paths

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test verifying format matches spec (colon-separated single entry)
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Lines 278-279 in helper.mts: XDG_CONFIG_DIRS: path.join(root, 'config-dirs'), XDG_DATA_DIRS: path.join(root, 'data-dirs') — single paths not lists.

PRA-3 Resolve/justify — Source-of-truth review needed: NEMOCLAW_* exact names only, no prefix rule

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Pin check verifying prefix rule in both locations
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: isForbiddenChildEnvName has 8 prefix rules (OPENSHELL_, NPM_CONFIG_, PIP_, LD_, DYLD_, BASH_FUNC_, GIT_CONFIG_, GIT_TRACE) but not NEMOCLAW_. PR description claims 'Deny ambient NEMOCLAW_*' but prefix rule missing.

PRA-4 Resolve/justify — Source-of-truth review needed: Execution root in repo scripts/ directory

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test verifying temp location is under os.tmpdir()
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Line 258: root = mkdtempSync(path.join(path.dirname(fileURLToPath(import.meta.url)), '.credential-child-')) — uses repo scripts/ directory.

PRA-11 Resolve/justify — Pin check missing sanitizeInheritedChildEnvironment source-text parity test

  • Location: test/local-credential-helper-pin.test.ts:1
  • Category: tests
  • Problem: The pin check verifies credential pattern, process-control names, and predicates but does not extract and compare the sanitizeInheritedChildEnvironment/createIsolatedChildEnvironment function body.
  • Impact: Function body divergence between helper, form, and future canonical module would not be caught.
  • Recommended action: Add test in test/local-credential-helper-pin.test.ts that extracts the function from helper using TS compiler API (like extractCredentialPattern) and compares against expected source snapshot. Once security-standalone.ts exists, verify helper imports it.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/local-credential-helper-pin.test.ts for extractCredentialPattern pattern; add similar extractIsolatedChildEnvironment function
  • Missing regression test: Pin check test for createIsolatedChildEnvironment source-text parity
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/local-credential-helper-pin.test.ts for extractCredentialPattern pattern; add similar extractIsolatedChildEnvironment function.
  • Evidence: Pin check has extractCredentialPattern, extractEmbeddedFormDigest, extractProcessControlRules, extractStringSet, extractStarterPrompt but no function body extraction for sanitizeInheritedChildEnvironment.

PRA-12 Resolve/justify — Starter prompt missing account-home profile warning about ~/.ssh, ~/.config, ~/.npmrc access

  • Location: docs/_components/StarterPrompt.tsx:102
  • Category: docs
  • Problem: The prompt mentions account-home profile but does not warn that it grants the command access to user configuration directories.
  • Impact: Users may unknowingly grant install/onboard commands access to sensitive config directories when isolated profile would suffice.
  • Recommended action: Add bullet: '⚠️ The account-home profile grants the command access to your user configuration directories (e.g., ~/.ssh, ~/.config, ~/.npmrc). Only use this profile for commands that must persist account state, such as the initial NemoClaw install. Prefer isolated profile for one-shot credential capture.'
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search StarterPrompt.tsx for account-home warning about ~/.ssh, ~/.config, ~/.npmrc
  • Missing regression test: Doc test verifying account-home warning text present in STARTER_PROMPT constant
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search StarterPrompt.tsx for account-home warning about ~/.ssh, ~/.config, ~/.npmrc.
  • Evidence: StarterPrompt.tsx lines 100-110 describe account-home profile but no warning about config directory access.

PRA-13 Resolve/justify — clearCredentialReferences called after spawn — best-effort only, strings not zeroed

  • Location: scripts/local-credential-helper.mts:860
  • Category: correctness
  • Problem: clearCredentialReferences is called in launchApprovedCommand after child.spawn(). JavaScript strings and child process environment copies cannot be reliably zeroed. The call ordering means credentials exist in childEnv during spawn.
  • Impact: Credential exposure window: secrets exist in memory during spawn; child process receives copied env. Cannot be fully mitigated in JS but ordering can minimize window.
  • Recommended action: Move clearCredentialReferences calls before spawn for values object (childEnv is newly created). Add explicit comment: 'Best-effort exposure minimization only. JavaScript strings and child process environment copies cannot be reliably zeroed.'
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check launchApprovedCommand for clearCredentialReferences call ordering relative to spawn()
  • Missing regression test: N/A — design limitation, document in code
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check launchApprovedCommand for clearCredentialReferences call ordering relative to spawn().
  • Evidence: Line 860: clearCredentialReferences called after spawn(). Function comment at line 845 acknowledges 'JavaScript strings and a spawned child's copied environment cannot be reliably zeroed.'

PRA-14 Resolve/justify — Missing test for Windows absolute executable path

  • Location: test/local-credential-helper.test.ts:615
  • Category: tests
  • Problem: Tests reject non-absolute paths but do not test Windows absolute paths like C:\Windows\System32\cmd.exe.
  • Impact: Windows users may encounter false rejection if path validation doesn't handle drive-letter paths correctly.
  • Recommended action: Add test case 'accepts Windows absolute executable path' using path.resolve('C:', 'Windows', 'System32', 'cmd.exe') or similar. Verify helper spawns successfully with such path.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search test file for Windows absolute path test; add new it() block
  • Missing regression test: Test accepting Windows absolute executable path
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search test file for Windows absolute path test; add new it() block.
  • Evidence: Test at line 615 checks non-absolute rejection but no positive test for Windows absolute paths.

PRA-15 Resolve/justify — XDG_CONFIG_DIRS and XDG_DATA_DIRS set as single paths, not colon-separated lists per XDG spec

  • Location: scripts/local-credential-helper.mts:278
  • Category: correctness
  • Problem: privateExecutionEnvironment sets XDG_CONFIG_DIRS and XDG_DATA_DIRS as single paths. XDG Base Directory Specification requires colon-separated lists. Setting as single paths may break applications expecting list format.
  • Impact: Applications reading XDG_CONFIG_DIRS/XDG_DATA_DIRS may not find config/data directories if they expect colon-separated format.
  • Recommended action: Set XDG_CONFIG_DIRS and XDG_DATA_DIRS to colon-separated lists with the private directory as the only entry: path.join(root, 'config-dirs') + ':' + path.join(root, 'config-dirs').
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check privateExecutionEnvironment function for XDG_CONFIG_DIRS and XDG_DATA_DIRS values
  • Missing regression test: Test verifying XDG vars format matches specification (colon-separated single entry)
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check privateExecutionEnvironment function for XDG_CONFIG_DIRS and XDG_DATA_DIRS values.
  • Evidence: Lines 278-279: XDG_CONFIG_DIRS: path.join(root, 'config-dirs'), XDG_DATA_DIRS: path.join(root, 'data-dirs') — single paths, not lists.

PRA-16 Resolve/justify — sanitizeInheritedChildEnvironment misnamed — returns {} for isolated, not used for account-home

  • Location: scripts/local-credential-helper.mts:235
  • Category: security
  • Problem: Function name suggests it sanitizes an inherited environment, but for isolated profile it returns empty object (child gets only profile env + submitted fields). For account-home, ambient env is not sanitized (uses accountHomeEnvironment directly). This behavior divergence is not documented.
  • Impact: Misleading API: callers expecting sanitization get isolation. Future maintainers may misuse.
  • Recommended action: Rename to createIsolatedChildEnvironment to match behavior. Add comment: 'Returns empty object for isolated profile — child receives only profile environment + submitted fields. For account-home, ambient env is not sanitized (uses accountHomeEnvironment directly).'
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check function name and comment for sanitizeInheritedChildEnvironment at line 235
  • Missing regression test: N/A — code clarity improvement
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check function name and comment for sanitizeInheritedChildEnvironment at line 235.
  • Evidence: Function returns {} unconditionally. accountHomeEnvironment used directly in createExecutionContext for account-home profile.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-17 Improvement — CSP hashes computed at runtime via buildCredentialFormCsp() instead of precomputed at build time

  • Location: scripts/local-credential-helper.mts:691
  • Category: architecture
  • Problem: buildCredentialFormCsp() parses form HTML at startup to extract script/style tags and compute SHA-256 hashes for CSP. This adds runtime overhead and requires form HTML to be parsed.
  • Impact: Minor startup latency; form HTML must be readable at runtime (already required).
  • Suggested action: Add build step (e.g., in package.json prepare or scripts/checks) to compute CSP hashes from form HTML and embed as constants EXPECTED_FORM_SCRIPT_CSP_HASH and EXPECTED_FORM_STYLE_CSP_HASH in helper.mts. Remove buildCredentialFormCsp runtime call at line 787.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check if CSP hashes are precomputed constants in helper.mts
  • Missing regression test: N/A — build-time optimization
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 691: export function buildCredentialFormCsp(formBytes: Buffer): string parses HTML. Line 787: const formCsp = buildCredentialFormCsp(formBytes).

PRA-18 Improvement — Non-absolute executable test uses toContain instead of exact error message

  • Location: test/local-credential-helper.test.ts:615
  • Category: tests
  • Problem: Test checks for 'approved command executable must use an absolute path' as substring rather than exact match.
  • Impact: Test could pass with different error message containing same substring.
  • Suggested action: Update test to use toBe() with exact error message string: 'The approved command executable must use an absolute path'.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test for non-absolute executable error message matching
  • Missing regression test: N/A — test precision improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Test uses expect(captured.output()).toContain('approved command executable must use an absolute path') instead of exact match.

PRA-19 Improvement — Pin check uses TypeScript compiler API — document as intentional design

  • Location: scripts/checks/local-credential-helper-pin.ts:1
  • Category: architecture
  • Problem: The pin check uses ts.createSourceFile and AST traversal for structural source-text parity verification. This sophisticated approach should be documented.
  • Impact: Future maintainers may not understand why TS compiler API is used instead of simpler string/regex matching.
  • Suggested action: Add comment in pin check header: 'Uses TypeScript compiler API for structural source-text parity verification. This ignores comments, formatting, and string-embedded decoys while verifying executable code structure.'
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check pin check file header for documentation comment
  • Missing regression test: N/A — documentation improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: No header comment explaining TS compiler API usage rationale.

PRA-20 Improvement — Refactor privileged-exec.ts to import canonical process-control policy

  • Location: src/lib/sandbox/privileged-exec.ts:22
  • Category: security
  • Problem: After parity test added, privileged-exec.ts should import from process-control-env.ts instead of hardcoded list. This is the fix for PRA-6 framed as improvement.
  • Impact: Eliminates maintenance burden and divergence risk.
  • Suggested action: Replace SANITIZED_PRIVILEGED_ENV const with dynamic generation using imported isProcessControlEnvName from process-control-env.ts. Filter process.env at runtime or generate --env args dynamically.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check privileged-exec.ts for import from process-control-env.ts
  • Missing regression test: Parity test in pin check
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: SANITIZED_PRIVILEGED_ENV at line 22 is hardcoded. process-control-env.ts exports isProcessControlEnvName.

PRA-21 Improvement — Pin check TypeScript compiler API usage — document in test file

  • Location: test/local-credential-helper-pin.test.ts:1
  • Category: architecture
  • Problem: Test file uses TS compiler API for structural parity verification. Should be documented.
  • Impact: Maintainability: test approach rationale not recorded.
  • Suggested action: Add comment in test file header explaining the TS compiler API approach for structural parity verification.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test file header for documentation
  • Missing regression test: N/A — documentation improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Test file has no header comment explaining extraction approach.

PRA-22 Improvement — Create shared security-standalone.ts module to deduplicate 5+ embedded security predicate copies

  • Location: scripts/security-standalone.ts:1
  • Category: architecture
  • Problem: Overarching fix for PRA-1, PRA-4, PRA-5, PRA-10, PRA-16, PRA-22. Single canonical module for all shared security predicates.
  • Impact: Eliminates duplication, ensures parity, simplifies maintenance.
  • Suggested action: Create scripts/security-standalone.ts exporting CREDENTIAL_SHAPED_NAME_PATTERN, PROCESS_CONTROL_ENV_NAMES, isCredentialShapedName, isProcessControlEnvName, and createIsolatedChildEnvironment. Update all consumers (helper.mts, form HTML, privileged-exec.ts, pin check).
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check if scripts/security-standalone.ts exists and is imported by all consumers
  • Missing regression test: Pin check verifying module is source of truth for all artifacts
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: 4 locations duplicate predicates: credential-env.ts, process-control-env.ts, helper.mts, form HTML. Pin check verifies parity but no canonical module.
Simplification opportunities: 11 possible cuts, net -978 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-5 shrink (scripts/local-credential-helper.mts:147): Duplicated predicate definitions in helper.mts (lines 50-150) and form HTML (lines 100-300)
    • Replacement: Import from scripts/security-standalone.ts
    • Net: -400 lines
    • Safety boundary: All security predicates must remain functionally identical; pin check enforces parity
  • PRA-6 delete (src/lib/sandbox/privileged-exec.ts:56): Hardcoded SANITIZED_PRIVILEGED_ENV const array (18 entries)
    • Replacement: Dynamic generation: const sanitized = [...PROCESS_CONTROL_ENV_NAMES].map(n => `${n}=`).concat(prefixRules.map(p => `${p}*`))
    • Net: -20 lines
    • Safety boundary: Must cover all canonical process-control names; pin check verifies parity
  • PRA-8 native (scripts/local-credential-helper.mts:223): Missing prefix rule in two functions
    • Replacement: Add name.startsWith('NEMOCLAW_') to both isForbiddenChildEnvName and isProcessControlEnvName
    • Net: 2 lines
    • Safety boundary: Must not block explicitly approved NEMOCLAW_* fields (validated at CLI parse time)
  • PRA-9 stdlib (scripts/local-credential-helper.mts:258): path.join(path.dirname(fileURLToPath(import.meta.url)), '.credential-child-')
    • Replacement: path.join(os.tmpdir(), 'nemoclaw-credential-child-')
    • Net: 0 lines
    • Safety boundary: 0o700 permissions on root and all subdirectories must be preserved
  • PRA-10 shrink (scripts/local-credential-helper.mts:235): sanitizeInheritedChildEnvironment function definition in helper.mts and embedded copy in form HTML
    • Replacement: Import createIsolatedChildEnvironment from scripts/security-standalone.ts
    • Net: -30 lines
    • Safety boundary: Behavior must remain: isolated profile gets empty inherited env; account-home uses accountHomeEnvironment directly
  • PRA-17 yagni (scripts/local-credential-helper.mts:691): buildCredentialFormCsp function (lines 691-710) and runtime call at line 787
    • Replacement: Precomputed constants: EXPECTED_FORM_SCRIPT_CSP_HASH and EXPECTED_FORM_STYLE_CSP_HASH
    • Net: -20 lines
    • Safety boundary: Form HTML must not change without rebuilding; pin check already verifies form SHA-256
  • PRA-18 shrink (test/local-credential-helper.test.ts:615): toContain substring match
    • Replacement: toBe exact string match
    • Net: 0 lines
    • Safety boundary: Error message text must not change
  • PRA-19 shrink (scripts/checks/local-credential-helper-pin.ts:1): Missing documentation comment
    • Replacement: Add explanatory header comment
    • Net: 5 lines
    • Safety boundary: No functional change
  • PRA-20 delete (src/lib/sandbox/privileged-exec.ts:22): SANITIZED_PRIVILEGED_ENV const (18 entries)
    • Replacement: Dynamic generation from imported canonical policy
    • Net: -20 lines
    • Safety boundary: Must cover all canonical process-control names; pin check verifies parity
  • PRA-21 shrink (test/local-credential-helper-pin.test.ts:1): Missing documentation comment
    • Replacement: Add explanatory header comment
    • Net: 5 lines
    • Safety boundary: No functional change
  • PRA-22 shrink (scripts/security-standalone.ts:1): All duplicated predicate definitions across 4 files
    • Replacement: Single scripts/security-standalone.ts module
    • Net: -500 lines
    • Safety boundary: All security predicates must remain functionally identical; pin check enforces parity
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — rejects Node.js < 22.16 with clear error — mock process.version in test/local-credential-helper.test.ts. Helper implementation has high unit/integration test coverage (30+ test cases). Runtime validation recommended for docs/starter prompt changes (pin check already provides this in CI). Missing tests for Windows paths, Node version check, and NEMOCLAW_* prefix blocking.
  • PRA-T2 Runtime validation — accepts Windows absolute executable path — path.resolve('C:', 'Windows', 'System32', 'cmd.exe') in test/local-credential-helper.test.ts. Helper implementation has high unit/integration test coverage (30+ test cases). Runtime validation recommended for docs/starter prompt changes (pin check already provides this in CI). Missing tests for Windows paths, Node version check, and NEMOCLAW_* prefix blocking.
  • PRA-T3 Runtime validation — blocks NEMOCLAW_* prefix in isolated profile — ambient NEMOCLAW_CUSTOM_VAR stripped in test/local-credential-helper.test.ts. Helper implementation has high unit/integration test coverage (30+ test cases). Runtime validation recommended for docs/starter prompt changes (pin check already provides this in CI). Missing tests for Windows paths, Node version check, and NEMOCLAW_* prefix blocking.
  • PRA-T4 Runtime validation — pin check extracts createIsolatedChildEnvironment source parity — TS compiler API extraction in test/local-credential-helper-pin.test.ts. Helper implementation has high unit/integration test coverage (30+ test cases). Runtime validation recommended for docs/starter prompt changes (pin check already provides this in CI). Missing tests for Windows paths, Node version check, and NEMOCLAW_* prefix blocking.
  • PRA-T5 Runtime validation — privileged-exec.ts SANITIZED_PRIVILEGED_ENV parity with canonical — dynamic generation test in test/local-credential-helper-pin.test.ts. Helper implementation has high unit/integration test coverage (30+ test cases). Runtime validation recommended for docs/starter prompt changes (pin check already provides this in CI). Missing tests for Windows paths, Node version check, and NEMOCLAW_* prefix blocking.
  • PRA-T6 Helper requires Node.js 22.16+ but has no runtime version check — Add runtime version check at top of main(): parse process.version, verify >= 22.16.0, exit with clear error message if not. Add test 'rejects Node.js < 22.16 with clear error' in test/local-credential-helper.test.ts (mock process.version).
  • PRA-T7 Pin check missing sanitizeInheritedChildEnvironment source-text parity test — Add test in test/local-credential-helper-pin.test.ts that extracts the function from helper using TS compiler API (like extractCredentialPattern) and compares against expected source snapshot. Once security-standalone.ts exists, verify helper imports it.
  • PRA-T8 Missing test for Windows absolute executable path — Add test case 'accepts Windows absolute executable path' using path.resolve('C:', 'Windows', 'System32', 'cmd.exe') or similar. Verify helper spawns successfully with such path.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: sanitizeInheritedChildEnvironment in helper only

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Pin check verifying parity across all 4 locations
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Function exists only in helper.mts:235 and form HTML embedded script. Pin check verifies parity but has no canonical source to reference.

PRA-2 Resolve/justify — Source-of-truth review needed: XDG_CONFIG_DIRS/XDG_DATA_DIRS as single paths

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test verifying format matches spec (colon-separated single entry)
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Lines 278-279 in helper.mts: XDG_CONFIG_DIRS: path.join(root, 'config-dirs'), XDG_DATA_DIRS: path.join(root, 'data-dirs') — single paths not lists.

PRA-3 Resolve/justify — Source-of-truth review needed: NEMOCLAW_* exact names only, no prefix rule

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Pin check verifying prefix rule in both locations
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: isForbiddenChildEnvName has 8 prefix rules (OPENSHELL_, NPM_CONFIG_, PIP_, LD_, DYLD_, BASH_FUNC_, GIT_CONFIG_, GIT_TRACE) but not NEMOCLAW_. PR description claims 'Deny ambient NEMOCLAW_*' but prefix rule missing.

PRA-4 Resolve/justify — Source-of-truth review needed: Execution root in repo scripts/ directory

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test verifying temp location is under os.tmpdir()
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Line 258: root = mkdtempSync(path.join(path.dirname(fileURLToPath(import.meta.url)), '.credential-child-')) — uses repo scripts/ directory.

PRA-5 Required — No canonical security-standalone module for duplicated security predicates

  • Location: scripts/local-credential-helper.mts:147
  • Category: architecture
  • Problem: The security-critical predicates (CREDENTIAL_SHAPED_NAME_PATTERN, PROCESS_CONTROL_ENV_NAMES, isCredentialShapedName, isProcessControlEnvName, sanitizeInheritedChildEnvironment) are duplicated across credential-env.ts, process-control-env.ts, helper.mts, and form HTML. No scripts/security-standalone.ts module exists as single source of truth.
  • Impact: Future divergence risk: changes to canonical policy may not propagate to helper/form, creating security gaps. Pin check enforces parity but cannot prevent drift if canonical source doesn't exist.
  • Required action: Create scripts/security-standalone.ts exporting CREDENTIAL_SHAPED_NAME_PATTERN, PROCESS_CONTROL_ENV_NAMES, isCredentialShapedName, isProcessControlEnvName, and sanitizeInheritedChildEnvironment (renamed to createIsolatedChildEnvironment). Update helper to import with --experimental-strip-types. Update form HTML to fetch module and eval (or generate embedded copy from module at build). Update pin check to verify module is source of truth.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check if scripts/security-standalone.ts exists and is imported by helper.mts and form HTML
  • Missing regression test: Pin check test verifying helper imports security-standalone.ts and form HTML embeds generated copy
  • Done when: The required change is committed and verification passes: Check if scripts/security-standalone.ts exists and is imported by helper.mts and form HTML.
  • Evidence: Helper and form embed identical literal copies of credential pattern, process-control names, and predicates. Pin check verifies parity but has no canonical module to reference.

PRA-6 Required — SANITIZED_PRIVILEGED_ENV hardcoded list diverges from canonical process-control policy

  • Location: src/lib/sandbox/privileged-exec.ts:56
  • Category: security
  • Problem: privileged-exec.ts defines SANITIZED_PRIVILEGED_ENV as 18 hardcoded entries. The canonical process-control-env.ts defines 100+ exact names plus prefix rules (BASH_FUNC_, LD_, DYLD_, GIT_CONFIG_, GIT_TRACE, NPM_CONFIG_, OPENSHELL_, PIP_). Privileged sandbox exec could inherit process-control variables that the credential helper blocks.
  • Impact: Security gap: privileged container exec may leak proxy settings, loader hooks, config paths, or Git/SSL credentials that are stripped in credential handoff.
  • Required action: Update privileged-exec.ts to import isProcessControlEnvName from process-control-env.ts and generate sanitization list dynamically at runtime. Add parity test in pin check verifying every canonical process-control name is covered.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read src/lib/sandbox/privileged-exec.ts:56-69 and verify it imports from process-control-env.ts
  • Missing regression test: Test in test/local-credential-helper-pin.test.ts verifying privileged-exec.ts sanitization list equals canonical policy
  • Done when: The required change is committed and verification passes: Read src/lib/sandbox/privileged-exec.ts:56-69 and verify it imports from process-control-env.ts.
  • Evidence: SANITIZED_PRIVILEGED_ENV has 18 entries vs 100+ in PROCESS_CONTROL_ENV_NAMES plus 8 prefix rules. No import from canonical module.

PRA-7 Required — Helper requires Node.js 22.16+ but has no runtime version check

  • Location: scripts/local-credential-helper.mts:1010
  • Category: tests
  • Problem: The helper uses --experimental-strip-types which requires Node.js >= 22.16.0. If run on older Node, it fails with cryptic syntax errors instead of a clear message.
  • Impact: Poor developer experience: coding agents running helper on unsupported Node versions get unactionable errors, potentially falling back to insecure credential handling.
  • Required action: Add runtime version check at top of main(): parse process.version, verify >= 22.16.0, exit with clear error message if not. Add test 'rejects Node.js < 22.16 with clear error' in test/local-credential-helper.test.ts (mock process.version).
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check main() function in helper.mts for process.version check before any other logic
  • Missing regression test: Test mocking process.version to verify clear error on old Node
  • Done when: The required change is committed and verification passes: Check main() function in helper.mts for process.version check before any other logic.
  • Evidence: main() function at line 1010 starts parsing CLI args immediately without version check. package.json engines specifies >=22.16.0 but no runtime enforcement.

PRA-8 Required — NEMOCLAW_* prefix not blocked in forbidden environment names

  • Location: scripts/local-credential-helper.mts:223
  • Category: security
  • Problem: isForbiddenChildEnvName has prefix rules for OPENSHELL_, NPM_CONFIG_, PIP_, LD_, DYLD_, BASH_FUNC_, GIT_CONFIG_, GIT_TRACE but NOT NEMOCLAW_*. FORBIDDEN_CHILD_ENV_NAMES contains only exact NEMOCLAW_* names. An ambient NEMOCLAW_CUSTOM_VAR could leak into the isolated child.
  • Impact: Credential/environment leakage: attacker-controlled ambient NEMOCLAW_* variables could influence child command behavior, bypassing the isolated profile's intent.
  • Required action: Add name.startsWith('NEMOCLAW_') to isForbiddenChildEnvName in helper.mts and isProcessControlEnvName in process-control-env.ts. Update FORBIDDEN_CHILD_ENV_NAMES and PROCESS_CONTROL_ENV_NAMES accordingly. Update pin check.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Search for NEMOCLAW_ prefix in isForbiddenChildEnvName (helper.mts:223) and isProcessControlEnvName (process-control-env.ts:140)
  • Missing regression test: Test verifying NEMOCLAW_* names are blocked in both helper and canonical policy
  • Done when: The required change is committed and verification passes: Search for NEMOCLAW_ prefix in isForbiddenChildEnvName (helper.mts:223) and isProcessControlEnvName (process-control-env.ts:140).
  • Evidence: isForbiddenChildEnvName has 8 prefix rules but not NEMOCLAW_. FORBIDDEN_CHILD_ENV_NAMES has 7 exact NEMOCLAW_* entries. PR description claims 'Deny ambient NEMOCLAW_*' but prefix rule missing.

PRA-9 Required — Isolated execution root created under repo scripts/ directory instead of system temp

  • Location: scripts/local-credential-helper.mts:258
  • Category: security
  • Problem: createPrivateExecutionRoot() uses path.join(path.dirname(fileURLToPath(import.meta.url)), '.credential-child-') which places the private root in the repository checkout. This violates isolation and could leave artifacts in the repo.
  • Impact: Isolation violation: temporary credential-handling artifacts (config, cache, data dirs) created in source tree. Could be committed accidentally or inspected by other processes.
  • Required action: Change createPrivateExecutionRoot to use mkdtempSync(path.join(os.tmpdir(), 'nemoclaw-credential-child-')). Ensure 0o700 permissions on root and all subdirectories (already done at line 262).
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check createPrivateExecutionRoot function for os.tmpdir() usage instead of repo-relative path
  • Missing regression test: Test verifying isolated execution root is under system temp directory (os.tmpdir())
  • Done when: The required change is committed and verification passes: Check createPrivateExecutionRoot function for os.tmpdir() usage instead of repo-relative path.
  • Evidence: Line 258: root = mkdtempSync(path.join(path.dirname(fileURLToPath(import.meta.url)), '.credential-child-')) — uses repo scripts/ directory.

PRA-10 Required — sanitizeInheritedChildEnvironment exists only in standalone helper — no canonical source

  • Location: scripts/local-credential-helper.mts:235
  • Category: architecture
  • Problem: The function sanitizeInheritedChildEnvironment (line 235) is defined only in helper.mts and embedded in form HTML. It returns {} for isolated profile (child gets only profile env + submitted fields) and is not used for account-home. No canonical module exports it.
  • Impact: Duplicated logic across 4 locations; function name misrepresents behavior (returns empty object, doesn't sanitize). Pin check cannot verify function body parity.
  • Required action: Rename to createIsolatedChildEnvironment and move to scripts/security-standalone.ts. Update helper and form to import/use it. Document: 'Returns empty object for isolated profile — child receives only profile environment + submitted fields. For account-home, ambient env is not sanitized (uses accountHomeEnvironment directly).'
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check if scripts/security-standalone.ts exists and exports createIsolatedChildEnvironment
  • Missing regression test: Pin check test extracting createIsolatedChildEnvironment source text and comparing to canonical
  • Done when: The required change is committed and verification passes: Check if scripts/security-standalone.ts exists and exports createIsolatedChildEnvironment.
  • Evidence: Function at line 235 returns {} with comment 'Unknown variables can be tool-specific execution controls. The child gets only the selected profile environment and explicitly submitted fields.' Name suggests sanitization but behavior is isolation.

PRA-11 Resolve/justify — Pin check missing sanitizeInheritedChildEnvironment source-text parity test

  • Location: test/local-credential-helper-pin.test.ts:1
  • Category: tests
  • Problem: The pin check verifies credential pattern, process-control names, and predicates but does not extract and compare the sanitizeInheritedChildEnvironment/createIsolatedChildEnvironment function body.
  • Impact: Function body divergence between helper, form, and future canonical module would not be caught.
  • Recommended action: Add test in test/local-credential-helper-pin.test.ts that extracts the function from helper using TS compiler API (like extractCredentialPattern) and compares against expected source snapshot. Once security-standalone.ts exists, verify helper imports it.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/local-credential-helper-pin.test.ts for extractCredentialPattern pattern; add similar extractIsolatedChildEnvironment function
  • Missing regression test: Pin check test for createIsolatedChildEnvironment source-text parity
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/local-credential-helper-pin.test.ts for extractCredentialPattern pattern; add similar extractIsolatedChildEnvironment function.
  • Evidence: Pin check has extractCredentialPattern, extractEmbeddedFormDigest, extractProcessControlRules, extractStringSet, extractStarterPrompt but no function body extraction for sanitizeInheritedChildEnvironment.

PRA-12 Resolve/justify — Starter prompt missing account-home profile warning about ~/.ssh, ~/.config, ~/.npmrc access

  • Location: docs/_components/StarterPrompt.tsx:102
  • Category: docs
  • Problem: The prompt mentions account-home profile but does not warn that it grants the command access to user configuration directories.
  • Impact: Users may unknowingly grant install/onboard commands access to sensitive config directories when isolated profile would suffice.
  • Recommended action: Add bullet: '⚠️ The account-home profile grants the command access to your user configuration directories (e.g., ~/.ssh, ~/.config, ~/.npmrc). Only use this profile for commands that must persist account state, such as the initial NemoClaw install. Prefer isolated profile for one-shot credential capture.'
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search StarterPrompt.tsx for account-home warning about ~/.ssh, ~/.config, ~/.npmrc
  • Missing regression test: Doc test verifying account-home warning text present in STARTER_PROMPT constant
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search StarterPrompt.tsx for account-home warning about ~/.ssh, ~/.config, ~/.npmrc.
  • Evidence: StarterPrompt.tsx lines 100-110 describe account-home profile but no warning about config directory access.

PRA-13 Resolve/justify — clearCredentialReferences called after spawn — best-effort only, strings not zeroed

  • Location: scripts/local-credential-helper.mts:860
  • Category: correctness
  • Problem: clearCredentialReferences is called in launchApprovedCommand after child.spawn(). JavaScript strings and child process environment copies cannot be reliably zeroed. The call ordering means credentials exist in childEnv during spawn.
  • Impact: Credential exposure window: secrets exist in memory during spawn; child process receives copied env. Cannot be fully mitigated in JS but ordering can minimize window.
  • Recommended action: Move clearCredentialReferences calls before spawn for values object (childEnv is newly created). Add explicit comment: 'Best-effort exposure minimization only. JavaScript strings and child process environment copies cannot be reliably zeroed.'
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check launchApprovedCommand for clearCredentialReferences call ordering relative to spawn()
  • Missing regression test: N/A — design limitation, document in code
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check launchApprovedCommand for clearCredentialReferences call ordering relative to spawn().
  • Evidence: Line 860: clearCredentialReferences called after spawn(). Function comment at line 845 acknowledges 'JavaScript strings and a spawned child's copied environment cannot be reliably zeroed.'

PRA-14 Resolve/justify — Missing test for Windows absolute executable path

  • Location: test/local-credential-helper.test.ts:615
  • Category: tests
  • Problem: Tests reject non-absolute paths but do not test Windows absolute paths like C:\Windows\System32\cmd.exe.
  • Impact: Windows users may encounter false rejection if path validation doesn't handle drive-letter paths correctly.
  • Recommended action: Add test case 'accepts Windows absolute executable path' using path.resolve('C:', 'Windows', 'System32', 'cmd.exe') or similar. Verify helper spawns successfully with such path.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search test file for Windows absolute path test; add new it() block
  • Missing regression test: Test accepting Windows absolute executable path
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search test file for Windows absolute path test; add new it() block.
  • Evidence: Test at line 615 checks non-absolute rejection but no positive test for Windows absolute paths.

PRA-15 Resolve/justify — XDG_CONFIG_DIRS and XDG_DATA_DIRS set as single paths, not colon-separated lists per XDG spec

  • Location: scripts/local-credential-helper.mts:278
  • Category: correctness
  • Problem: privateExecutionEnvironment sets XDG_CONFIG_DIRS and XDG_DATA_DIRS as single paths. XDG Base Directory Specification requires colon-separated lists. Setting as single paths may break applications expecting list format.
  • Impact: Applications reading XDG_CONFIG_DIRS/XDG_DATA_DIRS may not find config/data directories if they expect colon-separated format.
  • Recommended action: Set XDG_CONFIG_DIRS and XDG_DATA_DIRS to colon-separated lists with the private directory as the only entry: path.join(root, 'config-dirs') + ':' + path.join(root, 'config-dirs').
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check privateExecutionEnvironment function for XDG_CONFIG_DIRS and XDG_DATA_DIRS values
  • Missing regression test: Test verifying XDG vars format matches specification (colon-separated single entry)
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check privateExecutionEnvironment function for XDG_CONFIG_DIRS and XDG_DATA_DIRS values.
  • Evidence: Lines 278-279: XDG_CONFIG_DIRS: path.join(root, 'config-dirs'), XDG_DATA_DIRS: path.join(root, 'data-dirs') — single paths, not lists.

PRA-16 Resolve/justify — sanitizeInheritedChildEnvironment misnamed — returns {} for isolated, not used for account-home

  • Location: scripts/local-credential-helper.mts:235
  • Category: security
  • Problem: Function name suggests it sanitizes an inherited environment, but for isolated profile it returns empty object (child gets only profile env + submitted fields). For account-home, ambient env is not sanitized (uses accountHomeEnvironment directly). This behavior divergence is not documented.
  • Impact: Misleading API: callers expecting sanitization get isolation. Future maintainers may misuse.
  • Recommended action: Rename to createIsolatedChildEnvironment to match behavior. Add comment: 'Returns empty object for isolated profile — child receives only profile environment + submitted fields. For account-home, ambient env is not sanitized (uses accountHomeEnvironment directly).'
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check function name and comment for sanitizeInheritedChildEnvironment at line 235
  • Missing regression test: N/A — code clarity improvement
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check function name and comment for sanitizeInheritedChildEnvironment at line 235.
  • Evidence: Function returns {} unconditionally. accountHomeEnvironment used directly in createExecutionContext for account-home profile.

PRA-17 Improvement — CSP hashes computed at runtime via buildCredentialFormCsp() instead of precomputed at build time

  • Location: scripts/local-credential-helper.mts:691
  • Category: architecture
  • Problem: buildCredentialFormCsp() parses form HTML at startup to extract script/style tags and compute SHA-256 hashes for CSP. This adds runtime overhead and requires form HTML to be parsed.
  • Impact: Minor startup latency; form HTML must be readable at runtime (already required).
  • Suggested action: Add build step (e.g., in package.json prepare or scripts/checks) to compute CSP hashes from form HTML and embed as constants EXPECTED_FORM_SCRIPT_CSP_HASH and EXPECTED_FORM_STYLE_CSP_HASH in helper.mts. Remove buildCredentialFormCsp runtime call at line 787.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check if CSP hashes are precomputed constants in helper.mts
  • Missing regression test: N/A — build-time optimization
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 691: export function buildCredentialFormCsp(formBytes: Buffer): string parses HTML. Line 787: const formCsp = buildCredentialFormCsp(formBytes).

PRA-18 Improvement — Non-absolute executable test uses toContain instead of exact error message

  • Location: test/local-credential-helper.test.ts:615
  • Category: tests
  • Problem: Test checks for 'approved command executable must use an absolute path' as substring rather than exact match.
  • Impact: Test could pass with different error message containing same substring.
  • Suggested action: Update test to use toBe() with exact error message string: 'The approved command executable must use an absolute path'.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test for non-absolute executable error message matching
  • Missing regression test: N/A — test precision improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Test uses expect(captured.output()).toContain('approved command executable must use an absolute path') instead of exact match.

PRA-19 Improvement — Pin check uses TypeScript compiler API — document as intentional design

  • Location: scripts/checks/local-credential-helper-pin.ts:1
  • Category: architecture
  • Problem: The pin check uses ts.createSourceFile and AST traversal for structural source-text parity verification. This sophisticated approach should be documented.
  • Impact: Future maintainers may not understand why TS compiler API is used instead of simpler string/regex matching.
  • Suggested action: Add comment in pin check header: 'Uses TypeScript compiler API for structural source-text parity verification. This ignores comments, formatting, and string-embedded decoys while verifying executable code structure.'
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check pin check file header for documentation comment
  • Missing regression test: N/A — documentation improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: No header comment explaining TS compiler API usage rationale.

PRA-20 Improvement — Refactor privileged-exec.ts to import canonical process-control policy

  • Location: src/lib/sandbox/privileged-exec.ts:22
  • Category: security
  • Problem: After parity test added, privileged-exec.ts should import from process-control-env.ts instead of hardcoded list. This is the fix for PRA-6 framed as improvement.
  • Impact: Eliminates maintenance burden and divergence risk.
  • Suggested action: Replace SANITIZED_PRIVILEGED_ENV const with dynamic generation using imported isProcessControlEnvName from process-control-env.ts. Filter process.env at runtime or generate --env args dynamically.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check privileged-exec.ts for import from process-control-env.ts
  • Missing regression test: Parity test in pin check
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: SANITIZED_PRIVILEGED_ENV at line 22 is hardcoded. process-control-env.ts exports isProcessControlEnvName.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

Comment thread scripts/local-credential-helper.mts Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
test/local-credential-helper.test.ts (1)

300-300: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test titles are missing the required issue-reference suffix.

None of these titles carry a trailing (#1234) reference, unlike the sibling test/starter-prompt-docs.test.ts, which consistently tags its new tests with (#5048) for the same feature. For consistency and traceability, add the local issue reference suffix to each title here too.

Also applies to: 309-309, 342-342, 404-404, 494-494, 528-528

🤖 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/local-credential-helper.test.ts` at line 300, Update the affected test
titles in local-credential-helper.test.ts so each new case includes the required
trailing issue-reference suffix, matching the naming convention used in
starter-prompt-docs.test.ts. Specifically, adjust the titles in the test cases
around the `rejects $label before listening` and the other referenced
`it`/parameterized test declarations so they end with the local issue tag in the
same `(`#1234`)` style for consistency and traceability.

Source: Coding guidelines

scripts/checks/local-credential-helper-pin.ts (1)

118-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extend the parity check to cover the prefix rules. verifyFieldSafetyRules only compares the literal env-name sets, so the LD_, DYLD_, and GIT_CONFIG* branches in isForbiddenChildEnvName/isProcessControlFieldName can drift without this guardrail noticing. Consider deriving both checks from one shared source, or compare the full predicate logic instead of just the set contents.

🤖 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 `@scripts/checks/local-credential-helper-pin.ts` around lines 118 - 138, The
parity check in verifyFieldSafetyRules only compares the extracted env-name
sets, so it can miss drift in the prefix-based branches handled by
isForbiddenChildEnvName and isProcessControlFieldName. Update the check to
validate the full predicate behavior, either by deriving both rules from one
shared source or by comparing the complete logic for helper and form instead of
only FORBIDDEN_CHILD_ENV_NAMES and PROCESS_CONTROL_FIELD_NAMES. Make sure the
new guard covers the LD_, DYLD_, and GIT_CONFIG* cases as well as the literal
names.

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.

Inline comments:
In `@scripts/local-credential-helper.mts`:
- Around line 444-449: `scrubValues` and `scrubBuffers` only clear object
references, which does not באמת wipe secret data because the values come from
immutable strings created by `JSON.parse` and the serialized body text also
remains in memory. Update the credential capture flow around `scrubValues`,
`scrubBuffers`, and the body parsing path to either keep secrets in `Buffer`
form end-to-end or explicitly document this as a known limitation if a true wipe
is not feasible. If you keep the current approach, avoid implying that
deleting/reassigning parsed string fields is a secure scrub, and rely on the raw
`body.fill(0)` wipe as the only actual memory clearing step.

In `@test/local-credential-helper.test.ts`:
- Around line 1-561: The new test file is tripping the test-conditionals:scan
guardrail because it adds several if statements; reduce or consolidate branching
in local-credential-helper.test.ts, especially in helpers like waitForReadiness,
terminate, request, and captureChild where the new exit-code and body/header
checks were added. Refactor repeated conditional logic into helper functions or
predicate-based expressions where possible, or coordinate a baseline update if
the added branching is truly required for the integration-test behavior.

---

Nitpick comments:
In `@scripts/checks/local-credential-helper-pin.ts`:
- Around line 118-138: The parity check in verifyFieldSafetyRules only compares
the extracted env-name sets, so it can miss drift in the prefix-based branches
handled by isForbiddenChildEnvName and isProcessControlFieldName. Update the
check to validate the full predicate behavior, either by deriving both rules
from one shared source or by comparing the complete logic for helper and form
instead of only FORBIDDEN_CHILD_ENV_NAMES and PROCESS_CONTROL_FIELD_NAMES. Make
sure the new guard covers the LD_, DYLD_, and GIT_CONFIG* cases as well as the
literal names.

In `@test/local-credential-helper.test.ts`:
- Line 300: Update the affected test titles in local-credential-helper.test.ts
so each new case includes the required trailing issue-reference suffix, matching
the naming convention used in starter-prompt-docs.test.ts. Specifically, adjust
the titles in the test cases around the `rejects $label before listening` and
the other referenced `it`/parameterized test declarations so they end with the
local issue tag in the same `(`#1234`)` style for consistency and traceability.
🪄 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: 1650305a-c0da-4c07-b5f8-403989bd732d

📥 Commits

Reviewing files that changed from the base of the PR and between be3e7cc and e4b81c0.

📒 Files selected for processing (13)
  • docs/_components/StarterPrompt.tsx
  • docs/get-started/quickstart-hermes.mdx
  • docs/get-started/quickstart-langchain-deepagents-code.mdx
  • docs/get-started/quickstart.mdx
  • docs/index.mdx
  • docs/resources/agent-skills.mdx
  • docs/resources/local-credential-form.html
  • package.json
  • scripts/checks/local-credential-helper-pin.ts
  • scripts/checks/run.ts
  • scripts/local-credential-helper.mts
  • test/local-credential-helper.test.ts
  • test/starter-prompt-docs.test.ts

Comment thread scripts/local-credential-helper.mts Outdated
Comment thread test/local-credential-helper.test.ts
cv added 4 commits July 7, 2026 20:32
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Comment thread scripts/checks/local-credential-helper-pin.ts Fixed
cv added 3 commits July 7, 2026 21:17
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv

cv commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@test/local-credential-helper.test.ts`:
- Around line 520-550: The Bash hijack test still shells out to curl, which
makes it depend on a host binary unnecessarily. Update the approved command in
the inherited-function test to use a shell builtin such as echo while keeping
the same BASH_FUNC_curl%% and ATTACK_MARKER setup in startHelper, so the
sanitizer path is still exercised without relying on curl being present.
🪄 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: f5da6dcf-82ed-4fb0-bebb-32234844ea66

📥 Commits

Reviewing files that changed from the base of the PR and between e84eda7 and 0c3f930.

📒 Files selected for processing (7)
  • docs/_components/StarterPrompt.tsx
  • docs/resources/local-credential-form.html
  • scripts/checks/local-credential-helper-pin.ts
  • scripts/local-credential-helper.mts
  • test/local-credential-helper-pin.test.ts
  • test/local-credential-helper.test.ts
  • test/starter-prompt-docs.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • docs/_components/StarterPrompt.tsx
  • test/local-credential-helper-pin.test.ts
  • scripts/checks/local-credential-helper-pin.ts
  • docs/resources/local-credential-form.html
  • scripts/local-credential-helper.mts
  • test/starter-prompt-docs.test.ts

Comment thread test/local-credential-helper.test.ts
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 28920925588
Workflow ref: codex/harden-local-credential-helper
Requested targets: (default — all supported)
Requested jobs: credential-sanitization,cloud-onboard
Summary: 2 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
cloud-onboard ✅ success
credential-sanitization ✅ success

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
scripts/checks/local-credential-helper-pin.ts (1)

316-385: 🔒 Security & Privacy | 🔵 Trivial

Parity check syncs artifacts but doesn't guard against coordinated weakening.

verifyFieldSafetySourceParity ensures the canonical, helper, and form copies stay in sync (its stated purpose per the module comment), but a PR that removes an entry from PROCESS_CONTROL_ENV_NAMES/CREDENTIAL_SHAPED_NAME_PATTERN and mirrors the same removal in the helper/form would still pass this check — there's no independent floor enforcing a minimum required deny-set. This is likely acceptable since human review + the digest pin still gates changes to the reviewed artifacts, but worth being aware of given the check's role as the primary automated safety net here.

As per path instructions, "A ratchet must be monotonic and must not be weakenable by the PR it checks."

🤖 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 `@scripts/checks/local-credential-helper-pin.ts` around lines 316 - 385,
`verifyFieldSafetySourceParity` only checks that the canonical, helper, and form
artifacts match each other, so a coordinated removal can still pass. Add an
independent monotonic floor in this check by validating the parsed sets/rules
against a fixed required deny-list or pinned baseline for
`PROCESS_CONTROL_ENV_NAMES`, `CREDENTIAL_SHAPED_NAME_PATTERN`, and the
corresponding helper/form symbols. Keep the existing parity comparisons, but
extend the `failures` accumulation so any weakening of the security policy is
rejected even when all three copies are edited together.

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 `@scripts/checks/local-credential-helper-pin.ts`:
- Around line 316-385: `verifyFieldSafetySourceParity` only checks that the
canonical, helper, and form artifacts match each other, so a coordinated removal
can still pass. Add an independent monotonic floor in this check by validating
the parsed sets/rules against a fixed required deny-list or pinned baseline for
`PROCESS_CONTROL_ENV_NAMES`, `CREDENTIAL_SHAPED_NAME_PATTERN`, and the
corresponding helper/form symbols. Keep the existing parity comparisons, but
extend the `failures` accumulation so any weakening of the security policy is
rejected even when all three copies are edited together.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 044df307-7722-409a-8565-4bc3b8d7f099

📥 Commits

Reviewing files that changed from the base of the PR and between 0c3f930 and a413ba3.

📒 Files selected for processing (13)
  • docs/_components/StarterPrompt.tsx
  • docs/resources/local-credential-form.html
  • scripts/checks/local-credential-helper-pin.ts
  • scripts/local-credential-helper.mts
  • src/lib/adapters/http/curl-args.test.ts
  • src/lib/adapters/http/probe.test.ts
  • src/lib/security/credential-env.test.ts
  • src/lib/security/credential-env.ts
  • src/lib/security/process-control-env.test.ts
  • src/lib/security/process-control-env.ts
  • test/local-credential-helper-pin.test.ts
  • test/local-credential-helper.test.ts
  • test/starter-prompt-docs.test.ts
✅ Files skipped from review due to trivial changes (1)
  • src/lib/security/process-control-env.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • docs/_components/StarterPrompt.tsx
  • docs/resources/local-credential-form.html
  • test/starter-prompt-docs.test.ts

@cv

cv commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

CodeRabbit final-review nitpick acknowledged at current head. No code change is warranted:

verifyFieldSafetySourceParity is intentionally a cross-artifact parity and immutable-pin guard, not a monotonic policy ratchet. A fixed minimum list stored in another PR-controlled repository file could be weakened by the same PR and would duplicate the canonical policy. A true monotonic comparison would require trusted-base workflow logic; absent a concrete unsafe removal, that is not justified here. Coordinated policy changes remain gated by reviewed/Verified artifact commits, immutable digest repins, behavior tests, CodeQL, and maintainer review.

The final CodeRabbit review contains no inline findings, and GraphQL reports zero unresolved review threads.

@cv
cv requested review from ericksoa and miyoungc July 8, 2026 06:08
@ericksoa ericksoa added v0.0.78 Release target and removed v0.0.77 Release target labels Jul 8, 2026
@miyoungc

miyoungc commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Review: keep the security helper, but slim the starter prompt

Thanks for the thorough hardening work here. The checked-in, integrity-pinned, one-shot helper is a reasonable answer to a real threat (a coding agent running installers with live secrets, ambient-env hijacking, and agent-generated helper variance). I don't want to lose that.

My concern is scope, specifically where this lands in the starter prompt. That prompt is titled "Instructions for a Non-Technical User" and is built around plain language and one-question-at-a-time. The "Handle Tokens Securely and Visually" section roughly doubled and now instructs the agent about execution profiles, account-home reconstruction, frozen argv, process-control env names, NPM_CONFIG_*/PIP_*, and building a curl | bash script that copies each credential into a non-exported shell variable and passes --disable to curl. That's a security-appliance spec, not a starter prompt.

Concrete worries:

  1. It works against docs: stabilize starter prompt credential capture #6423's own goal. docs: stabilize starter prompt credential capture #6423 existed to reduce setup variability for medium-sized models. Longer, denser agent instructions are more brittle for exactly those models. The likely failure mode is a mid-tier agent half-following a 19-bullet protocol, which is worse than a short, reliable one.
  2. Maintenance/parity drift is already real. The standalone helper duplicates env-strip policy that also lives in src/lib/security/credential-env.ts and src/lib/sandbox/privileged-exec.ts, and the prompt pins two SHA-256 digests that must be re-bumped on every byte change. The Nemotron advisor is currently needs_rework on exactly this (PRA-6/7/9/11: policy divergence, helper-only sanitizer).
  3. Advisors split. GPT-5.5 says merge_as_is; Nemotron says do-not-merge with 6 required items. That split is a signal the scope is contentious.

Suggested direction (decouple, don't delete)

  • Keep the copy-paste starter prompt short and plain: a few sentences that say use the checked-in credential helper, verify its digest, never put secrets in chat or argv, confirm-then-run. Keep the pinned helper/form URLs + digests so the trust boundary and pin check stay intact.
  • Move the detailed mechanics (execution profiles, argv construction, curl | bash non-exported-variable pattern, account-home, Windows .cmd) into a dedicated docs page the agent can fetch when it needs them.
  • Collapse the env-strip policy to one source of truth so the helper can't silently diverge from credential-env.ts (generate the standalone copy in the pin check rather than hand-maintaining it), which also resolves the Nemotron parity findings.

@miyoungc

miyoungc commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Following up on my earlier review: I opened a stacked PR that slims the starter-prompt credential section along the lines I suggested, so we can compare side by side.

It keeps the hardened helper, form, and pins from this PR unchanged, and only slims the copy-paste starter prompt: pinned trust boundary + one canonical --execution-profile isolated recipe + core safety rules, dropping the NPM_CONFIG_*/PIP_* enumeration, Windows .cmd handling, and the curl | bash wrapper synthesis (install-time secrets now prefer the installer's own prompt). Targeted tests and the pin check pass.

It's meant as a discussion starter for the UX split, not a required change to this PR. Happy to iterate or fold it in however you prefer.

miyoungc and others added 3 commits July 8, 2026 11:28
<!-- markdownlint-disable MD041 -->
## Summary

Stacked on top of #6439. Keeps the hardened credential helper and form
from that PR, but slims the copy-paste starter prompt so the default
"non-technical user" onboarding path stays easy. The prompt no longer
asks the coding agent to synthesize execution-profile prose, per-tool
env-strip reasoning, or a `curl | bash` credential wrapper.

## Related Issue

Follow-up to #6439. Merge #6439 first, then this on top.

## Changes

- Slim the "Handle Tokens Securely and Visually" section of
`STARTER_PROMPT` from ~18 dense bullets to 10, keeping the pinned
helper/form URLs and SHA-256 digests, one canonical `--execution-profile
isolated` recipe, and the core safety rules (verify digests, one-shot
confirm-then-run, redacted preview, no secrets in chat or argv,
exposure-minimization).
- Drop the prose the agent had to assemble itself: the
`NPM_CONFIG_*`/`PIP_*` env-strip enumeration, Windows `.cmd` handling,
and the `bash -c` non-exported-variable `curl | bash` wrapper. Compress
`account-home` to one line and prefer the installer's own secure
credential prompt for install-time secrets.
- Update `test/starter-prompt-docs.test.ts` to match, and add
`not.toContain` regression guards so the wrapper synthesis prose cannot
creep back into the copied prompt.

The hardened helper still enforces loopback-only access, one-shot
submission, absolute executables, and ambient env stripping, so the
slimmer prompt keeps the same security posture with far less procedural
reasoning for the agent. Helper and form bytes are unchanged, so the
pinned digests and commit stay valid.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: prompt-only slimming;
no change to helper/form bytes or their enforcement,
`scripts/checks/local-credential-helper-pin.ts` passes, and this is
intended to be reviewed on top of #6439.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set — `npx
vitest run --project integration test/starter-prompt-docs.test.ts
test/local-credential-helper-pin.test.ts` passed 42/42, and `node
--experimental-strip-types
scripts/checks/local-credential-helper-pin.ts` reported pins immutable
and current.
- [ ] Applicable broad gate passed — not run; narrow starter-prompt and
single-test change covered by the targeted tests and the pin check
above.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — not
run; change is prose inside the existing `STARTER_PROMPT` template
literal, validated by the TS-AST pin check and starter-prompt tests.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security blocker at exact head bdf336052f5fbb5b546c20ce7d4140dd859eac89: sanitizeInheritedChildEnvironment is denylist-based and deliberately preserves arbitrary unrecognized ambient variables (the current unit test enshrines this as SAFE_SETTING). That leaves tool-specific execution controls available to the approved child.

A concrete exploit is GIT_TEMPLATE_DIR, which is not rejected. With a submitted credential and an otherwise approved absolute argv such as /usr/bin/git clone <source> <dest>, an ambient GIT_TEMPLATE_DIR can supply an executable hooks/post-checkout; Git runs that hook during clone with the submitted credential in its environment. I reproduced this locally and the hook received the fixture secret. The coding agent controls the helper launch environment, while the user approves argv, so this bypasses the exact-command approval boundary the helper is meant to establish. Similar open-ended controls exist across other toolchains (JAVA_HOME, compiler wrappers/selectors, package-manager homes, and so on), so adding only GIT_TEMPLATE_DIR would not close the class.

Please construct the inherited child environment from a narrow, documented positive allowlist of required non-control OS/locale variables (or from no ambient values), then overlay the execution-profile environment and explicitly approved submitted fields. Any required search path should be reconstructed or explicitly approved rather than inherited. Add an integration regression that launches an approved Git operation with a malicious ambient template hook and proves the hook cannot observe the submitted value. Until then, the stated ambient-env hardening guarantee does not hold.

@cjagwani

cjagwani commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

@cv The narrowest fix for the requested-change blocker is to inherit zero ambient entries. launchApprovedCommand already constructs the child environment as controlled executionContext.environment plus the explicitly submitted field values; the sanitized ambient layer is not needed for HOME, cwd, temp/config roots, or approved text fields. An empty inherited layer closes GIT_TEMPLATE_DIR and the unbounded family of tool-specific controls without growing another denylist.

Please replace the current SAFE_SETTING: preserved expectation with an unknown ambient canary that must be absent, and add an integration regression where ambient GIT_TEMPLATE_DIR contains a malicious post-checkout template hook while the approved command is real /usr/bin/git clone; the hook must not run and must not observe the submitted credential. I would not accept a one-name GIT_TEMPLATE_DIR patch because the design would remain bypassable by the next unlisted control variable.

cjagwani added 2 commits July 8, 2026 19:48
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Signed-off-by: cjagwani <cjagwani@nvidia.com>
@cjagwani
cjagwani dismissed their stale review July 8, 2026 19:51

Addressed at e5be9d5: the child now inherits zero ambient entries, and a real Git template-hook regression proves submitted credentials remain unavailable. Independent exact-head review remains required.

@cjagwani

cjagwani commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Maintainer follow-up landed at exact head e5be9d587439330f07e5a9bac86dcff26c6122a3 and closes my ambient-environment blocker without extending the denylist:

  • the approved child now inherits zero ambient entries;
  • its environment is only the selected execution profile plus explicitly submitted fields;
  • a real /usr/bin/git clone regression supplies a malicious ambient GIT_TEMPLATE_DIR / post-checkout hook and proves the clone succeeds while the hook is neither installed nor executed and cannot observe the submitted secret;
  • helper and form raw URLs were repinned to the immutable signed helper commit.

Validation: helper suite 122/122; pin/docs suites 42/42; immutable-pin checker passed; CLI build and Node 22 typecheck passed; size budget, changed-file Biome, and diff hygiene passed. Both new commits have good ED25519 signatures. I dismissed my now-addressed change request, but I contributed the fix and will not self-approve. @miyoungc @ericksoa, please review the new exact head independently.

@cv

cv commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/_components/StarterPrompt.tsx`:
- Line 100: Update the helper description in StarterPrompt so it accurately
states that the child process receives no inherited ambient environment at all,
not just that some credential or process-control variables are stripped. Adjust
the wording around the stateless command helper text to reference
sanitizeInheritedChildEnvironment and describe that only the execution-profile
environment plus explicitly submitted fields are provided, keeping the rest of
the behavior unchanged.
🪄 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: 147a3300-60bf-4697-83a0-48ed5e6f2c61

📥 Commits

Reviewing files that changed from the base of the PR and between a413ba3 and e5be9d5.

📒 Files selected for processing (7)
  • docs/_components/StarterPrompt.tsx
  • docs/get-started/quickstart-langchain-deepagents-code.mdx
  • scripts/local-credential-helper.mts
  • src/lib/adapters/http/curl-args.test.ts
  • src/lib/adapters/http/probe.test.ts
  • test/local-credential-helper.test.ts
  • test/starter-prompt-docs.test.ts
✅ Files skipped from review due to trivial changes (1)
  • docs/get-started/quickstart-langchain-deepagents-code.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/adapters/http/curl-args.test.ts
  • src/lib/adapters/http/probe.test.ts
  • test/starter-prompt-docs.test.ts

- Form: https://raw.githubusercontent.com/NVIDIA/NemoClaw/dd61a307d7ddf7be99de8ff1e2678fb8ef42f8e6/docs/resources/local-credential-form.html (SHA-256 \`5512a256e0ad7c63a26ab82cf4f5924e98652097172ab8a5dc9d9358dd4f6ae8\`)
- Treat the two immutable URL and digest pairs as one reviewed trust boundary. Stop if either verification fails; do not substitute another URL, helper, form, or digest. Put fetched copies in a private temporary directory restricted to the current user.
- The helper requires Node.js 22.16 or newer. If that runtime is unavailable, use a secure local terminal prompt or local app prompt instead; never ask for the value in chat and never fall back to generated code.
- Run the helper with \`--execution-profile isolated\` for stateless commands. Pass one \`--field NAME:type\` per value, then a literal \`--\` and the exact approved argv. The helper serves a one-time \`http://127.0.0.1\` form, accepts a single submission, then runs that argv; it enforces loopback-only access, requires an absolute executable, and strips ambient credential and process-control variables. Never put credentials in argv. For example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Env-hardening wording understates the actual behavior.

The helper now inherits zero ambient environment (sanitizeInheritedChildEnvironment returns {}), so the child receives only the execution-profile environment plus explicitly submitted fields. "strips ambient credential and process-control variables" reads as selective stripping and understates the guarantee this prompt asks non-technical users to trust.

📝 Suggested wording
-it enforces loopback-only access, requires an absolute executable, and strips ambient credential and process-control variables.
+it enforces loopback-only access, requires an absolute executable, and inherits no ambient environment (the child receives only the execution-profile environment and the values I submit).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Run the helper with \`--execution-profile isolated\` for stateless commands. Pass one \`--field NAME:type\` per value, then a literal \`--\` and the exact approved argv. The helper serves a one-time \`http://127.0.0.1\` form, accepts a single submission, then runs that argv; it enforces loopback-only access, requires an absolute executable, and strips ambient credential and process-control variables. Never put credentials in argv. For example:
- Run the helper with `--execution-profile isolated` for stateless commands. Pass one `--field NAME:type` per value, then a literal `--` and the exact approved argv. The helper serves a one-time `http://127.0.0.1` form, accepts a single submission, then runs that argv; it enforces loopback-only access, requires an absolute executable, and inherits no ambient environment (the child receives only the execution-profile environment and the values I submit). Never put credentials in argv. For example:
🤖 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 `@docs/_components/StarterPrompt.tsx` at line 100, Update the helper
description in StarterPrompt so it accurately states that the child process
receives no inherited ambient environment at all, not just that some credential
or process-control variables are stripped. Adjust the wording around the
stateless command helper text to reference sanitizeInheritedChildEnvironment and
describe that only the execution-profile environment plus explicitly submitted
fields are provided, keeping the rest of the behavior unchanged.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cjagwani

cjagwani commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Exact head ec50830c531e9567a9ce2bc809d8bf93fe2f3afe is now fully green. The only delta after the audited credential-isolation commits is cv’s signed deletion of one blank line to restore the 1500-line DCode image-test ceiling; npm run test-size:check passes locally. The security evidence remains 122 helper tests, the real /usr/bin/git clone hostile-template regression proving zero ambient environment leakage, 42 pin/docs tests, pin checker, CLI build, Node 22 typecheck, changed-file Biome, and diff hygiene. Because I authored the final credential-boundary fix, independent exact-head approval from @ericksoa or @miyoungc remains the only gate.

@cv
cv merged commit 4642936 into main Jul 8, 2026
43 checks passed
@cv
cv deleted the codex/harden-local-credential-helper branch July 8, 2026 20:47
@cjagwani cjagwani mentioned this pull request Jul 9, 2026
21 tasks
cv pushed a commit that referenced this pull request Jul 9, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Prepares the user documentation for NemoClaw v0.0.78 by replacing the
unreleased section with release highlights and synchronizing the
affected inference, lifecycle, messaging, and CLI reference pages with
merged behavior.

## Changes

- Publish the v0.0.78 release-notes section with links to the most
specific user guides for each shipped behavior.
- Document authoritative Deep Agents route health, Nemotron Ultra
profile behavior, and Hermes compatible-endpoint context metadata.
- Document forced rebuild recovery after total backup failure and the
ownership-safe tunnel/full-stop behavior.
- Keep command examples and shared agent variants aligned with the
current OpenClaw, Hermes, and Deep Agents interfaces.

Source mapping:

- [#3787](#3787) ->
`docs/about/release-notes.mdx`: Record reliable workspace template
seeding during sandbox startup.
- [#4960](#4960) ->
`docs/about/release-notes.mdx`: Record safer detection of rewritten
OpenClaw gateway processes.
- [#5676](#5676) ->
`docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON
handling.
- [#5857](#5857) ->
`docs/about/release-notes.mdx`: Record synchronization of explicit
OpenClaw main-agent model state.
- [#5929](#5929) ->
`docs/about/release-notes.mdx`: Record copyable SSH port-forward
guidance for remote dashboards.
- [#6068](#6068) ->
`docs/about/release-notes.mdx`: Record custom-image plugin provenance
reconciliation.
- [#6116](#6116) ->
`docs/about/release-notes.mdx`: Record live-loopback dashboard-forward
recovery.
- [#6122](#6122) ->
`docs/about/release-notes.mdx`: Announce validated, round-trippable
policy YAML output.
- [#6211](#6211) ->
`docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`,
`docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild
--force` recovery boundary.
- [#6283](#6283) ->
`docs/about/release-notes.mdx`: Record Hermes WebUI port alignment.
- [#6293](#6293) ->
`docs/inference/switch-inference-providers.mdx`,
`docs/about/release-notes.mdx`: Document compatible-endpoint
context-length probing for Hermes.
- [#6320](#6320) ->
`docs/about/release-notes.mdx`: Record bounded gateway-recovery waits.
- [#6377](#6377) ->
`docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain
rebuild diagnostics and prepared MCP-destroy recovery.
- [#6412](#6412) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document authoritative agent-visible
inference route health.
- [#6421](#6421) ->
`docs/about/release-notes.mdx`: Record the longer quiet-pull window for
managed vLLM images.
- [#6431](#6431) ->
`docs/inference/model-capability-audit.mdx`,
`docs/about/release-notes.mdx`: Document the version-pinned Nemotron
Ultra profile plugin.
- [#6439](#6439) ->
`docs/about/release-notes.mdx`: Summarize the authenticated, pinned
credential-capture helper boundary.
- [#6450](#6450) ->
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document
host-forward cleanup and ownership-safe gateway-port release.
- [#6474](#6474) ->
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/about/release-notes.mdx`: Record composable OpenClaw messaging
runtime loaders.
- [#6475](#6475) ->
`docs/about/release-notes.mdx`: Record removal of the unavailable Kimi
K2.6 production endpoint option.
- [#6480](#6480) ->
`docs/about/release-notes.mdx`: Record stderr routing for the plugin
registration banner.
- [#6481](#6481) ->
`docs/about/release-notes.mdx`: Record post-pull Ollama model discovery
checks.
- [#6482](#6482) ->
`docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon
restart.
- [#6486](#6486) ->
`docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep
Agents auto-approval boundary.
- [#6490](#6490) ->
`docs/about/release-notes.mdx`: Record diagnostics for custom images
missing the managed runtime.
- [#6494](#6494) ->
`docs/inference/model-capability-audit.mdx`,
`docs/about/release-notes.mdx`: Document nonempty tool-call content
preservation and placeholder rejection.
- [#6497](#6497) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document isolated Deep Agents
route-probe output.
- [#6506](#6506) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document observability-preserving
managed route probes.
- [#6508](#6508) ->
`docs/about/release-notes.mdx`: Link the new extension taxonomy and
SDK-readiness reference from the release summary.

Release-source verification: GitHub reports all 29 cited source PRs as
merged with base `main`, and every merge commit is an ancestor of
`origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No
source-mapping mismatches were found.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates

<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: Documentation-only
release-prep changes; `npm run docs` validates variants, routes, and
Fern content.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: Tests
are not applicable to this documentation-only change set.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — exited
0 with zero errors; Fern reported the existing unauthenticated
redirect-check and light-mode contrast warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>

---------

Signed-off-by: cjagwani <cjagwani@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

This follow-up to NVIDIA#6423 replaces prose-generated credential capture with
a checked-in, authenticated one-shot loopback helper and a hardened
local form. It binds the reviewed helper/form bytes to immutable commit
URLs and SHA-256 digests so coding agents fail closed before collecting
secrets. The helper now requires an explicit `isolated` or
`account-home` execution profile so stateless commands use private
config roots while persistent install/onboard commands use only an
explicitly approved absolute working directory and the OS account home.

## Changes

- Add a Node.js 22 helper that validates the approved field schema, form
digest, loopback request boundary, one-shot capability, and exact child
argv without invoking a shell.
- Require explicit `isolated` and `account-home` profiles, with private
per-session config/temp roots for isolated work and reconstructed
account-home roots plus an approved absolute cwd for persistent work.
- Deny ambient `NEMOCLAW_*`, `OPENSHELL_*`, config-root, loader,
package-manager, container, crypto-provider, and other process-control
selectors while permitting only explicitly approved collected fields.
- Harden the local form with a no-network preview/edit/confirm flow,
redacted summaries, strict response locking, and no retry after an
ambiguous submission.
- Pin both reviewed artifacts in the starter prompt and add a repository
check for digest, package, executable-bit, and helper/form safety-rule
parity.
- Update user-facing starter-prompt summaries and add integration
coverage for malformed requests, races, abandoned responses, CSP, form
behavior, and immutable pins.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: independent
nine-category repository security review completed with final PASS after
the CodeQL filesystem-race, CodeRabbit memory-semantics/parity, and
GPT-advisor ambient-environment findings were addressed; the helper/form
boundary was reviewed for secret handling, input validation,
authentication, error behavior, data protection, headers, security
tests, and race safety.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run --project integration test/local-credential-helper.test.ts
test/local-credential-helper-pin.test.ts
test/starter-prompt-docs.test.ts` passed 163/163 tests.
- [x] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: `umask 022 && npm test`
passed 14,323 tests across 1,267 passing files, with 39 expected skips
and 1 todo.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — not a
doc-only PR; the build completed with 0 errors and 2 pre-existing
warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

Advisor-required live E2E: [run
28920925588](https://github.com/NVIDIA/NemoClaw/actions/runs/28920925588)
passed credential-sanitization and cloud-onboard at head
a413ba3.

Additional checks: `npm run checks`, `npm run typecheck:cli`, `npm run
source-shape:check`, `npm run test-size:check`, package dry-run
contents, post-push raw artifact SHA-256 verification, and `git diff
--check` all passed.

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added a stricter “preview then confirm” credential-capture workflow
before any credentialed commands are generated or executed.
* Improved non-interactive guidance to support secure `curl | bash`
execution with approved-command enforcement.
* **Bug Fixes**
* Hardened the local credential form UI and submission behavior with
stronger input validation, loopback-only access, and safer handling of
ambiguous/failed outcomes.
* **Documentation**
* Updated quickstarts and starter prompt text to match the new
approve-then-capture flow.
* **Tests**
* Expanded integration and security tests for the local helper,
integrity pinning, and credential/processing policy checks.
* **Chores**
* Packaged the hardened credential-form asset and added automated
integrity verification.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Co-authored-by: Miyoung Choi <miyoungc@nvidia.com>
Co-authored-by: cjagwani <cjagwani@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Prepares the user documentation for NemoClaw v0.0.78 by replacing the
unreleased section with release highlights and synchronizing the
affected inference, lifecycle, messaging, and CLI reference pages with
merged behavior.

## Changes

- Publish the v0.0.78 release-notes section with links to the most
specific user guides for each shipped behavior.
- Document authoritative Deep Agents route health, Nemotron Ultra
profile behavior, and Hermes compatible-endpoint context metadata.
- Document forced rebuild recovery after total backup failure and the
ownership-safe tunnel/full-stop behavior.
- Keep command examples and shared agent variants aligned with the
current OpenClaw, Hermes, and Deep Agents interfaces.

Source mapping:

- [NVIDIA#3787](NVIDIA#3787) ->
`docs/about/release-notes.mdx`: Record reliable workspace template
seeding during sandbox startup.
- [NVIDIA#4960](NVIDIA#4960) ->
`docs/about/release-notes.mdx`: Record safer detection of rewritten
OpenClaw gateway processes.
- [NVIDIA#5676](NVIDIA#5676) ->
`docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON
handling.
- [NVIDIA#5857](NVIDIA#5857) ->
`docs/about/release-notes.mdx`: Record synchronization of explicit
OpenClaw main-agent model state.
- [NVIDIA#5929](NVIDIA#5929) ->
`docs/about/release-notes.mdx`: Record copyable SSH port-forward
guidance for remote dashboards.
- [NVIDIA#6068](NVIDIA#6068) ->
`docs/about/release-notes.mdx`: Record custom-image plugin provenance
reconciliation.
- [NVIDIA#6116](NVIDIA#6116) ->
`docs/about/release-notes.mdx`: Record live-loopback dashboard-forward
recovery.
- [NVIDIA#6122](NVIDIA#6122) ->
`docs/about/release-notes.mdx`: Announce validated, round-trippable
policy YAML output.
- [NVIDIA#6211](NVIDIA#6211) ->
`docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`,
`docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild
--force` recovery boundary.
- [NVIDIA#6283](NVIDIA#6283) ->
`docs/about/release-notes.mdx`: Record Hermes WebUI port alignment.
- [NVIDIA#6293](NVIDIA#6293) ->
`docs/inference/switch-inference-providers.mdx`,
`docs/about/release-notes.mdx`: Document compatible-endpoint
context-length probing for Hermes.
- [NVIDIA#6320](NVIDIA#6320) ->
`docs/about/release-notes.mdx`: Record bounded gateway-recovery waits.
- [NVIDIA#6377](NVIDIA#6377) ->
`docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain
rebuild diagnostics and prepared MCP-destroy recovery.
- [NVIDIA#6412](NVIDIA#6412) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document authoritative agent-visible
inference route health.
- [NVIDIA#6421](NVIDIA#6421) ->
`docs/about/release-notes.mdx`: Record the longer quiet-pull window for
managed vLLM images.
- [NVIDIA#6431](NVIDIA#6431) ->
`docs/inference/model-capability-audit.mdx`,
`docs/about/release-notes.mdx`: Document the version-pinned Nemotron
Ultra profile plugin.
- [NVIDIA#6439](NVIDIA#6439) ->
`docs/about/release-notes.mdx`: Summarize the authenticated, pinned
credential-capture helper boundary.
- [NVIDIA#6450](NVIDIA#6450) ->
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document
host-forward cleanup and ownership-safe gateway-port release.
- [NVIDIA#6474](NVIDIA#6474) ->
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/about/release-notes.mdx`: Record composable OpenClaw messaging
runtime loaders.
- [NVIDIA#6475](NVIDIA#6475) ->
`docs/about/release-notes.mdx`: Record removal of the unavailable Kimi
K2.6 production endpoint option.
- [NVIDIA#6480](NVIDIA#6480) ->
`docs/about/release-notes.mdx`: Record stderr routing for the plugin
registration banner.
- [NVIDIA#6481](NVIDIA#6481) ->
`docs/about/release-notes.mdx`: Record post-pull Ollama model discovery
checks.
- [NVIDIA#6482](NVIDIA#6482) ->
`docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon
restart.
- [NVIDIA#6486](NVIDIA#6486) ->
`docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep
Agents auto-approval boundary.
- [NVIDIA#6490](NVIDIA#6490) ->
`docs/about/release-notes.mdx`: Record diagnostics for custom images
missing the managed runtime.
- [NVIDIA#6494](NVIDIA#6494) ->
`docs/inference/model-capability-audit.mdx`,
`docs/about/release-notes.mdx`: Document nonempty tool-call content
preservation and placeholder rejection.
- [NVIDIA#6497](NVIDIA#6497) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document isolated Deep Agents
route-probe output.
- [NVIDIA#6506](NVIDIA#6506) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document observability-preserving
managed route probes.
- [NVIDIA#6508](NVIDIA#6508) ->
`docs/about/release-notes.mdx`: Link the new extension taxonomy and
SDK-readiness reference from the release summary.

Release-source verification: GitHub reports all 29 cited source PRs as
merged with base `main`, and every merge commit is an ancestor of
`origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No
source-mapping mismatches were found.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates

<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: Documentation-only
release-prep changes; `npm run docs` validates variants, routes, and
Fern content.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: Tests
are not applicable to this documentation-only change set.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — exited
0 with zero errors; Fern reported the existing unauthenticated
redirect-check and light-mode contrast warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>

---------

Signed-off-by: cjagwani <cjagwani@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation, examples, guides, or docs build area: install Install, setup, prerequisites, or uninstall flow area: security Security controls, permissions, secrets, or hardening bug-fix PR fixes a bug or regression v0.0.78 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants