rasterstate/fj

Watch

Fork 0

Fix Fjord auth defaults and token scoping #251

Merged

stephen merged 1 commit from fix/250-auth-fixes into main

2026-08-13 19:18:46 +00:00

stephen commented

2026-08-13 18:41:16 +00:00

Owner

Fixes #250.

Fixed and Tested

Production Fjord default:
- Changed the default platform URL from the old preview URL to https://fjord.sh.
- Added HTML/non-JSON response detection on Fjord platform calls so a preview/static page failure names --platform-url instead of falling through as an opaque JSON decode error.
- Covered by fjord_default_platform_url_is_production, fjord_sign_in_html_response_points_at_platform_url, and device_authorization_html_response_points_at_platform_url.
FJ_TOKEN no longer blocks fj auth login --fjord:
- Removed clap's env binding from auth login --token.
- Runtime login mode resolution now lets explicit --fjord win over generic FJ_TOKEN.
- Covered by fjord_login_parses_when_generic_env_token_is_set and fjord_mode_wins_over_generic_env_token.
Generic FJ_TOKEN is no longer cross-host:
- load_token(host) now loads persisted credentials only.
- Runtime client resolution handles env tokens with host context.
- Generic FJ_TOKEN is accepted for the default/current-host path, including when the user explicitly spells that same host with --host.
- Generic FJ_TOKEN is ignored when explicit --host X targets a different configured host.
- Host-scoped FJ_TOKEN_<HOST> is accepted for explicit hosts. The host suffix now uses an injective H + uppercase hex encoding of the host bytes, for example rasterhub.com maps to FJ_TOKEN_H7261737465726875622E636F6D. I chose injective encoding over ambiguity rejection so CI can use a host-scoped variable without first enumerating all configured hosts, while still making it impossible for distinct host strings such as foo-bar.com and foo.bar.com to silently share a variable.
- Covered by resolver tests for default generic env token, explicit same-host generic env token, explicit different-host generic env rejection, and explicit host-scoped env token.
FJ_SESSION sibling:
- Removed clap's env binding from auth session --cookie, so FJ_SESSION cannot synthesize --cookie and conflict with --clear.
- Added host-scoped FJ_SESSION_<HOST> using the same injective host-byte hex suffix as FJ_TOKEN_<HOST>.
- Generic FJ_SESSION is accepted only for the default/current host or an explicit --host naming that same host; different explicit hosts need FJ_SESSION_<HOST> or persisted session storage.
- Covered by host_scoped_env_session_is_named_from_host, host_scoped_env_vars_do_not_collapse_punctuation, and session_clear_parses_when_generic_env_session_is_set.
Global auth host propagation:
- Global --host is now propagated into fj auth status, token, refresh, session, logout, and setup-git.
- fj auth status --host X reports only host X, and token/session presence uses the same same-host/host-scoped env rules as client resolution.
Git fixture pre-push failure:
- The earlier full-suite failure was real: git fixture tests inherited hook Git env such as GIT_DIR/GIT_WORK_TREE, so temp-repo commands could operate on the PR checkout.
- Sanitized repository-scoped Git environment variables in the git command wrappers and fixture helpers.
- Verified with a poisoned-env reproducer for git::repo_tests::fetch_branch_reads_the_remote_tip_without_moving_local_refs.

Audit Checked and Cleared

Checked Fjord platform decode paths touched by login and instance listing:
- sign_in
- OIDC strategy probe
- OAuth token exchange and refresh
- device authorization
- device token polling
- instance listing
Checked clap env bindings for FJ_TOKEN and FJ_SESSION; both are now runtime-resolved instead of clap-synthesized secret arguments.
Checked token/session resolution for default/current host versus explicit same-host versus explicit different-host. Also pinned the old collision shape directly: foo-bar.com and foo.bar.com no longer derive the same token or session env var.
Checked global auth command host propagation.
Checked the pre-push fixture failure and fixed it rather than leaving it as an aside.

Still Not Claimed

Documentation still needs a follow-up pass. Some README/docs text still describes generic FJ_TOKEN/FJ_SESSION as always checked before persistent stores; this PR changes explicit different-host behavior.
I did not complete a manual audit of every non-auth command that may indirectly load credentials beyond the resolver/status/session paths changed here.

Verification

cargo fmt --all -- --check
git diff --check
RUSTC_WRAPPER= cargo check
RUSTC_WRAPPER= cargo clippy -- -D warnings
RUSTC_WRAPPER= cargo test client::resolve::tests -- --nocapture
RUSTC_WRAPPER= cargo test auth::tests -- --nocapture
RUSTC_WRAPPER= cargo test cli::tests::session_clear_parses_when_generic_env_session_is_set -- --nocapture
RUSTC_WRAPPER= cargo test cli::tests::fjord_login_parses_when_generic_env_token_is_set -- --nocapture
GIT_DIR=/home/dev/workspaces/codex-1/fj-250-auth-fixes-clean/.git GIT_WORK_TREE=/home/dev/workspaces/codex-1/fj-250-auth-fixes-clean RUSTC_WRAPPER= cargo test git::repo_tests::fetch_branch_reads_the_remote_tip_without_moving_local_refs -- --nocapture
RUST_TEST_THREADS=1 RUSTC_WRAPPER= cargo test passed: 765 unit tests passed, 2 ignored; version integration test passed.

Fixes #250. ## Fixed and Tested 1. Production Fjord default: - Changed the default platform URL from the old preview URL to `https://fjord.sh`. - Added HTML/non-JSON response detection on Fjord platform calls so a preview/static page failure names `--platform-url` instead of falling through as an opaque JSON decode error. - Covered by `fjord_default_platform_url_is_production`, `fjord_sign_in_html_response_points_at_platform_url`, and `device_authorization_html_response_points_at_platform_url`. 2. `FJ_TOKEN` no longer blocks `fj auth login --fjord`: - Removed clap's env binding from `auth login --token`. - Runtime login mode resolution now lets explicit `--fjord` win over generic `FJ_TOKEN`. - Covered by `fjord_login_parses_when_generic_env_token_is_set` and `fjord_mode_wins_over_generic_env_token`. 3. Generic `FJ_TOKEN` is no longer cross-host: - `load_token(host)` now loads persisted credentials only. - Runtime client resolution handles env tokens with host context. - Generic `FJ_TOKEN` is accepted for the default/current-host path, including when the user explicitly spells that same host with `--host`. - Generic `FJ_TOKEN` is ignored when explicit `--host X` targets a different configured host. - Host-scoped `FJ_TOKEN_<HOST>` is accepted for explicit hosts. The host suffix now uses an injective `H` + uppercase hex encoding of the host bytes, for example `rasterhub.com` maps to `FJ_TOKEN_H7261737465726875622E636F6D`. I chose injective encoding over ambiguity rejection so CI can use a host-scoped variable without first enumerating all configured hosts, while still making it impossible for distinct host strings such as `foo-bar.com` and `foo.bar.com` to silently share a variable. - Covered by resolver tests for default generic env token, explicit same-host generic env token, explicit different-host generic env rejection, and explicit host-scoped env token. 4. `FJ_SESSION` sibling: - Removed clap's env binding from `auth session --cookie`, so `FJ_SESSION` cannot synthesize `--cookie` and conflict with `--clear`. - Added host-scoped `FJ_SESSION_<HOST>` using the same injective host-byte hex suffix as `FJ_TOKEN_<HOST>`. - Generic `FJ_SESSION` is accepted only for the default/current host or an explicit `--host` naming that same host; different explicit hosts need `FJ_SESSION_<HOST>` or persisted session storage. - Covered by `host_scoped_env_session_is_named_from_host`, `host_scoped_env_vars_do_not_collapse_punctuation`, and `session_clear_parses_when_generic_env_session_is_set`. 5. Global auth host propagation: - Global `--host` is now propagated into `fj auth status`, `token`, `refresh`, `session`, `logout`, and `setup-git`. - `fj auth status --host X` reports only host `X`, and token/session presence uses the same same-host/host-scoped env rules as client resolution. 6. Git fixture pre-push failure: - The earlier full-suite failure was real: git fixture tests inherited hook Git env such as `GIT_DIR`/`GIT_WORK_TREE`, so temp-repo commands could operate on the PR checkout. - Sanitized repository-scoped Git environment variables in the git command wrappers and fixture helpers. - Verified with a poisoned-env reproducer for `git::repo_tests::fetch_branch_reads_the_remote_tip_without_moving_local_refs`. ## Audit Checked and Cleared - Checked Fjord platform decode paths touched by login and instance listing: - `sign_in` - OIDC strategy probe - OAuth token exchange and refresh - device authorization - device token polling - instance listing - Checked clap env bindings for `FJ_TOKEN` and `FJ_SESSION`; both are now runtime-resolved instead of clap-synthesized secret arguments. - Checked token/session resolution for default/current host versus explicit same-host versus explicit different-host. Also pinned the old collision shape directly: `foo-bar.com` and `foo.bar.com` no longer derive the same token or session env var. - Checked global auth command host propagation. - Checked the pre-push fixture failure and fixed it rather than leaving it as an aside. ## Still Not Claimed - Documentation still needs a follow-up pass. Some README/docs text still describes generic `FJ_TOKEN`/`FJ_SESSION` as always checked before persistent stores; this PR changes explicit different-host behavior. - I did not complete a manual audit of every non-auth command that may indirectly load credentials beyond the resolver/status/session paths changed here. ## Verification - `cargo fmt --all -- --check` - `git diff --check` - `RUSTC_WRAPPER= cargo check` - `RUSTC_WRAPPER= cargo clippy -- -D warnings` - `RUSTC_WRAPPER= cargo test client::resolve::tests -- --nocapture` - `RUSTC_WRAPPER= cargo test auth::tests -- --nocapture` - `RUSTC_WRAPPER= cargo test cli::tests::session_clear_parses_when_generic_env_session_is_set -- --nocapture` - `RUSTC_WRAPPER= cargo test cli::tests::fjord_login_parses_when_generic_env_token_is_set -- --nocapture` - `GIT_DIR=/home/dev/workspaces/codex-1/fj-250-auth-fixes-clean/.git GIT_WORK_TREE=/home/dev/workspaces/codex-1/fj-250-auth-fixes-clean RUSTC_WRAPPER= cargo test git::repo_tests::fetch_branch_reads_the_remote_tip_without_moving_local_refs -- --nocapture` - `RUST_TEST_THREADS=1 RUSTC_WRAPPER= cargo test` passed: 765 unit tests passed, 2 ignored; version integration test passed.

stephen added 2 commits

2026-08-13 18:41:16 +00:00

Fix Fjord auth defaults and token scoping 8125ae629a

trunk

Forseti review / forseti review (advisory) (pull_request_target) Successful in 2m50s

Details

ci / check (pull_request) Successful in 11m4s

Details

ci / coverage (pull_request) Has been cancelled

Details

ci / live-e2e (pull_request) Has been cancelled

Details

cb6f3755ab

forgejo-actions commented

2026-08-13 18:44:06 +00:00

Superseded by Forseti review for 0ca48b279f8a.

Forseti review

1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet).

Findings

P1 trunk.txt:1 — A file trunk.txt containing only 't' has been added to the repository root. This appears to be an accidental commit unrelated to the auth fixes described in the PR. It should be removed.
P1 src/cli/auth_login.rs:766 — The PR's stated fix is that explicit --host X must ignore generic FJ_TOKEN and accept only host-scoped FJ_TOKEN_<HOST> or persisted credentials. However, read_token() at line 763-768 checks env_token() unconditionally when args.token is None, regardless of whether the login targets an explicit host. This means a user running fj auth login --host git.example.com with `FJ_TOKEN= REDACTED

Notes

trunk.txt:1 (P3, disagreement) — The PR adds a new root-level trunk.txt containing only t, which is unrelated to the auth and Fjord changes and looks like a stray artifact.
PR: rasterstate/fj#251
Head SHA: cb6f3755ab73
Review job: sha256:e087bfef536553ad535ad77109767495cb4c09bb72e81aa6dd23fa514dc58211
Provider pair: openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001
Blocking findings: 1
Inline findings: 2
Model tokens: 87303 in / 17321 out (36402 from cache) ≈ $0.5081
Token source: GITHUB_TOKEN
Runner: 54dcbdea9852
Run: https://rasterhub.com/rasterstate/fj/actions/runs/469

> Superseded by Forseti review for `0ca48b279f8a`. ## Forseti review 1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet). ### Findings - **P1** `trunk.txt:1` — A file `trunk.txt` containing only 't' has been added to the repository root. This appears to be an accidental commit unrelated to the auth fixes described in the PR. It should be removed. - **P1** `src/cli/auth_login.rs:766` — The PR's stated fix is that explicit `--host X` must ignore generic `FJ_TOKEN` and accept only host-scoped `FJ_TOKEN_<HOST>` or persisted credentials. However, `read_token()` at line 763-768 checks `env_token()` unconditionally when `args.token` is None, regardless of whether the login targets an explicit host. This means a user running `fj auth login --host git.example.com` with `FJ_TOKEN= ***REDACTED*** ### Notes - `trunk.txt:1` (P3, disagreement) — The PR adds a new root-level `trunk.txt` containing only `t`, which is unrelated to the auth and Fjord changes and looks like a stray artifact. - PR: `rasterstate/fj#251` - Head SHA: `cb6f3755ab73` - Review job: `sha256:e087bfef536553ad535ad77109767495cb4c09bb72e81aa6dd23fa514dc58211` - Provider pair: `openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001` - Blocking findings: `1` - Inline findings: `2` - Model tokens: `87303 in / 17321 out` (`36402` from cache) ≈ `$0.5081` - Token source: `GITHUB_TOKEN` - Runner: `54dcbdea9852` - Run: https://rasterhub.com/rasterstate/fj/actions/runs/469

forgejo-actions added the

forseti:changes-requested

label

2026-08-13 18:44:06 +00:00

forgejo-actions requested changes

2026-08-13 18:44:07 +00:00

Dismissed

forgejo-actions left a comment

Forseti review

1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet).

Findings

P1 trunk.txt:1 — A file trunk.txt containing only 't' has been added to the repository root. This appears to be an accidental commit unrelated to the auth fixes described in the PR. It should be removed.
P1 src/cli/auth_login.rs:766 — The PR's stated fix is that explicit --host X must ignore generic FJ_TOKEN and accept only host-scoped FJ_TOKEN_<HOST> or persisted credentials. However, read_token() at line 763-768 checks env_token() unconditionally when args.token is None, regardless of whether the login targets an explicit host. This means a user running fj auth login --host git.example.com with `FJ_TOKEN= REDACTED

Notes

trunk.txt:1 (P3, disagreement) — The PR adds a new root-level trunk.txt containing only t, which is unrelated to the auth and Fjord changes and looks like a stray artifact.
PR: rasterstate/fj#251
Head SHA: cb6f3755ab73
Review job: sha256:e087bfef536553ad535ad77109767495cb4c09bb72e81aa6dd23fa514dc58211
Provider pair: openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001
Blocking findings: 1
Inline findings: 2
Model tokens: 87303 in / 17321 out (36402 from cache) ≈ $0.5081
Token source: GITHUB_TOKEN
Runner: 54dcbdea9852
Run: https://rasterhub.com/rasterstate/fj/actions/runs/469

## Forseti review 1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet). ### Findings - **P1** `trunk.txt:1` — A file `trunk.txt` containing only 't' has been added to the repository root. This appears to be an accidental commit unrelated to the auth fixes described in the PR. It should be removed. - **P1** `src/cli/auth_login.rs:766` — The PR's stated fix is that explicit `--host X` must ignore generic `FJ_TOKEN` and accept only host-scoped `FJ_TOKEN_<HOST>` or persisted credentials. However, `read_token()` at line 763-768 checks `env_token()` unconditionally when `args.token` is None, regardless of whether the login targets an explicit host. This means a user running `fj auth login --host git.example.com` with `FJ_TOKEN= ***REDACTED*** ### Notes - `trunk.txt:1` (P3, disagreement) — The PR adds a new root-level `trunk.txt` containing only `t`, which is unrelated to the auth and Fjord changes and looks like a stray artifact. - PR: `rasterstate/fj#251` - Head SHA: `cb6f3755ab73` - Review job: `sha256:e087bfef536553ad535ad77109767495cb4c09bb72e81aa6dd23fa514dc58211` - Provider pair: `openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001` - Blocking findings: `1` - Inline findings: `2` - Model tokens: `87303 in / 17321 out` (`36402` from cache) ≈ `$0.5081` - Token source: `GITHUB_TOKEN` - Runner: `54dcbdea9852` - Run: https://rasterhub.com/rasterstate/fj/actions/runs/469

stephen commented

2026-08-13 18:44:54 +00:00

Author

Owner

Good fix, and the host-scoped FJ_TOKEN_<HOST> is the right shape. One change to make before this merges.

Naming the default host explicitly should not drop the token

src/client/resolve.rs:60:

if !explicit_host {
    if let Some(token) = auth::env_token() { return Ok(Some(token)); }
}
auth::load_token(name)

So fj api --host rasterhub.com repos/x/y skips generic FJ_TOKEN and falls back to persisted credentials, even though rasterhub.com is the current host. That is not a cross-host leak, it is the same host named explicitly, and the security property the flag exists to protect is not engaged.

Why it matters here specifically: fj#147's guidance is to export FJ_TOKEN from the shell profile precisely because the Secret Service keychain is not available in non-interactive shells on this fleet. On such a box load_token has nothing to fall back to, so an explicit --host rasterhub.com goes from working to unauthenticated. Any script or lane that names the host for clarity breaks, and it breaks as a 401, which reads as a bad token rather than a dropped one.

Suggested change: honour generic FJ_TOKEN when the explicit host resolves to the same host as the default. Reject it only when the target differs from the default, which is the actual leak case (--host commons.fjord.host sending the rasterhub PAT, the 401 in #250).

A test worth adding, since neither existing case covers it: generic FJ_TOKEN set, no persisted credential, --host <default-host> explicitly, expect the request to be authenticated. Assert on the same host being named, not merely that some explicit host is rejected; the current tests pass against this bug.

Two smaller things

The PR notes the pre-push run was skipped with --no-verify because the full cargo test hit git fixture lock and ref collisions. That reads like fixture contention under parallelism rather than something unrelated, and a suite that cannot pass locally is a suite nobody will run. Worth a look, or an issue if it is out of scope here, rather than leaving it as an aside.
FJ_SESSION is still host-agnostic and clap-bound, which was listed as not reached. That is the same defect class as the one being fixed, so it should not be left to be rediscovered by whoever hits it.

Good fix, and the host-scoped `FJ_TOKEN_<HOST>` is the right shape. One change to make before this merges. ## Naming the default host explicitly should not drop the token `src/client/resolve.rs:60`: ```rust if !explicit_host { if let Some(token) = auth::env_token() { return Ok(Some(token)); } } auth::load_token(name) ``` So `fj api --host rasterhub.com repos/x/y` skips generic `FJ_TOKEN` and falls back to persisted credentials, even though `rasterhub.com` **is** the current host. That is not a cross-host leak, it is the same host named explicitly, and the security property the flag exists to protect is not engaged. Why it matters here specifically: fj#147's guidance is to export `FJ_TOKEN` from the shell profile precisely because the Secret Service keychain is not available in non-interactive shells on this fleet. On such a box `load_token` has nothing to fall back to, so an explicit `--host rasterhub.com` goes from working to unauthenticated. Any script or lane that names the host for clarity breaks, and it breaks as a 401, which reads as a bad token rather than a dropped one. **Suggested change:** honour generic `FJ_TOKEN` when the explicit host resolves to the same host as the default. Reject it only when the target differs from the default, which is the actual leak case (`--host commons.fjord.host` sending the rasterhub PAT, the 401 in #250). A test worth adding, since neither existing case covers it: generic `FJ_TOKEN` set, no persisted credential, `--host <default-host>` explicitly, expect the request to be authenticated. Assert on the same host being named, not merely that some explicit host is rejected; the current tests pass against this bug. ## Two smaller things - The PR notes the pre-push run was skipped with `--no-verify` because the full `cargo test` hit git fixture lock and ref collisions. That reads like fixture contention under parallelism rather than something unrelated, and a suite that cannot pass locally is a suite nobody will run. Worth a look, or an issue if it is out of scope here, rather than leaving it as an aside. - `FJ_SESSION` is still host-agnostic and clap-bound, which was listed as not reached. That is the same defect class as the one being fixed, so it should not be left to be rediscovered by whoever hits it.

stephen force-pushed fix/250-auth-fixes from cb6f3755ab

Forseti review / forseti review (advisory) (pull_request_target) Successful in 2m50s

Details

ci / check (pull_request) Successful in 11m4s

Details

ci / coverage (pull_request) Has been cancelled

Details

ci / live-e2e (pull_request) Has been cancelled

Details

to 0ca48b279f

Forseti review / forseti review (advisory) (pull_request_target) Successful in 3m25s

Details

ci / coverage (pull_request) Has been cancelled

Details

ci / live-e2e (pull_request) Has been cancelled

Details

ci / check (pull_request) Has been cancelled

Details

2026-08-13 18:54:10 +00:00

Compare

forgejo-actions commented

2026-08-13 18:57:33 +00:00

Superseded by Forseti review for b08c80331cb6.

Forseti review

1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet).

Findings

P1 src/auth/mod.rs:199 — env_token_for_host and host_env_session_var derive the environment variable name through host_env_suffix, which maps every non-ASCII-alphanumeric byte to _. Distinct valid hosts such as foo-bar.com and foo.bar.com therefore both map to FJ_TOKEN_FOO_BAR_COM / FJ_SESSION_FOO_BAR_COM. Because these host-scoped variables are now trusted even for explicit host selection, a credential intended for one host can be sent to a different colliding host, undermining the token-scoping fix.
PR: rasterstate/fj#251
Head SHA: 0ca48b279f8a
Review job: sha256:185c017d9cf55abcc052b09f4d4c4db4fd0aaca6be9d691b8c14512d0de0cb50
Provider pair: openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001
Blocking findings: 1
Inline findings: 1
Model tokens: 117984 in / 14482 out (46642 from cache) ≈ $0.5702
Token source: GITHUB_TOKEN
Runner: 58fc327bb1c7
Run: https://rasterhub.com/rasterstate/fj/actions/runs/471

> Superseded by Forseti review for `b08c80331cb6`. ## Forseti review 1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet). ### Findings - **P1** `src/auth/mod.rs:199` — `env_token_for_host` and `host_env_session_var` derive the environment variable name through `host_env_suffix`, which maps every non-ASCII-alphanumeric byte to `_`. Distinct valid hosts such as `foo-bar.com` and `foo.bar.com` therefore both map to `FJ_TOKEN_FOO_BAR_COM` / `FJ_SESSION_FOO_BAR_COM`. Because these host-scoped variables are now trusted even for explicit host selection, a credential intended for one host can be sent to a different colliding host, undermining the token-scoping fix. - PR: `rasterstate/fj#251` - Head SHA: `0ca48b279f8a` - Review job: `sha256:185c017d9cf55abcc052b09f4d4c4db4fd0aaca6be9d691b8c14512d0de0cb50` - Provider pair: `openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001` - Blocking findings: `1` - Inline findings: `1` - Model tokens: `117984 in / 14482 out` (`46642` from cache) ≈ `$0.5702` - Token source: `GITHUB_TOKEN` - Runner: `58fc327bb1c7` - Run: https://rasterhub.com/rasterstate/fj/actions/runs/471

forgejo-actions requested changes

2026-08-13 18:57:35 +00:00

Dismissed

forgejo-actions left a comment

Forseti review

1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet).

Findings

P1 src/auth/mod.rs:199 — env_token_for_host and host_env_session_var derive the environment variable name through host_env_suffix, which maps every non-ASCII-alphanumeric byte to _. Distinct valid hosts such as foo-bar.com and foo.bar.com therefore both map to FJ_TOKEN_FOO_BAR_COM / FJ_SESSION_FOO_BAR_COM. Because these host-scoped variables are now trusted even for explicit host selection, a credential intended for one host can be sent to a different colliding host, undermining the token-scoping fix.
PR: rasterstate/fj#251
Head SHA: 0ca48b279f8a
Review job: sha256:185c017d9cf55abcc052b09f4d4c4db4fd0aaca6be9d691b8c14512d0de0cb50
Provider pair: openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001
Blocking findings: 1
Inline findings: 1
Model tokens: 117984 in / 14482 out (46642 from cache) ≈ $0.5702
Token source: GITHUB_TOKEN
Runner: 58fc327bb1c7
Run: https://rasterhub.com/rasterstate/fj/actions/runs/471

## Forseti review 1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet). ### Findings - **P1** `src/auth/mod.rs:199` — `env_token_for_host` and `host_env_session_var` derive the environment variable name through `host_env_suffix`, which maps every non-ASCII-alphanumeric byte to `_`. Distinct valid hosts such as `foo-bar.com` and `foo.bar.com` therefore both map to `FJ_TOKEN_FOO_BAR_COM` / `FJ_SESSION_FOO_BAR_COM`. Because these host-scoped variables are now trusted even for explicit host selection, a credential intended for one host can be sent to a different colliding host, undermining the token-scoping fix. - PR: `rasterstate/fj#251` - Head SHA: `0ca48b279f8a` - Review job: `sha256:185c017d9cf55abcc052b09f4d4c4db4fd0aaca6be9d691b8c14512d0de0cb50` - Provider pair: `openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001` - Blocking findings: `1` - Inline findings: `1` - Model tokens: `117984 in / 14482 out` (`46642` from cache) ≈ `$0.5702` - Token source: `GITHUB_TOKEN` - Runner: `58fc327bb1c7` - Run: https://rasterhub.com/rasterstate/fj/actions/runs/471

stephen force-pushed fix/250-auth-fixes from 0ca48b279f

Forseti review / forseti review (advisory) (pull_request_target) Successful in 3m25s

Details

ci / coverage (pull_request) Has been cancelled

Details

ci / live-e2e (pull_request) Has been cancelled

Details

ci / check (pull_request) Has been cancelled

Details

to b08c80331c

Forseti review / forseti review (advisory) (pull_request_target) Successful in 3m21s

Details

ci / check (pull_request) Successful in 10m46s

Details

ci / live-e2e (pull_request) Successful in 2m8s

Details

ci / coverage (pull_request) Successful in 3m5s

Details

2026-08-13 19:03:24 +00:00

Compare

forgejo-actions commented

2026-08-13 19:06:44 +00:00

Forseti review

No blocking findings from the lead reviewer.

Findings

P2 src/cli/auth.rs:381 — When fj auth status is run without a global --host, selected contains every configured host, but generic_env_allowed_for_resolved_host returns true whenever the explicit host argument is None. That means the loop treats generic FJ_TOKEN/FJ_SESSION as present for every PAT host, even non-current hosts where the new resolver would ignore the generic env credential if targeted explicitly. This makes status output disagree with the new same-host/host-scoped credential rules and can label a token/session as available for the wrong host.
P2 src/cli/mod.rs:324 — These tests mutate process-wide FJ_TOKEN/FJ_SESSION directly and restore them afterward, but Rust unit tests run in parallel by default. Other new tests also read or write FJ_TOKEN, so concurrent execution can observe another test's temporary value or restore over it, producing flaky or misleading results. The resolver tests added a mutex, but it is local to that module and does not protect these mutations.
PR: rasterstate/fj#251
Head SHA: b08c80331cb6
Review job: sha256:9d6f88fd63e6fc3b123d66074a6a56c9f7a337cadacae78a04b27008c1ece358
Provider pair: openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001
Blocking findings: 0
Inline findings: 2
Model tokens: 119916 in / 15065 out (46642 from cache) ≈ $0.5945
Token source: GITHUB_TOKEN
Runner: dcef557ae26e
Run: https://rasterhub.com/rasterstate/fj/actions/runs/473

## Forseti review No blocking findings from the lead reviewer. ### Findings - **P2** `src/cli/auth.rs:381` — When `fj auth status` is run without a global `--host`, `selected` contains every configured host, but `generic_env_allowed_for_resolved_host` returns `true` whenever the explicit host argument is `None`. That means the loop treats generic `FJ_TOKEN`/`FJ_SESSION` as present for every PAT host, even non-current hosts where the new resolver would ignore the generic env credential if targeted explicitly. This makes status output disagree with the new same-host/host-scoped credential rules and can label a token/session as available for the wrong host. - **P2** `src/cli/mod.rs:324` — These tests mutate process-wide `FJ_TOKEN`/`FJ_SESSION` directly and restore them afterward, but Rust unit tests run in parallel by default. Other new tests also read or write `FJ_TOKEN`, so concurrent execution can observe another test's temporary value or restore over it, producing flaky or misleading results. The resolver tests added a mutex, but it is local to that module and does not protect these mutations. - PR: `rasterstate/fj#251` - Head SHA: `b08c80331cb6` - Review job: `sha256:9d6f88fd63e6fc3b123d66074a6a56c9f7a337cadacae78a04b27008c1ece358` - Provider pair: `openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001` - Blocking findings: `0` - Inline findings: `2` - Model tokens: `119916 in / 15065 out` (`46642` from cache) ≈ `$0.5945` - Token source: `GITHUB_TOKEN` - Runner: `dcef557ae26e` - Run: https://rasterhub.com/rasterstate/fj/actions/runs/473

forgejo-actions approved these changes

2026-08-13 19:06:45 +00:00

forgejo-actions left a comment

Forseti review

No blocking findings from the lead reviewer.

Findings

P2 src/cli/auth.rs:381 — When fj auth status is run without a global --host, selected contains every configured host, but generic_env_allowed_for_resolved_host returns true whenever the explicit host argument is None. That means the loop treats generic FJ_TOKEN/FJ_SESSION as present for every PAT host, even non-current hosts where the new resolver would ignore the generic env credential if targeted explicitly. This makes status output disagree with the new same-host/host-scoped credential rules and can label a token/session as available for the wrong host.
P2 src/cli/mod.rs:324 — These tests mutate process-wide FJ_TOKEN/FJ_SESSION directly and restore them afterward, but Rust unit tests run in parallel by default. Other new tests also read or write FJ_TOKEN, so concurrent execution can observe another test's temporary value or restore over it, producing flaky or misleading results. The resolver tests added a mutex, but it is local to that module and does not protect these mutations.
PR: rasterstate/fj#251
Head SHA: b08c80331cb6
Review job: sha256:9d6f88fd63e6fc3b123d66074a6a56c9f7a337cadacae78a04b27008c1ece358
Provider pair: openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001
Blocking findings: 0
Inline findings: 2
Model tokens: 119916 in / 15065 out (46642 from cache) ≈ $0.5945
Token source: GITHUB_TOKEN
Runner: dcef557ae26e
Run: https://rasterhub.com/rasterstate/fj/actions/runs/473