feat(cli): add --token/-t flag for ephemeral token override#19
Conversation
…e --token and -t (value or =value)\n- Prefer inline token over env/config for this run\n- Do not persist when provided via CLI flag\n- Help text: document token flag in usage
WalkthroughAdds CLI support for an ephemeral token flag (--token/-t), threads it into the UI, and updates App initialisation to prioritise inline tokens with optional non-persistence. Documentation gains sections on flags, precedence, and security notes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (index.tsx)
participant App as App (UI)
User->>CLI: Run command with optional --token/-t
CLI->>CLI: Parse flags (getFlagValue / getShortFlagValue)
CLI->>App: Mount App(inlineToken, inlineTokenEphemeral)
rect rgb(240,248,255)
note right of App: On mount or inlineToken change
alt Inline token provided
App->>App: Set token = inlineToken<br/>mode = validating
else No inline token
App->>App: Try env token (GITHUB_TOKEN/GH_TOKEN)
alt Env token found
App->>App: Set token = env token<br/>mode = validating
else
App->>App: Try stored token
alt Stored token found
App->>App: Set token = stored token<br/>mode = validating
else
App->>App: mode = auth_method_selection
end
end
end
end
App->>App: Validate/authenticate token
alt Auth success
opt Persist decision
note over App: Persist only if no stored token and not ephemeral
alt inlineTokenEphemeral == true
App->>App: Skip persistence
else
App->>App: Store token locally
end
end
else Auth failure
App->>App: Handle error / prompt
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Lines 243 to 246 in 9fcf7dd
[P1] Avoid deleting persisted token when inline flag token fails
The new CLI flag lets users supply a one-off token, but when validation fails the error branch still invokes clearStoredToken(). If a user already has a valid token saved and tries an inline token that is invalid or encounters a network error, this code deletes the saved token even though it was not used. That contradicts the promise that --token/-t is an “ephemeral” override and forces users to re-enter their stored token. Consider skipping the clear when the current token originated from the CLI flag or otherwise track the token source before wiping persisted credentials.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| setMode('auth_method_selection'); | ||
| } | ||
| }, []); | ||
| }, [inlineToken]); |
There was a problem hiding this comment.
Bug: Token Source Misalignment in CLI
When an inline token is provided via a CLI flag, the tokenSource state variable is incorrectly set. It's initialized from getTokenSource() (which checks environment or stored tokens) and isn't updated to reflect the inline token's origin. This leads to misleading log information about the token's source, making debugging and monitoring difficult.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/App.tsx (1)
242-249: Avoid erasing a valid stored token when an inline/env token failsOn validation failure you unconditionally call
clearStoredToken(), which can wipe a previously valid stored token even if the failing token came from--tokenor env. Clear only if the stored token matches the one that failed validation.- } else { - // Invalid token or other error: clear token input and return to selection - setError(errorMessage); - setInput(''); - setToken(null); - // Clear stored token since it's invalid - clearStoredToken(); - setMode('auth_method_selection'); - } + } else { + // Invalid token or other error: clear token input and return to selection + const badToken = token; // capture before clearing + setError(errorMessage); + setInput(''); + setToken(null); + // Only clear stored token if it is the one that just failed validation + try { + const storedNow = getStoredToken(); + if (storedNow && badToken && storedNow === badToken) { + clearStoredToken(); + } + } catch {} + setMode('auth_method_selection'); + }
🧹 Nitpick comments (8)
wiki/Usage.md (1)
5-15: Clarify env-token persistence and tighten phrasing
- Suggest explicitly noting that env-provided tokens may be persisted on first successful auth when no stored token exists (current App.tsx logic). This sets expectations and aligns docs with behaviour.
- Minor copy tweak for brevity.
## CLI Flags -- `--token, -t <pat>`: Provide a Personal Access Token just for this run (not persisted). +- `--token, -t <pat>`: Provide a Personal Access Token for this run only (not persisted). - Examples: `gh-manager-cli --token ghp_XXX`, `gh-manager-cli -t=ghp_XXX` - Precedence: CLI token > env (`GITHUB_TOKEN`/`GH_TOKEN`) > stored config. - - Security: Passing tokens on the command line can appear in shell history. Prefer env vars or the interactive prompt. + - Security: Passing tokens on the command line can appear in shell history and process lists. Prefer env vars or the interactive prompt. + - Persistence note: If no stored token exists, an env-provided token may be saved after successful validation (so subsequent runs don’t require re-entry). Use `--token/-t` if you want a truly non-persistent, per-run token.wiki/Token-and-Security.md (1)
18-26: Document actual persistence behaviour for env tokensThe code path currently persists env tokens when no stored token exists, while
--token/-tremains non-persistent. Call this out to avoid surprises.### CLI Token Flag vs Environment Variables - You can supply a token just for a single run with the CLI flag: `--token <pat>` or `-t <pat>`. - Precedence: CLI token > environment (`GITHUB_TOKEN` / `GH_TOKEN`) > stored config. - Non‑persistence: Tokens provided via `--token/-t` are NOT saved to disk. - Security: Command‑line arguments may be recorded in shell history and visible to other local users via process listings. Prefer environment variables or the interactive prompt when possible. - Safer example (one‑off): `GITHUB_TOKEN=ghp_XXXX gh-manager-cli` - Or run without flags and enter the token at the prompt (it is masked and then stored locally with restricted permissions). + - Persistence behaviour: If there is no stored token yet, an environment‑provided token may be persisted after successful validation so future runs work without the env var. If you prefer not to persist, use `--token/-t`.src/ui/App.tsx (2)
169-173: Fix inverted logging field
tokenStored: !getStoredToken()reports the opposite of reality. Use a boolean cast.- tokenSource, - tokenStored: !getStoredToken() + tokenSource, + tokenStored: Boolean(getStoredToken())
179-182: Don’t persist env-sourced tokens; pass the source when persistingAligns with the docs’ emphasis on safer env usage and avoids silently writing env secrets to disk. Also, provide the source to
storeTokenfor accurate provenance.- // If token came from prompt, it will be in input and not yet stored - if (!getStoredToken() && !inlineTokenEphemeral) { - storeToken(token); - } + // Persist only when there isn't already a stored token, + // not an inline ephemeral token, and not from env. + if (!getStoredToken() && !inlineTokenEphemeral && tokenSource !== 'env') { + storeToken(token, tokenSource); + }Confirm
storeTokenaccepts(value: string, source: TokenSource)outside the OAuth path; if not, add the overload and updateconfigaccordingly.src/index.tsx (4)
13-40: Make flag parsing robust: handle '=' safely, last-wins semantics, and respect '--' terminator
- split('=') truncates values containing '='; slice after the first '=' instead.
- Typical CLIs use “last flag wins” across long/short forms; current code always prefers the long flag even if -t appears later.
- Respect the argument terminator '--' so positionals aren’t parsed as flags.
Apply this diff to replace both helpers with a single, safer parser:
-// Simple argv helpers -const getFlagValue = (name: string): string | undefined => { - // Supports --name value and --name=value - const idx = argv.findIndex(a => a === `--${name}` || a.startsWith(`--${name}=`)); - if (idx === -1) return undefined; - const at = argv[idx]; - if (at.includes('=')) { - const [, v] = at.split('='); - return v?.trim() || undefined; - } - const next = argv[idx + 1]; - if (next && !next.startsWith('-')) return next.trim(); - return undefined; -}; -const getShortFlagValue = (short: string): string | undefined => { - // Supports -x value and -x=value - const exact = `-${short}`; - const idx = argv.findIndex(a => a === exact || a.startsWith(`${exact}=`)); - if (idx === -1) return undefined; - const at = argv[idx]; - if (at.includes('=')) { - const [, v] = at.split('='); - return v?.trim() || undefined; - } - const next = argv[idx + 1]; - if (next && !next.startsWith('-')) return next.trim(); - return undefined; -}; +// Simple argv helper (last-wins, supports --, and '=' inside values) +const getFlagValue = (name: string, short?: string): string | undefined => { + const stop = argv.indexOf('--'); // do not parse beyond '--' + const end = stop === -1 ? argv.length : stop; + for (let i = end - 1; i >= 0; i--) { + const a = argv[i]; + // --name=value + if (a.startsWith(`--${name}=`)) { + const eq = a.indexOf('='); + const v = a.slice(eq + 1).trim(); + return v || undefined; + } + // --name value + if (a === `--${name}`) { + const next = argv[i + 1]; + if (next && !next.startsWith('-')) return next.trim(); + return undefined; + } + if (short) { + // -s=value + if (a.startsWith(`-${short}=`)) { + const eq = a.indexOf('='); + const v = a.slice(eq + 1).trim(); + return v || undefined; + } + // -s value + if (a === `-${short}`) { + const next = argv[i + 1]; + if (next && !next.startsWith('-')) return next.trim(); + return undefined; + } + } + } + return undefined; +};
52-55: Tighten help text: show short aliases and examplesInclude short forms for version/help and demonstrate value forms for the token to match supported syntax.
- ` gh-manager-cli Launch the TUI\n` + - ` gh-manager-cli --token, -t Use a token just for this run (not persisted)\n` + - ` gh-manager-cli --version Print version\n` + - ` gh-manager-cli --help Show help\n\n` + + ` gh-manager-cli Launch the TUI\n` + + ` gh-manager-cli --token=<pat> Use a token just for this run (not persisted)\n` + + ` gh-manager-cli -t <pat> Same as --token=<pat>\n` + + ` gh-manager-cli --version, -v Print version\n` + + ` gh-manager-cli --help, -h Show help\n\n` +
104-110: Flag present but missing value silently falls back — confirm UX and consider explicit errorIf users pass “--token” or “-t” without a value, we currently ignore it and fall back to env/stored tokens, which may be surprising. If you prefer explicitness, fail fast with a helpful message.
Also, after adopting the unified helper above, use it here and (optionally) scrub argv to reduce accidental logging exposure.
-// Parse optional token flag (ephemeral) -const inlineToken = (() => { - const v = getFlagValue('token') ?? getShortFlagValue('t'); - if (!v) return undefined; - return v.trim(); -})(); +// Parse optional token flag (ephemeral) +const inlineToken = (() => { + const token = getFlagValue('token', 't'); + const hasTokenFlag = argv.some(a => a === '--token' || a.startsWith('--token=') || a === '-t' || a.startsWith('-t=')); + if (hasTokenFlag && token === undefined) { + console.error('Error: --token/-t requires a value (e.g. --token=ghp_xxx or -t ghp_xxx)'); + process.exit(2); + } + const v = token?.trim(); + // Best-effort scrub to avoid accidental argv logging (does not hide from system process lists) + if (v) { + for (let i = 0; i < argv.length; i++) { + if (argv[i].startsWith('--token=')) argv[i] = '--token=***'; + if (argv[i] === '--token' && argv[i + 1] && !argv[i + 1].startsWith('-')) argv[i + 1] = '***'; + if (argv[i].startsWith('-t=')) argv[i] = '-t=***'; + if (argv[i] === '-t' && argv[i + 1] && !argv[i + 1].startsWith('-')) argv[i + 1] = '***'; + } + } + return v; +})();
114-114: Minor: make ephemerality explicitBoolean(inlineToken) is fine; inlineToken !== undefined reads clearer given we already trim to undefined.
- <App inlineToken={inlineToken} inlineTokenEphemeral={Boolean(inlineToken)} /> + <App inlineToken={inlineToken} inlineTokenEphemeral={inlineToken !== undefined} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/index.tsx(3 hunks)src/ui/App.tsx(4 hunks)wiki/Token-and-Security.md(1 hunks)wiki/Usage.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/ui/App.tsx (1)
src/config.ts (1)
getStoredToken(64-67)
src/index.tsx (1)
src/ui/App.tsx (1)
App(16-510)
🪛 LanguageTool
wiki/Usage.md
[uncategorized] ~7-~7: Loose punctuation mark.
Context: ...es. ## CLI Flags - --token, -t <pat>: Provide a Personal Access Token just fo...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~12-~12: Loose punctuation mark.
Context: ... the interactive prompt. - --help, -h: Show usage information and exit. - `--...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...information and exit. - --version, -v: Print the current version and exit. ##...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (1)
src/ui/App.tsx (1)
16-16: Props extension LGTMAccepting
inlineTokenandinlineTokenEphemeralis a clean interface for ephemeral CLI tokens.
|
Closing in favour of #18 which now includes both --org/-o and --token/-t. Keeping the work consolidated in a single PR to reduce review overhead and avoid duplicate changes. |
Adds a short and long flag to provide a one-off token without persisting it.\n\nChanges:\n- CLI: parse
--tokenand-t(both value and =value forms)\n- App: prefer inline token over env/config for current run only; skip persistence\n- Help/Docs: usage text and wiki updated; security note on CLI tokens vs env vars\n\nNotes:\n- Precedence: CLI token > env (GITHUB_TOKEN/GH_TOKEN) > stored config\n- Security: CLI args may appear in shell history; env or prompt is saferSummary by CodeRabbit