fix(auth): document stdin paste-token setup and surface validTypes#63050
Conversation
Greptile SummaryThis PR makes three targeted fixes: adds a Confidence Score: 5/5Safe to merge; only finding is a P2 suggestion to add Anthropic token format validation in the non-interactive path. All three changes are narrow and correct. The normalizeOptionalString behavior (trims + nullifies empty) is confirmed from source, so whitespace-only --token values correctly fall back to the interactive prompt. The only gap (Anthropic format validation bypass) is a quality suggestion, not a defect that blocks merge. No files require special attention.
|
| const rawToken = normalizeOptionalString(opts.token); | ||
| const tokenInput = | ||
| rawToken ?? | ||
| (await text({ | ||
| message: `Paste token for ${provider}`, | ||
| validate: (value) => { | ||
| const trimmed = value?.trim(); | ||
| if (!trimmed) { | ||
| return "Required"; | ||
| } | ||
| if (provider === "anthropic") { | ||
| return validateAnthropicSetupToken(trimmed.replaceAll(/\s+/g, "")); | ||
| } | ||
| return undefined; | ||
| }, | ||
| })); |
There was a problem hiding this comment.
--token flag
When rawToken is non-null (i.e. --token was passed), the interactive text({ validate }) branch is skipped entirely, so validateAnthropicSetupToken is never invoked. A malformed token passed non-interactively is stored without any format check and will only surface as an API-level error on first use. Consider calling validateAnthropicSetupToken on rawToken directly before reaching upsertAuthProfile, mirroring the validation already applied in the interactive path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/models/auth.ts
Line: 388-403
Comment:
**Anthropic token validation bypassed for `--token` flag**
When `rawToken` is non-null (i.e. `--token` was passed), the interactive `text({ validate })` branch is skipped entirely, so `validateAnthropicSetupToken` is never invoked. A malformed token passed non-interactively is stored without any format check and will only surface as an API-level error on first use. Consider calling `validateAnthropicSetupToken` on `rawToken` directly before reaching `upsertAuthProfile`, mirroring the validation already applied in the interactive path.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce3e364a9f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const tokenInput = | ||
| rawToken ?? | ||
| (await text({ |
There was a problem hiding this comment.
Validate
--token value before persisting Anthropic auth
When --token is supplied, execution takes the rawToken ?? ... branch and bypasses the text(...validate...) callback, so validateAnthropicSetupToken is never applied to non-interactive input. In CI/scripted usage this allows malformed Anthropic setup tokens (wrong prefix/too short) to be saved into auth-profiles.json, and the command reports success even though auth will fail later. The --token path should run the same provider-specific validation before calling upsertAuthProfile.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76cb6b0654
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const rawToken = normalizeOptionalString(opts.token); | ||
| if (rawToken !== undefined) { | ||
| if (!rawToken) { |
There was a problem hiding this comment.
Reject empty --token values before normalization
normalizeOptionalString(opts.token) turns an explicitly provided empty token (--token "" or whitespace-only) into undefined, so the subsequent if (!rawToken) guard never runs and the command falls through to the interactive text(...) prompt. In scripted/non-TTY usage this regresses the new non-interactive path by hanging/failing at prompt time instead of returning a clear validation error for an empty --token input.
Useful? React with 👍 / 👎.
|
Codex review: needs maintainer review before merge. Reviewed May 27, 2026, 5:23 AM ET / 09:23 UTC. Summary PR surface: Source -1, Tests +4, Docs +2. Total +5 across 6 files. Reproducibility: yes. source inspection gives a high-confidence path: current main already reads piped stdin for Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge Security Review detailsBest possible solution: Merge the narrow docs and diagnostic change after normal maintainer CI/review, while leaving broader Copilot PAT or new non-interactive auth surfaces to separate reviewed work. Do we have a high-confidence way to reproduce the issue? Yes, source inspection gives a high-confidence path: current main already reads piped stdin for Is this the best way to solve the issue? Yes, the final patch is the narrow maintainable solution: it documents the existing stdin path, adds an additive diagnostic field, and avoids introducing an argv secret option. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against d6949d5951e3. Label changesLabel justifications:
Evidence reviewedPR surface: Source -1, Tests +4, Docs +2. Total +5 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
ab7efc1 to
57874b9
Compare
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Pearl Merge Sprite Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
e43cef9 to
ac4933e
Compare
29c0686 to
3cb5e0c
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
@clawsweeper re-review Added current-head real behavior proof for the invalid auth-profile diagnostic path: a real CLI run loads an invalid auth-profiles.json with JSON console logging and emits the warning payload including . |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
@clawsweeper re-review Added current-head real behavior proof for the invalid auth-profile diagnostic path. A real CLI |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
When --token is supplied non-interactively, the interactive text() validate callback is bypassed. Add the same validateAnthropicSetupToken check for the --token path so malformed tokens are rejected early. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
normalizeOptionalString turns empty/whitespace-only strings into undefined, so the previous check (inside the rawToken !== undefined block) was unreachable. Move the empty-value guard before normalization by checking opts.token directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Maintainer merge proof for #63050. Behavior addressed: documents the existing stdin path for Real environment tested: GitHub CI on PR head Exact steps or command run after this patch:
Evidence after fix:
Observed result after fix: maintainer source review confirms the PR documents existing stdin ingestion via What was not tested: I did not rerun local Vitest locally before merge because the PR head already has green GitHub CI and real-behavior proof for the touched surface. No live provider token authentication was tested; this change covers CLI ingestion/docs and local auth-profile warning payload shape. Thanks @liaoandi. |
|
Landed on
Thanks @liaoandi. |
Summary
Fixes #63042 by keeping non-interactive
paste-tokensetup on the existing stdin path and improving auth-profile load diagnostics.Changes
paste-tokenstdin path instead of adding an argv secret option.paste-tokentoken validation on the sharedreadPastedSecretpath.validTypesin invalid auth-profile warning logs.validTypeswarning payload.CHANGELOG.mdunchanged because this is a small PR repair on top of currentmain.Security note
This version does not accept provider tokens through
--token <value>. Automation should pipe token material on stdin so credentials do not appear in shell history, process listings, command echoing, or CI logs.Real behavior proof
Behavior addressed:
openclaw models auth paste-tokenstill supports non-interactive token setup through stdin, the removed--tokenargv secret path is rejected by the real CLI, and invalid auth-profile load warnings now include acceptedvalidTypes.Real environment tested: local macOS checkout of current PR head
f82a7a24e412da9f6557a93f25fc50f86dcddef1on 2026-05-26, using an isolatedOPENCLAW_HOMEandOPENCLAW_STATE_DIR.Exact steps or command run after this patch:
Evidence after fix:
Observed result after fix: stdin token setup succeeded and persisted the redacted
openai:prooftoken profile; the removed argv token path failed before accepting a provider credential; loading an invalid auth-profile store emitted a real warning withvalidTypes: ["api_key","oauth","token"].What was not tested: real provider-side authentication was not tested with a live provider token. The changed behavior under test is CLI token ingestion through stdin, local profile persistence, rejection of argv-carried provider credentials, and runtime warning payload shape for invalid auth-profile entries.
Test plan
node scripts/run-vitest.mjs src/commands/models/auth.test.ts src/cli/models-cli.test.ts src/agents/auth-profiles.ensureauthprofilestore.test.ts./node_modules/.bin/oxfmt --check --threads=1 docs/cli/models.md src/agents/auth-profiles.ensureauthprofilestore.test.ts src/agents/auth-profiles/persisted.ts src/cli/models-cli.ts src/commands/models/auth.test.ts src/commands/models/auth.tsgit diff --check upstream/main...HEAD# no output🤖 Generated with Claude Code