Skip to content

docs(starter-prompt): slim credential capture guidance#6477

Merged
cv merged 1 commit into
codex/harden-local-credential-helperfrom
slim-starter-prompt-credential
Jul 8, 2026
Merged

docs(starter-prompt): slim credential capture guidance#6477
cv merged 1 commit into
codex/harden-local-credential-helperfrom
slim-starter-prompt-credential

Conversation

@miyoungc

@miyoungc miyoungc commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

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)
  • 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: 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 fix(security): harden local credential capture #6439.
  • 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 — 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.
  • 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 run; change is prose inside the existing STARTER_PROMPT template literal, validated by the TS-AST pin check and starter-prompt tests.
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

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

Reduce the "Handle Tokens Securely and Visually" section of the copy-paste
starter prompt to the pinned helper/form trust boundary, one canonical
isolated-profile recipe, and the core safety rules. Move install-time
account-home and curl | bash mechanics off the non-technical prompt: prefer
the installer's own secure credential prompt and stop asking the agent to
hand-assemble a curl | bash wrapper.

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.

Update the starter-prompt docs test to match and guard against the wrapper
prose creeping back.

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

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e65df4ba-2191-45cb-9365-6368dca046d9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch slim-starter-prompt-credential

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

@miyoungc
miyoungc requested a review from cv July 8, 2026 16:19
@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 in the branch is 96%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File d659aec +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the branch is 76%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File d659aec +/-
src/lib/onboard/preflight.ts 82%
src/lib/state/o...oard-session.ts 82%
src/lib/actions...all/run-plan.ts 81%
src/lib/actions...licy-channel.ts 79%
src/lib/actions...box/snapshot.ts 79%
src/lib/state/sandbox.ts 75%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/policy/index.ts 65%
src/lib/shields/index.ts 61%
src/lib/onboard.ts 28%

Updated July 08, 2026 16:27 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

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/codex/harden-local-credential-helper
Head: HEAD
Confidence: high

Required E2E

  • None. No E2E is required because this is a documentation prompt and associated test update only. Although the prose discusses credential and installer flows, it does not modify runtime credential handling, onboarding/resume state, sandbox lifecycle, network policy, inference routing, or deployment behavior that existing E2E jobs would exercise.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: None
Optional E2E targets: None

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/codex/harden-local-credential-helper
Head: HEAD
Confidence: high

Required E2E targets

  • None. Docs component changes and tests outside test/e2e do not affect the E2E target registry, workflow machinery, fixtures, live support, or target behavior.

Optional E2E targets

  • None.

Relevant changed files

  • None.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Account-home trust boundary underspecified for install-time credentials.
Open items: 0 required · 3 warnings · 2 suggestions · 1 test follow-up
Since last review: 0 prior items resolved · 0 still apply · 5 new items found

Action checklist

  • PRA-1 Resolve or justify: Account-home trust boundary underspecified for install-time credentials in docs/_components/StarterPrompt.tsx:95
  • PRA-2 Resolve or justify: PATH-based executable lookup rejection and stripped-variable taxonomy removed in docs/_components/StarterPrompt.tsx:100
  • PRA-3 Resolve or justify: Windows .cmd/.bat executable guard removed in docs/_components/StarterPrompt.tsx:105
  • PRA-T1 Add or justify test follow-up: Missing positive assertion for new installer secure-prompt guidance
  • PRA-4 In-scope improvement: Missing positive assertion for new installer secure-prompt guidance in test/starter-prompt-docs.test.ts:400
  • PRA-5 In-scope improvement: Exposure-minimization explanation incomplete in docs/_components/StarterPrompt.tsx:110

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify security docs/_components/StarterPrompt.tsx:95 Add a brief inline note: 'account-home trusts the OS account home and the approved working directory; isolated uses a private temp home deleted after the command exits.'
PRA-2 Resolve/justify security docs/_components/StarterPrompt.tsx:100 Restore 'rejects PATH-based executable lookup' and add parenthetical: '(including configuration roots, source/executable selectors, proxy/TLS overrides, NPM_CONFIG_*, PIP_*)'.
PRA-3 Resolve/justify correctness docs/_components/StarterPrompt.tsx:105 Re-add the Windows .cmd/.bat guidance as a one-sentence note.
PRA-4 Improvement tests test/starter-prompt-docs.test.ts:400 Add positive assertion: `expect(promptSource).toContain("prefer the installer's own secure prompt");`
PRA-5 Improvement docs docs/_components/StarterPrompt.tsx:110 Append '(Node.js strings and the child process environment cannot be reliably zeroed)' to the exposure-minimization sentence.
Review findings by urgency: 0 required fixes, 3 items to resolve/justify, 2 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 — Account-home trust boundary underspecified for install-time credentials

  • Location: docs/_components/StarterPrompt.tsx:95
  • Category: security
  • Problem: The slimmed prompt tells agents to use `--execution-profile account-home --cwd <approved-absolute-directory>` for install/onboarding but does not explain why account-home is needed or what trust boundaries it implies (trusts OS account home and approved working directory; isolated uses a private temp home deleted after command exits).
  • Impact: Operators may not understand the security difference between profiles, leading to incorrect profile choice for commands that persist account state.
  • Recommended action: Add a brief inline note: 'account-home trusts the OS account home and the approved working directory; isolated uses a private temp home deleted after the command exits.'
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read docs/_components/StarterPrompt.tsx lines 115-125 and confirm the account-home trust boundary is explained.
  • Missing regression test: Test that the starter prompt contains 'account-home trusts' or equivalent trust-boundary explanation for the account-home profile.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read docs/_components/StarterPrompt.tsx lines 115-125 and confirm the account-home trust boundary is explained.
  • Evidence: Old prompt had multi-line explanation of account-home vs isolated trust boundaries; new prompt compresses to one line without the trust-model explanation.

PRA-2 Resolve/justify — PATH-based executable lookup rejection and stripped-variable taxonomy removed

  • Location: docs/_components/StarterPrompt.tsx:100
  • Category: security
  • Problem: Old prompt explicitly stated the helper 'rejects PATH-based executable lookup and removes recognized ambient credential-shaped and process-control variables, including configuration roots, source/executable selectors, proxy-routing and TLS-trust overrides, and NPM_CONFIG_* and PIP_* package-source settings'. New prompt only says 'requires an absolute executable, and strips ambient credential and process-control variables'.
  • Impact: Agents/operators lose visibility into the specific categories of environment variables the helper strips, reducing ability to diagnose why a needed variable might be missing or to verify the helper's safety model.
  • Recommended action: Restore 'rejects PATH-based executable lookup' and add parenthetical: '(including configuration roots, source/executable selectors, proxy/TLS overrides, NPM_CONFIG_*, PIP_*)'.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search docs/_components/StarterPrompt.tsx for 'PATH-based' and 'configuration roots' to verify presence.
  • Missing regression test: Test that starter prompt mentions PATH-based lookup rejection and lists the stripped variable categories.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search docs/_components/StarterPrompt.tsx for 'PATH-based' and 'configuration roots' to verify presence.
  • Evidence: Diff shows removal of the detailed stripped-variable enumeration from the prompt prose.

PRA-3 Resolve/justify — Windows .cmd/.bat executable guard removed

  • Location: docs/_components/StarterPrompt.tsx:105
  • Category: correctness
  • Problem: Old prompt included: 'On native Windows, do not target a .cmd or .bat file directly; use the absolute cmd.exe path and make its arguments part of the approved argv.' This concrete input-validation rule was removed.
  • Impact: On Windows, agents may incorrectly pass a .cmd/.bat file as the executable, which the helper will reject (requires absolute native executable). The guidance helped prevent this common mistake.
  • Recommended action: Re-add the Windows .cmd/.bat guidance as a one-sentence note.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search docs/_components/StarterPrompt.tsx for '.cmd' or '.bat' to verify presence.
  • Missing regression test: Test that starter prompt contains Windows .cmd/.bat guidance.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search docs/_components/StarterPrompt.tsx for '.cmd' or '.bat' to verify presence.
  • Evidence: Diff shows the Windows-specific bullet was deleted from the prompt.

💡 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-4 Improvement — Missing positive assertion for new installer secure-prompt guidance

  • Location: test/starter-prompt-docs.test.ts:400
  • Category: tests
  • Problem: New negative assertions guard against `curl | bash` wrapper synthesis (`not.toContain` checks), but no positive assertion verifies the new guidance line 'prefer the installer's own secure prompt over routing it through the helper' remains in the prompt.
  • Impact: Future edits could accidentally remove the new installer-guidance line without test failure.
  • Suggested action: Add positive assertion: `expect(promptSource).toContain("prefer the installer's own secure prompt");`
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run `npx vitest run test/starter-prompt-docs.test.ts` and verify the new assertion passes.
  • Missing regression test: Add the positive assertion for installer secure prompt guidance in the 'pins local credential capture' test block.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Test file has 3 `not.toContain` regression guards but no matching positive assertion for the new guidance.

PRA-5 Improvement — Exposure-minimization explanation incomplete

  • Location: docs/_components/StarterPrompt.tsx:110
  • Category: docs
  • Problem: Prompt says 'Keep secrets in memory only long enough to start the command; treat this as exposure minimization, not guaranteed erasure' but removed the explanation 'Node.js strings and the approved child process environment cannot be reliably zeroed.'
  • Impact: Operators may not understand why exposure minimization ≠ guaranteed erasure, potentially overestimating the helper's memory-scrubbing guarantees.
  • Suggested action: Append '(Node.js strings and the child process environment cannot be reliably zeroed)' to the exposure-minimization sentence.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read docs/_components/StarterPrompt.tsx line ~110 and confirm the parenthetical is present.
  • Missing regression test: Test that starter prompt explains why exposure minimization is not guaranteed erasure.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Diff shows the parenthetical explanation was removed from the prompt.
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 Missing positive assertion for new installer secure-prompt guidance — Add positive assertion: `expect(promptSource).toContain("prefer the installer's own secure prompt");`
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Account-home trust boundary underspecified for install-time credentials

  • Location: docs/_components/StarterPrompt.tsx:95
  • Category: security
  • Problem: The slimmed prompt tells agents to use `--execution-profile account-home --cwd <approved-absolute-directory>` for install/onboarding but does not explain why account-home is needed or what trust boundaries it implies (trusts OS account home and approved working directory; isolated uses a private temp home deleted after command exits).
  • Impact: Operators may not understand the security difference between profiles, leading to incorrect profile choice for commands that persist account state.
  • Recommended action: Add a brief inline note: 'account-home trusts the OS account home and the approved working directory; isolated uses a private temp home deleted after the command exits.'
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read docs/_components/StarterPrompt.tsx lines 115-125 and confirm the account-home trust boundary is explained.
  • Missing regression test: Test that the starter prompt contains 'account-home trusts' or equivalent trust-boundary explanation for the account-home profile.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read docs/_components/StarterPrompt.tsx lines 115-125 and confirm the account-home trust boundary is explained.
  • Evidence: Old prompt had multi-line explanation of account-home vs isolated trust boundaries; new prompt compresses to one line without the trust-model explanation.

PRA-2 Resolve/justify — PATH-based executable lookup rejection and stripped-variable taxonomy removed

  • Location: docs/_components/StarterPrompt.tsx:100
  • Category: security
  • Problem: Old prompt explicitly stated the helper 'rejects PATH-based executable lookup and removes recognized ambient credential-shaped and process-control variables, including configuration roots, source/executable selectors, proxy-routing and TLS-trust overrides, and NPM_CONFIG_* and PIP_* package-source settings'. New prompt only says 'requires an absolute executable, and strips ambient credential and process-control variables'.
  • Impact: Agents/operators lose visibility into the specific categories of environment variables the helper strips, reducing ability to diagnose why a needed variable might be missing or to verify the helper's safety model.
  • Recommended action: Restore 'rejects PATH-based executable lookup' and add parenthetical: '(including configuration roots, source/executable selectors, proxy/TLS overrides, NPM_CONFIG_*, PIP_*)'.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search docs/_components/StarterPrompt.tsx for 'PATH-based' and 'configuration roots' to verify presence.
  • Missing regression test: Test that starter prompt mentions PATH-based lookup rejection and lists the stripped variable categories.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search docs/_components/StarterPrompt.tsx for 'PATH-based' and 'configuration roots' to verify presence.
  • Evidence: Diff shows removal of the detailed stripped-variable enumeration from the prompt prose.

PRA-3 Resolve/justify — Windows .cmd/.bat executable guard removed

  • Location: docs/_components/StarterPrompt.tsx:105
  • Category: correctness
  • Problem: Old prompt included: 'On native Windows, do not target a .cmd or .bat file directly; use the absolute cmd.exe path and make its arguments part of the approved argv.' This concrete input-validation rule was removed.
  • Impact: On Windows, agents may incorrectly pass a .cmd/.bat file as the executable, which the helper will reject (requires absolute native executable). The guidance helped prevent this common mistake.
  • Recommended action: Re-add the Windows .cmd/.bat guidance as a one-sentence note.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search docs/_components/StarterPrompt.tsx for '.cmd' or '.bat' to verify presence.
  • Missing regression test: Test that starter prompt contains Windows .cmd/.bat guidance.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search docs/_components/StarterPrompt.tsx for '.cmd' or '.bat' to verify presence.
  • Evidence: Diff shows the Windows-specific bullet was deleted from the prompt.

PRA-4 Improvement — Missing positive assertion for new installer secure-prompt guidance

  • Location: test/starter-prompt-docs.test.ts:400
  • Category: tests
  • Problem: New negative assertions guard against `curl | bash` wrapper synthesis (`not.toContain` checks), but no positive assertion verifies the new guidance line 'prefer the installer's own secure prompt over routing it through the helper' remains in the prompt.
  • Impact: Future edits could accidentally remove the new installer-guidance line without test failure.
  • Suggested action: Add positive assertion: `expect(promptSource).toContain("prefer the installer's own secure prompt");`
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run `npx vitest run test/starter-prompt-docs.test.ts` and verify the new assertion passes.
  • Missing regression test: Add the positive assertion for installer secure prompt guidance in the 'pins local credential capture' test block.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Test file has 3 `not.toContain` regression guards but no matching positive assertion for the new guidance.

PRA-5 Improvement — Exposure-minimization explanation incomplete

  • Location: docs/_components/StarterPrompt.tsx:110
  • Category: docs
  • Problem: Prompt says 'Keep secrets in memory only long enough to start the command; treat this as exposure minimization, not guaranteed erasure' but removed the explanation 'Node.js strings and the approved child process environment cannot be reliably zeroed.'
  • Impact: Operators may not understand why exposure minimization ≠ guaranteed erasure, potentially overestimating the helper's memory-scrubbing guarantees.
  • Suggested action: Append '(Node.js strings and the child process environment cannot be reliably zeroed)' to the exposure-minimization sentence.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read docs/_components/StarterPrompt.tsx line ~110 and confirm the parenthetical is present.
  • Missing regression test: Test that starter prompt explains why exposure minimization is not guaranteed erasure.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Diff shows the parenthetical explanation was removed from the prompt.

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 — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: Install-time credential collection guidance in STARTER_PROMPT.
Open items: 0 required · 2 warnings · 0 suggestions · 3 test follow-ups
Top item: Clarify install-time credential handling

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: Install-time credential collection guidance in STARTER_PROMPT
  • PRA-2 Resolve or justify: Install-time credential guidance now has two competing paths in docs/_components/StarterPrompt.tsx:126
  • PRA-T1 Add or justify test follow-up: Acceptance clause
  • PRA-T2 Add or justify test follow-up: Acceptance clause
  • PRA-T3 Add or justify test follow-up: Install-time credential collection guidance in STARTER_PROMPT

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 security docs/_components/StarterPrompt.tsx:126 Make the install-time path explicit in the copied prompt. For example, say either: install without provider credentials and then run the relevant `nemoclaw onboard`/agent-specific onboard command so the CLI secret prompt collects the key locally; or, when non-interactive env credentials are required, collect them through the reviewed helper with `--execution-profile account-home --cwd <approved-absolute-directory>` and the exact approved argv. Avoid the generic phrase "When you have the approved values" unless it is tied to one of those safe collection paths.
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 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: Install-time credential collection guidance in STARTER_PROMPT

  • 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: Existing tests prove the old wrapper strings are absent and the helper/form pins remain. They do not prove the new copied prompt names one safe install-time credential flow.
  • 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: `StarterPrompt.tsx` lines 92, 110, and 126 each describe a different credential-handling concept; the changed test only asserts selected strings are present/absent.

PRA-2 Resolve/justify — Install-time credential guidance now has two competing paths

  • Location: docs/_components/StarterPrompt.tsx:126
  • Category: security
  • Problem: The slimmed prompt still starts with the rule to use the pinned local credential helper for every API key or token, but later tells agents to prefer the installer's own secure prompt for install-time credentials and also says to run the installer with approved credential values in the environment on the `bash` side of the `curl | bash` pipe. I found the helper enforces the important runtime controls, and CLI onboarding has a masked `prompt(..., { secret: true })` path, but `scripts/install.sh` itself documents `NVIDIA_INFERENCE_API_KEY` as an environment variable that skips a credential prompt. The copied prompt therefore does not clearly tell a coding agent whether the safe path is helper `account-home`, install without credentials followed by CLI onboarding, or a non-interactive env-based pipe.
  • Impact: A coding agent assisting a nontechnical user could collect a hosted-provider secret and then choose an env-based piped installer path without the helper's approved `account-home --cwd` boundary or a clearly identified CLI secret prompt. That weakens the prompt's no-secrets/local-only credential handling guarantees and makes future regressions harder to detect.
  • Recommended action: Make the install-time path explicit in the copied prompt. For example, say either: install without provider credentials and then run the relevant `nemoclaw onboard`/agent-specific onboard command so the CLI secret prompt collects the key locally; or, when non-interactive env credentials are required, collect them through the reviewed helper with `--execution-profile account-home --cwd <approved-absolute-directory>` and the exact approved argv. Avoid the generic phrase "When you have the approved values" unless it is tied to one of those safe collection paths.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `docs/_components/StarterPrompt.tsx` around lines 92-126 and compare it with `scripts/install.sh` usage text for `NVIDIA_INFERENCE_API_KEY` plus `src/lib/credentials/store.ts` `promptSecret`/`prompt(..., { secret: true })` to confirm which component actually prompts securely.
  • Missing regression test: Add or update a `starter-prompt-docs.test.ts` assertion that the install-time credential section names one unambiguous safe flow: either helper `account-home --cwd` for env-based non-interactive install or install-then-CLI-onboard secure prompt, and that it does not leave a standalone "approved values" env instruction detached from the safe collection mechanism.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `docs/_components/StarterPrompt.tsx` around lines 92-126 and compare it with `scripts/install.sh` usage text for `NVIDIA_INFERENCE_API_KEY` plus `src/lib/credentials/store.ts` `promptSecret`/`prompt(..., { secret: true })` to confirm which component actually prompts securely.
  • Evidence: `StarterPrompt.tsx` line 92 says to use the checked-in helper for secrets; line 110 says to prefer the command's own credential prompt for account-state commands; line 126 says to run the installer with credentials in the environment on the `bash` side of the pipe. The updated test guards against old wrapper prose but does not assert the replacement path is unambiguous.

💡 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.

  • None.
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 Acceptance clause — 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). — add test evidence or identify existing coverage. The diff does shrink the section and keeps the pinned helper/form URLs, SHA-256 digests, isolated example, redacted preview, no-secrets-in-chat/argv, and exposure-minimization language. The install-time safety path is less clear after the shrink, as described in the finding.
  • PRA-T2 Acceptance clause — Compress `account-home` to one line and prefer the installer's own secure credential prompt for install-time secrets. — add test evidence or identify existing coverage. `StarterPrompt.tsx` has a one-line `account-home --cwd <approved-absolute-directory>` instruction and says to prefer the installer's own secure prompt. Nearby code review found CLI secure prompt support, but `scripts/install.sh` itself documents credential env vars as skipping prompts, so the copied prompt needs clearer source-of-truth wording.
  • PRA-T3 Install-time credential collection guidance in STARTER_PROMPT — Existing tests prove the old wrapper strings are absent and the helper/form pins remain. They do not prove the new copied prompt names one safe install-time credential flow.. `StarterPrompt.tsx` lines 92, 110, and 126 each describe a different credential-handling concept; the changed test only asserts selected strings are present/absent.

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.

@cv
cv merged commit 22f6ac3 into codex/harden-local-credential-helper Jul 8, 2026
41 checks passed
@cv
cv deleted the slim-starter-prompt-credential branch July 8, 2026 18:28
@wscurran wscurran added area: docs Documentation, examples, guides, or docs build area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: security Security controls, permissions, secrets, or hardening chore Build, CI, dependency, or tooling maintenance labels Jul 11, 2026
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: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: security Security controls, permissions, secrets, or hardening chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants