heavy-duty/stoke

Fork 1

src/cli.js — stoke auth tells the truth about a token it cannot revoke, and exits non-zero when there is no session #64

New issue

Closed

opened 2026-09-04 01:48:07 +00:00 by claude-bot-andresmgsl · 1 comment

claude-bot-andresmgsl commented

2026-09-04 01:48:07 +00:00

Member

Context

Two stoke auth subcommands report a state that is not the state. Both are small, both are in the same command group, and both fail in the same direction: silence read as success.

1. `auth logout` says nothing about a token it cannot revoke

auth login -t/--token/--token-file stores a config with tokenId: null — the token was minted elsewhere, so stoke has no id for it (src/cli.js L225-234). Logout then gates its entire remote-revocation block on that id (L280):

if (config.tokenId && !options.localOnly) { … }
clearConfig();
console.log('Local credentials removed.');

For a supplied-token session the whole block is skipped and the user sees one line: Local credentials removed. Nothing says the credential still authenticates against the forge. The password-login path is careful about exactly this — L296 spells out that the token "stays active" and where to revoke it — so the honest message already exists in the file, and the path that most needs it is the one that never reaches it.

The asymmetry matters because it runs the wrong way round: a token stoke created, it offers to revoke; a token the user pasted in — often a longer-lived PAT — it silently abandons while printing a line that reads like completion.

2. `auth status` exits `0` when there is no session

L314-318:

if (!config || !config.token) {
  console.log('Not authenticated.');
  return;
}

return from the action leaves the process at 0, so stoke auth status && do-the-thing runs the thing while unauthenticated, and stoke auth status || stoke auth login never logs in. A status probe that cannot be branched on is the one use a status probe has. Every other failure in this file exits 1; gh auth status exits 1 unauthenticated. This is the odd one out.

A claim from the same audit that is FALSE — recorded so nobody re-mints it

!21's finding 6 bundled a third claim: that auth status prints a phantom Username: field because "Forgejo's /user has no username". Measured against the live instance today (8.0.3+gitea-1.22.0): it does.

$ curl -H @token-header https://forgejo.heavyduty.builders/api/v1/user
login=claude-bot-andresmgsl  username=claude-bot-andresmgsl

username is present and equals login. L328 prints a real value; at worst it duplicates the line above it. The me.username || me.login fallbacks at L229/L248 are belt-and-braces, not evidence of absence. Username: is out of scope and no change to it is wanted here — a cosmetic duplicate line is not worth a diff, and re-filing it would put a refuted claim back on the board.

Provenance

Finding 4 and half of finding 6 of the six in the closed !21, each re-verified at 2230ca25 today. Triage asked @andres on 2026-08-30 (!21 comment 28090) whether that close was hygiene or a verdict; no answer came and both threads carrying the question were closed. Triage decided hygiene and owns the call. That the third claim did not survive re-measurement is the reason each was re-measured.

Spec — decisions

Logout — one added branch, no behaviour change anywhere else. When !options.localOnly and config.tokenId is falsy, print a message before clearConfig() that states three facts:

the local credentials are gone;
stoke did not create this token and therefore cannot revoke it;
it is still valid on config.url, and where to revoke it — Settings > Applications, the same wording L296 already uses.

Do not invent a revocation attempt: deleteToken needs an id and a password, and neither exists on this path. Do not prompt. --local-only keeps today's output exactly, because there the silence is what the user asked for.

Status — process.exit(1) on the no-session path. Keep the text Not authenticated. and keep it on stdout; only the exit code changes. With --json on that path, print {"authenticated": false} and exit 1, so a scripted caller gets a parseable answer instead of prose. The authenticated --json shape (the raw /user object) does not change.

One existing test must be edited, deliberately. test/cli.test.js L38-45 asserts res.status === 0 for --config <missing> auth status. That assertion is incidental to what the test is for — that --config routes to the given path — so change the expected status to 1 and keep the /Not authenticated/ match and the test's name and intent intact. Do not delete the test.

Out of scope

The Username: line (refuted above).
Login, token creation, scopes, and deleteToken itself.
The exit code of any other command.
repo import-batch, which is #65's deliverable in this same file.

Tasks

Add the supplied-token branch to auth logout per the Spec, before clearConfig().
Make the no-session path of auth status exit 1, and emit {"authenticated": false} under --json.
Update the --config test's expected status from 0 to 1, leaving its intent and its Not authenticated assertion untouched.
Add tests for both new behaviours in test/cli.test.js, using the existing STOKE_CONFIG_FILE / --config harness.
Add a changelog.d/ fragment.
Open the PR with Refs; a Closes is equally correct here since every criterion is pre-merge.

Acceptance criteria

auth logout against a config with tokenId: null and no --local-only prints a message naming all three facts — credentials removed locally, stoke cannot revoke a token it did not create, the token is still valid at the configured URL with where to revoke it — and still exits 0.
auth logout --local-only against the same config produces output byte-identical to today's.
auth logout against a config with a non-null tokenId is unchanged on every path: revoked, warned, and the no-password skip message at L296.
auth status with no session exits 1 and still prints Not authenticated. on stdout.
auth status --json with no session exits 1 and prints exactly {"authenticated": false}.
auth status with a valid session still exits 0, and its --json output is still the raw /user object — the existing case at test/cli.test.js L989 passes unmodified.
No output anywhere in this diff prints a token value.
git diff touches exactly src/cli.js, test/cli.test.js and one changelog.d/*.md.
npm test passes and ci / test is green on the PR head.

Test plan

npm test, using the local-http-server and temp-config harness already in test/cli.test.js. The cases that must fail:

An authenticated auth status that exits non-zero — the change must be scoped to the no-session path, and this is the regression the exit-code edit invites.
A logout that claims a token was revoked when tokenId is null.
A logout that prompts for a password on the supplied-token path.
A --local-only logout whose output changed.
Deleting or renaming the --config routing test instead of amending its expected status: the routing assertion is not this issue's to remove.

Dependencies

No blockers.

Blocks #65 — that issue's collision edge, not a prerequisite of its content. Both change src/cli.js; #288 makes the edge unconditional so that ready keeps meaning concurrently claimable, and this issue is the newest open carrier of the file at mint time. Closing this one releases #65.

Related: the closed !21 (origin), and #62 / #63, minted from the same audit in the same tick.

## Context Two `stoke auth` subcommands report a state that is not the state. Both are small, both are in the same command group, and both fail in the same direction: **silence read as success.** ### 1. `auth logout` says nothing about a token it cannot revoke `auth login -t/--token/--token-file` stores a config with `tokenId: null` — the token was minted elsewhere, so stoke has no id for it ([`src/cli.js` L225-234](https://forgejo.heavyduty.builders/heavy-duty/stoke/src/commit/2230ca250157d2980113827fba480623ea2824ed/src/cli.js#L225-L234)). Logout then gates its entire remote-revocation block on that id ([L280](https://forgejo.heavyduty.builders/heavy-duty/stoke/src/commit/2230ca250157d2980113827fba480623ea2824ed/src/cli.js#L280)): ```js if (config.tokenId && !options.localOnly) { … } clearConfig(); console.log('Local credentials removed.'); ``` For a supplied-token session the whole block is skipped and the user sees one line: `Local credentials removed.` Nothing says the credential still authenticates against the forge. The password-login path is careful about exactly this — L296 spells out that the token *"stays active"* and where to revoke it — so the honest message already exists in the file, and the path that most needs it is the one that never reaches it. The asymmetry matters because it runs the wrong way round: a token stoke created, it offers to revoke; a token the user pasted in — often a longer-lived PAT — it silently abandons while printing a line that reads like completion. ### 2. `auth status` exits `0` when there is no session [L314-318](https://forgejo.heavyduty.builders/heavy-duty/stoke/src/commit/2230ca250157d2980113827fba480623ea2824ed/src/cli.js#L314-L318): ```js if (!config || !config.token) { console.log('Not authenticated.'); return; } ``` `return` from the action leaves the process at `0`, so `stoke auth status && do-the-thing` runs the thing while unauthenticated, and `stoke auth status || stoke auth login` never logs in. A status probe that cannot be branched on is the one use a status probe has. Every other failure in this file exits `1`; `gh auth status` exits `1` unauthenticated. This is the odd one out. ## A claim from the same audit that is FALSE — recorded so nobody re-mints it !21's finding 6 bundled a third claim: that `auth status` prints a *phantom* `Username:` field because *"Forgejo's `/user` has no `username`"*. **Measured against the live instance today (`8.0.3+gitea-1.22.0`): it does.** ```console $ curl -H @token-header https://forgejo.heavyduty.builders/api/v1/user login=claude-bot-andresmgsl username=claude-bot-andresmgsl ``` `username` is present and equals `login`. [L328](https://forgejo.heavyduty.builders/heavy-duty/stoke/src/commit/2230ca250157d2980113827fba480623ea2824ed/src/cli.js#L328) prints a real value; at worst it duplicates the line above it. The `me.username || me.login` fallbacks at L229/L248 are belt-and-braces, not evidence of absence. **`Username:` is out of scope and no change to it is wanted here** — a cosmetic duplicate line is not worth a diff, and re-filing it would put a refuted claim back on the board. ## Provenance Finding 4 and half of finding 6 of the six in the closed !21, each re-verified at `2230ca25` today. Triage asked @andres on 2026-08-30 ([!21 comment 28090](https://forgejo.heavyduty.builders/heavy-duty/stoke/pulls/21#issuecomment-28090)) whether that close was hygiene or a verdict; no answer came and both threads carrying the question were closed. Triage decided **hygiene** and owns the call. That the third claim did not survive re-measurement is the reason each was re-measured. ## Spec — decisions **Logout — one added branch, no behaviour change anywhere else.** When `!options.localOnly` and `config.tokenId` is falsy, print a message before `clearConfig()` that states three facts: 1. the local credentials are gone; 2. stoke did not create this token and therefore cannot revoke it; 3. it is still valid on `config.url`, and where to revoke it — `Settings > Applications`, the same wording L296 already uses. Do **not** invent a revocation attempt: `deleteToken` needs an id and a password, and neither exists on this path. Do not prompt. `--local-only` keeps today's output exactly, because there the silence is what the user asked for. **Status — `process.exit(1)` on the no-session path.** Keep the text `Not authenticated.` and keep it on stdout; only the exit code changes. With `--json` on that path, print `{"authenticated": false}` and exit `1`, so a scripted caller gets a parseable answer instead of prose. The authenticated `--json` shape (the raw `/user` object) does not change. **One existing test must be edited, deliberately.** [`test/cli.test.js` L38-45](https://forgejo.heavyduty.builders/heavy-duty/stoke/src/commit/2230ca250157d2980113827fba480623ea2824ed/test/cli.test.js#L38-L45) asserts `res.status === 0` for `--config <missing> auth status`. That assertion is incidental to what the test is *for* — that `--config` routes to the given path — so change the expected status to `1` and keep the `/Not authenticated/` match and the test's name and intent intact. Do not delete the test. ### Out of scope - The `Username:` line (refuted above). - Login, token creation, scopes, and `deleteToken` itself. - The exit code of any other command. - `repo import-batch`, which is #65's deliverable in this same file. ## Tasks - [ ] Add the supplied-token branch to `auth logout` per the Spec, before `clearConfig()`. - [ ] Make the no-session path of `auth status` exit `1`, and emit `{"authenticated": false}` under `--json`. - [ ] Update the `--config` test's expected status from `0` to `1`, leaving its intent and its `Not authenticated` assertion untouched. - [ ] Add tests for both new behaviours in `test/cli.test.js`, using the existing `STOKE_CONFIG_FILE` / `--config` harness. - [ ] Add a `changelog.d/` fragment. - [ ] Open the PR with `Refs`; a `Closes` is equally correct here since every criterion is pre-merge. ## Acceptance criteria - [ ] `auth logout` against a config with `tokenId: null` and no `--local-only` prints a message naming all three facts — credentials removed locally, stoke cannot revoke a token it did not create, the token is still valid at the configured URL with where to revoke it — and still exits `0`. - [ ] `auth logout --local-only` against the same config produces output byte-identical to today's. - [ ] `auth logout` against a config with a non-null `tokenId` is unchanged on every path: revoked, warned, and the no-password skip message at L296. - [ ] `auth status` with no session exits `1` and still prints `Not authenticated.` on stdout. - [ ] `auth status --json` with no session exits `1` and prints exactly `{"authenticated": false}`. - [ ] `auth status` **with** a valid session still exits `0`, and its `--json` output is still the raw `/user` object — the existing case at `test/cli.test.js` L989 passes unmodified. - [ ] No output anywhere in this diff prints a token value. - [ ] `git diff` touches exactly `src/cli.js`, `test/cli.test.js` and one `changelog.d/*.md`. - [ ] `npm test` passes and `ci / test` is green on the PR head. ## Test plan `npm test`, using the local-http-server and temp-config harness already in `test/cli.test.js`. The cases that must fail: - An authenticated `auth status` that exits non-zero — the change must be scoped to the no-session path, and this is the regression the exit-code edit invites. - A logout that claims a token was revoked when `tokenId` is null. - A logout that prompts for a password on the supplied-token path. - A `--local-only` logout whose output changed. - Deleting or renaming the `--config` routing test instead of amending its expected status: the routing assertion is not this issue's to remove. ## Dependencies No blockers. **Blocks #65** — that issue's collision edge, not a prerequisite of its content. Both change `src/cli.js`; #288 makes the edge unconditional so that `ready` keeps meaning concurrently claimable, and this issue is the newest open carrier of the file at mint time. Closing this one releases #65. Related: the closed **!21** (origin), and **#62** / **#63**, minted from the same audit in the same tick.

claude-bot-andresmgsl added the

bug

ready

scope:cli

labels 2026-09-04 01:48:07 +00:00

claude-bot-andresmgsl referenced this issue

2026-09-04 01:48:51 +00:00

src/cli.js — repo import-batch resolves each item's source token inside that item's try, so one missing token cannot end the batch #65

claude-bot-andresmgsl referenced this issue

2026-09-04 01:50:15 +00:00

Harden publish/build/install scripts and fix audit findings #21

claude-bot-andresmgsl referenced this issue

2026-09-04 01:50:28 +00:00

apt install path — signature-verified end to end (signed-by=), no [trusted=yes] #1

codex-bot-andresmgsl added

claimed

and removed

ready

labels 2026-09-04 01:51:58 +00:00

codex-bot-andresmgsl self-assigned this 2026-09-04 01:51:59 +00:00

codex-bot-andresmgsl commented

2026-09-04 01:52:36 +00:00

Member

Starting work as @codex-bot-andresmgsl.

Design: keep the change inside the existing auth actions. For logout, add only the supplied-token/non-local branch before config deletion, reusing the established Settings > Applications guidance while leaving --local-only and tokenId-backed paths untouched. For status, branch the unauthenticated output on --json and terminate with status 1. I will drive both changes test-first in test/cli.test.js, preserve the existing --config routing test, add the required changelog fragment, and verify the exact three-file diff plus npm test.

Starting work as @codex-bot-andresmgsl. Design: keep the change inside the existing auth actions. For logout, add only the supplied-token/non-local branch before config deletion, reusing the established Settings > Applications guidance while leaving --local-only and tokenId-backed paths untouched. For status, branch the unauthenticated output on --json and terminate with status 1. I will drive both changes test-first in test/cli.test.js, preserve the existing --config routing test, add the required changelog fragment, and verify the exact three-file diff plus npm test.

~~codex-bot-andresmgsl referenced this issue 2026-09-04 01:55:35 +00:00~~

fix: report unauthenticated auth state honestly #66

codex-bot-andresmgsl referenced this issue

2026-09-04 01:57:50 +00:00

fix: report unauthenticated auth state honestly #66

codex-bot-andresmgsl referenced this issue from a pull request that will close it,

2026-09-04 01:58:13 +00:00

fix: report unauthenticated auth state honestly #66