Skip to content

fix(cli): name VERYFRONT_API_TOKEN when only VERYFRONT_API_KEY is set - #5479

Open
kwakayama wants to merge 10 commits into
mainfrom
fix/4220-api-key-hint
Open

kwakayama wants to merge 10 commits into
mainfrom
fix/4220-api-key-hint

Conversation

@kwakayama

@kwakayama kwakayama commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

With only VERYFRONT_API_KEY set (the name Studio's key page prints), missing-auth messages now name VERYFRONT_API_TOKEN instead of a bare "Not logged in".

  • One helper, misnamedApiKeyHint() in cli/auth/login.ts, used by whoami (human and JSON), login's non-interactive paths, resolveConfig* (push/pull and other commands) and up.
  • No hint when VERYFRONT_API_TOKEN is also set. No rename or alias (that's veryfront/veryfront-issue-inbox#4088).
  • Tests for whoami human output, whoami JSON, both vars set, and the resolveConfig family; they fail without the fix.

Fixes veryfront/veryfront-issue-inbox#4220

Studio tells users to export VERYFRONT_API_KEY, which the CLI does not
read. whoami, login and the commands that need a credential now say so
instead of a bare 'Not logged in'.

Fixes veryfront/veryfront-issue-inbox#4220
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-07T15:37:45.372825Z 5c33c92 Manual request
ℹ️ 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
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 66 366 KiB ✅ 0
src/react/components/Head.tsx 5 74 KiB ✅ 0
src/react/router/index.tsx 5 74 KiB ✅ 0
src/react/context/index.tsx 5 74 KiB ✅ 0
src/react/fonts/index.ts 6 81 KiB ✅ 0
src/react/components/ui/index.ts 111 717 KiB ✅ 0
src/react/components/ui/adapter/contract.ts 1 18 KiB ✅ 0
src/react/components/ui/icons/index.ts 3 17 KiB ✅ 0
src/chat/index.ts 305 2046 KiB ✅ 0
src/chat/ag-ui.ts 61 459 KiB ✅ 0
src/chat/protocol.ts 1 11 KiB ✅ 0
src/chat/types.ts 46 246 KiB ✅ 0
src/chat/message-prep.ts 68 517 KiB ✅ 0
src/markdown/index.ts 6 30 KiB ✅ 0
src/mdx/index.ts 2 2 KiB ✅ 0
src/agent/identity-contracts.ts 1 3 KiB ✅ 0
src/schemas/index.ts 43 220 KiB ✅ 0
src/platform/env.ts 5 53 KiB ✅ 0
src/workflow/claude-code/react/index.ts 9 91 KiB ✅ 0
src/events/index.ts 31 567 KiB ✅ 0
src/events/ag-ui/public.ts 30 561 KiB ✅ 0
src/run-events/index.ts 54 321 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

@codex review

Copy link
Copy Markdown
Contributor Author

Score: 82/100. Small, well-scoped fix that names VERYFRONT_API_TOKEN when only VERYFRONT_API_KEY is set. It has one misplaced doc comment and a few untested call sites.

  • Strength: The change is minimal and reuses one helper, misnamedApiKeyHint(), across whoami, login, up, and resolveConfigBase. The hint only fires when VERYFRONT_API_TOKEN is unset, and tests confirm the key value is never printed.
  • Strength: Tests cover human and JSON whoami output, the case where both variables are set, and both resolveConfig paths. The JSON envelope shape (slug, registrySlug) is preserved.
  • Concern (should fix): In cli/auth/login.ts, the new MISNAMED_API_KEY_HINT_LINES and misnamedApiKeyHint() sit between the existing docblock ("block sign-in forever, so the check is bounded...") and DEFAULT_EXISTING_SESSION_TIMEOUT_MS. That orphans the timeout comment and attaches it to the new constant. Move the new block above that docblock.
  • Concern (minor): The login() non-interactive branch, the requestInteractiveAuthentication branch, the outputLoginAuthenticationRequiredJson message and the up command's JSON auth error have no test coverage. The "Not logged in. ${hint ?? fallback}" template is also repeated three times in login.ts. A small helper such as notLoggedInMessage(fallback) would remove the duplication.
  • Nit: misnamedApiKeyHint() reads getEnv directly instead of the injected env config. That is fine for a raw-variable check, but getApiTokenEnv is the usual path, so a short comment would help.

Overall this is ready after the docblock move. The extra tests are optional.


Generated by Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 47e9c8c390

ℹ️ 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".

@codecov

codecov Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.48649% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/commands/init/init-command.ts 0.00% 7 Missing ⚠️
cli/auth/login.ts 94.23% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

They pushed the sanitizer opt-out count over its baseline (342 > 340) and the
tests pass without them.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc64523e4e

ℹ️ 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".

Comment thread cli/auth/login.ts Outdated
* variable the CLI does read instead of a bare "Not logged in".
*/
export function misnamedApiKeyHint(): string | undefined {
if (!getEnv("VERYFRONT_API_KEY") || getEnv("VERYFRONT_API_TOKEN")) return undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Do not promote project .env keys into login credentials

When a checkout contains VERYFRONT_API_KEY in its .env, CLI startup loads that repository-controlled value into getEnv, so this condition presents it as though the user exported it and instructs them to copy the same value into the trusted VERYFRONT_API_TOKEN variable. Following that guidance before a push or deploy can authenticate as the repository author's account and upload the checkout to a project they control. Check getEnvSource("VERYFRONT_API_KEY") and only recommend reusing the value when it came from the process environment; for env-file values, do not suggest promoting the credential.

Useful? React with 👍 / 👎.

Comment thread cli/auth/login.ts Outdated
* variable the CLI does read instead of a bare "Not logged in".
*/
export function misnamedApiKeyHint(): string | undefined {
if (!getEnv("VERYFRONT_API_KEY") || getEnv("VERYFRONT_API_TOKEN")) return undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Account for token-file credentials before showing this hint

When VERYFRONT_API_TOKEN_FILE supplies a credential and VERYFRONT_API_KEY is also set, a rejected file credential reaches this helper with the literal VERYFRONT_API_TOKEN variable unset, so whoami, up, and JSON login output recommend setting that variable to the API key. That recovery cannot work because environment configuration resolves the token file ahead of VERYFRONT_API_TOKEN on the next invocation as well. Base this check on the resolved credential inputs, or suppress the hint and identify the token file when it is the effective credential source.

Useful? React with 👍 / 👎.

Skip the hint when the key came from a project .env, or when
VERYFRONT_API_TOKEN_FILE supplies the credential.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6529730b8f

ℹ️ 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".

Comment thread cli/auth/login.ts Outdated
* suggested as the user's own credential. A token file, when set, is the
* effective credential, and setting the variable would not change that.
*/
export function misnamedApiKeyHint(): string | undefined {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the API-key hint to JSON init deploy failures

When veryfront init --deploy --json runs with only a process-supplied VERYFRONT_API_KEY, ensureAuthenticated() intentionally emits nothing in JSON mode and init-command.ts constructs its own AUTH_REQUIRED_DEPLOY_MESSAGE, so this helper is never used and the resulting envelope still says only Authentication required for --deploy. Apply the hint when constructing that caller's deployment failure as well.

AGENTS.md reference: AGENTS.md:L20-L25

Useful? React with 👍 / 👎.

Comment thread cli/shared/config.test.ts
Comment on lines +2082 to +2086
const originalKey = Deno.env.get("VERYFRONT_API_KEY");
const originalToken = Deno.env.get("VERYFRONT_API_TOKEN");
try {
Deno.env.set("VERYFRONT_API_KEY", "vf_misnamed_key");
Deno.env.delete("VERYFRONT_API_TOKEN");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear the token-file variable in the config hint tests

When the test process already has VERYFRONT_API_TOKEN_FILE set, misnamedApiKeyHint() suppresses the hint by design, but this fixture only saves and clears VERYFRONT_API_TOKEN. Consequently both new assertions fail on authenticated developer or CI environments that supply credentials through a token file. Save, delete, and restore VERYFRONT_API_TOKEN_FILE alongside the other credential variables.

Useful? React with 👍 / 👎.

Comment thread cli/auth/login.ts Outdated
export function misnamedApiKeyHint(): string | undefined {
if (!getEnv("VERYFRONT_API_KEY")) return undefined;
if (getEnvSource("VERYFRONT_API_KEY").source !== "process") return undefined;
if (getEnv("VERYFRONT_API_TOKEN") || getEnv("VERYFRONT_API_TOKEN_FILE")) return undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Suppress the hint when another credential is retained

When an exported VERYFRONT_API_KEY exists alongside a stored-login or veryfront.json credential, a transient network or service failure leaves that credential in place but makes whoami and JSON up reach this helper. Because it checks only the two token environment variables, the output incorrectly recommends replacing the credential with the API key even though authentication could not be verified due to availability. Base the hint on the resolved candidate sources or suppress it when validation failed without rejecting the existing credential.

AGENTS.md reference: AGENTS.md:L20-L25

Useful? React with 👍 / 👎.

The hint now stays out when a stored login or veryfront.json token is
the credential that failed, and init --deploy --json carries it in its
deployment failure message. Config hint tests clear the token-file
variable.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dfabc36519

ℹ️ 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".

Comment thread cli/auth/login.ts
Comment on lines +152 to +153
const candidates = await resolveApiCredentialCandidatesForAuth(env, projectDir);
return candidates.length > 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Count credentials before applying host trust filtering

When a project .env steers the API URL and the user has both a stored login and an exported VERYFRONT_API_KEY, resolveApiCredentialCandidatesForAuth() deliberately filters out the token-store candidate, so this helper concludes that no credential is configured and whoami, login, and up recommend copying the API key into VERYFRONT_API_TOKEN. That recovery cannot work because the same repository-host trust rule will filter the new token too. Check the unfiltered configured credential sources, or suppress the hint for a repository-steered host. Fresh evidence beyond the prior retained-credential report is this trust-filtered candidate path.

AGENTS.md reference: AGENTS.md:L26-L28

Useful? React with 👍 / 👎.

That host filters out the user's own credentials, so copying the key
into VERYFRONT_API_TOKEN would not help.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

The fixture's operator API base outranks veryfront.json, so the old test
never steered the host.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a0b4563b2

ℹ️ 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".

Comment thread cli/auth/login.ts Outdated
): Promise<string | undefined> {
if (!getEnv("VERYFRONT_API_KEY")) return undefined;
if (getEnvSource("VERYFRONT_API_KEY").source !== "process") return undefined;
if (getEnv("VERYFRONT_API_TOKEN") || getEnv("VERYFRONT_API_TOKEN_FILE")) return undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Treat whitespace-only token exports as unset

When VERYFRONT_API_TOKEN contains only whitespace and a valid process-supplied VERYFRONT_API_KEY is present, this check suppresses the new hint even though resolveApiCredentialCandidates() trims the token and treats it as absent. Authentication therefore fails with the generic message in exactly the configuration where the API-key recovery guidance is needed; trim the token before deciding it is configured, or rely on the resolved candidate set.

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5c9089f36

ℹ️ 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".

Comment thread cli/auth/login.ts Outdated
env: EnvironmentConfig = getEnvironmentConfig(),
projectDir: string = cwd(),
): Promise<string | undefined> {
if (!getEnv("VERYFRONT_API_KEY")) return undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Treat a whitespace-only API key as unset

When VERYFRONT_API_KEY is exported as whitespace and no credential is configured, this truthiness check treats it as a usable key, so every new authentication error recommends setting VERYFRONT_API_TOKEN to the same whitespace value. The credential resolver trims that copied value and still treats it as absent, making the recovery guidance ineffective; trim the API-key value before deciding to show the hint.

Useful? React with 👍 / 👎.

Comment thread cli/auth/login.ts Outdated
// would filter a newly exported VERYFRONT_API_TOKEN the same way.
const configFile = await readConfigJsonFile(projectDir);
const trust = resolveApiUrlTrust(env, configFile, join(projectDir, "veryfront.json"));
if (trust.repositorySteered) return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Limit hint suppression to env-file-steered hosts

When a project's veryfront.json sets a non-default apiUrl but no apiToken, this blanket branch suppresses the hint even if an exported VERYFRONT_API_KEY is the only credential. Unlike the env-file-steered case, resolveApiCredentialCandidatesForAuth() deliberately validates shell and token-store credentials against the environment-derived URL and retains them for config-file steering, so exporting VERYFRONT_API_TOKEN would make whoami and login succeed; preserve the suppression for steeringEnvFile, but do not apply it to config-file steering on those authentication surfaces.

AGENTS.md reference: AGENTS.md:L26-L28

Useful? React with 👍 / 👎.

…steering

Shell credentials still validate against the operator host when only
veryfront.json names an apiUrl, so exporting VERYFRONT_API_TOKEN works
there and the hint applies.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 5c33c92ae5

ℹ️ 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".

@sonarqubecloud

sonarqubecloud Bot commented Nov 7, 2026

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants