[HemSoft]

Closes #329.

Codex app-server request errors and failed login notifications previously copied arbitrary CLI error text into Display and Debug diagnostics. A short token or authorization code survived the 200-character cap. Both mock-transport regression tests failed on main 66764b6f65964d1422e63aa5c575333b5bafb0e0.

Discard that text at the adapter boundary. Request refusals use a fixed local message; failed logins use the existing local sign-in failure message. CLI stdout/stderr, credentials, account state and authentication protocols are unchanged. No layout or interaction files change.

Validation

• Three in-memory transport regressions cover malformed login-start responses and across synthetic bearer tokens, access/refresh tokens, codes, cookies, multiline and oversized Unicode messages. Both Display and Debug must omit the sentinel.
• cargo test -p codexbar-providers --locked: passed, 364 unit tests and all integration/command-fixture tests; 5 pre-existing installed-provider tests ignored.
• cargo fmt --all --check: passed.
• cargo clippy --workspace --all-targets --locked -- -D warnings: passed on Laptop.
• cargo check -p codexbar-app --bin codexbar --locked: passed on Laptop without test-support.
• cargo test --workspace --locked: passed on Laptop headlessly, including the new mock-transport regressions.
• Independent SFL Round 2 review PASS on exact final head b4641be8189acbfef409342a3e305fa7783ef2a7, reviewed by Claude Opus 5.5 with no findings; authenticated unedited review and zero unresolved threads.
• The initial metrics gate reported a ratio drop after removing covered lines. A meaningful malformed login response regression was added in test-only commit b4641be; all four local Rust gates pass on b4641be8189acbfef409342a3e305fa7783ef2a7. All seven required hosted checks pass on that head, including metrics and mutation qualification. Package was not triggered by these paths. Guarded squash merge delivered 899bf3fe7c589ce45ca7ca96c9684d43e1f956d1.

Only invented synthetic data and mock responses were used. No real auth/config/vault files, provider calls, installed CLI sign-in or historical exposure reproduction. This source-security fix makes no rendered UI qualification claim. No application deployment or launch.

[HemSoft]

@sfl review

[hemsoft-sfl-reviewer]

✅ Verdict: PASS • Round 1

Confidence: 8/10 · Project risk: 4/10 (codexbar)

Review done by claude-opus-5-5 (adversarial to codex)

The PR stops copying Codex app-server JSON-RPC error.message and failed account/login/completed params.error text into AppServerError::Refused, uses fixed local messages instead, and adds mock-transport tests that check both Display and Debug output. The fix closes the leak on both paths with no Critical, High or Medium defects, so it is approvable. Two Low quality findings remain: a stale UI test and duplicated failure text in the sign-in dialog.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 2m 23s
Token count: Unavailable

Pass	🔴 Critical	🟠 High	🟡 Medium	⚪ Low
🔒 Security	0	0	0	0
✅ Accuracy	0	0	0	0
🧰 Quality & Maintainability	0	0	0	2
Total	0	0	0	2

Key Findings

1. ⚪ LOW — Quality & Maintainability: Failed Codex sign-in dialog now shows the same sentence twice. crates/codexbar-providers/src/codex_app_server.rs:267: Observed: codex_sign_in.rs:214 renders the Stage::Failed status as "The sign-in didn't complete." and puts err.to_string() in the sign-in-error alert. With the new fixed message, that alert always reads "Codex: The sign-in didn't complete." Trigger: any failed browser sign-in reported by Codex (success: false). Impact: the alert repeats the status line and carries no distinguishing diagnostic. Before, only the no-reason fallback did this. Issue #329's goal of 'keeping useful fixed diagnostics' is only partly met. This is inferred from the source; the dialog was not rendered. (non-blocking)
2. ⚪ LOW — Quality & Maintainability: UI test still asserts a Codex-supplied sign-in reason that production can no longer produce. crates/codexbar-providers/src/codex_app_server.rs:267: Observed: after this change, SignIn::finish always returns Refused("The sign-in didn't complete.") for a failed login. The unchanged test a_failed_codex_sign_in_says_why_and_changes_nothing in crates/codexbar-app/src/ui_tests.rs (around line 4582) still has its fake service return Refused("Login was cancelled in the browser") and asserts the dialog shows Codex: Login was cancelled in the browser. Trigger: the test passes because it goes through the fake PendingSignIn, not the real adapter. Impact: the UI suite now documents and protects a CLI-reason display that the security fix intentionally removed. A later reader could take that as the expected behavior, and the test does not cover the message users will actually see. (non-blocking)

Coverage

• Files reviewed:
  • crates/codexbar-providers/src/codex_app_server.rs
  • crates/codexbar-providers/tests/codex_error_privacy.rs
• Context read:
  • crates/codexbar-providers/src/codex.rs
  • crates/codexbar-providers/src/lib.rs
  • crates/codexbar-providers/src/error.rs
  • crates/codexbar-providers/Cargo.toml
  • Cargo.toml
  • crates/codexbar-app/src/codex_sign_in.rs
  • crates/codexbar-app/src/ui_tests.rs
  • AGENTS.md
  • issue #329
  • PR #330 description
• Verified:
  • Compare base_commit.sha = 66764b6f65964d1422e63aa5c575333b5bafb0e0 and last commit = 7126f17f096cd37ed491a75e991cbaaee36570a8; merge base equals base, ahead_by 1, behind_by 0
  • Full diff covers two files: +11/-31 in codex_app_server.rs, and the new codex_error_privacy.rs (+61)
  • AppServer::request maps any error member to refused(), a fixed string; no CLI text reaches Refused on that path
  • SignIn::finish returns a fixed Refused message whenever success is not true; params.error is never read
  • The rest of the head module stores no CLI-provided text in AppServerError; MAX_ERROR and the old refused(Option<&Value>) have no leftover references
  • The integration test uses only public API: codex_app_server is pub mod, and AppServer::over, Transport, begin_sign_in and finish are pub
  • The new integration test is auto-discovered: the [[test]] command_runner entry does not set autotests = false, and serde_json is a normal dependency
  • Production callers: CodexProvider::renew uses only is_ok(); the sign-in and sign-out UI show err.to_string(), which now contains only fixed text
  • FakeRenewer in codex.rs still builds Refused("refresh token expired") locally; it is test-only and no production code matches on that text
  • Updated unit tests in the module are consistent with the new fixed messages
  • Workspace lints are only clippy::all warn and unsafe_op_in_unsafe_fn deny; nothing in the diff obviously trips them
• Not verified:
  • Runtime and test results; the PR's claims of passing cargo test, fmt and clippy were not checked because execution is forbidden
  • Rendered dialog appearance; the duplicated-text finding comes from reading source only
  • Whether real Codex CLI error payloads contain credentials; that premise comes from issue #329 and was not reproduced
  • Hosted CI check status on the head SHA was not queried
• Tests run: None. Reviewer execution is disabled.

---

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head 7126f17f096cd37ed491a75e991cbaaee36570a8 • SFL v0

[HemSoft]

Metrics failure diagnosed from the retained CI artifact: production line coverage moved from 21181/23961 to 21172/23951 after deleting covered sanitization code. No CRAP regression or unrelated coverage loss. Commit b4641be adds a mock-transport malformed login-start response case, rejecting credential-bearing fields without echoing them; production code is unchanged. All gates are re-running. Exact current-head independent review remains required by this assignment.

[HemSoft]

@sfl review

[hemsoft-sfl-reviewer]

✅ Verdict: PASS • Round 2

Confidence: 9/10 · Project risk: 4/10 (codexbar)

Since round 1

New: 0; still open (repeated): 0; resolved: 2.
Resolved means no matching finding in this review, not independent proof of a fix.

New (0)

• None.

Still open (repeated) (0)

• None.

Resolved (2)

• LOW UI test still asserts a Codex-supplied sign-in reason that production can no longer produce at crates/codexbar-providers/src/codex_app_server.rs:267
• LOW Failed Codex sign-in dialog now shows the same sentence twice at crates/codexbar-providers/src/codex_app_server.rs:267

Review done by claude-opus-5-5 (adversarial to codex)

The PR stops copying Codex app-server request-error and failed-login text into AppServerError::Refused and uses fixed local messages instead. It also adds integration tests with synthetic credentials covering request errors, failed logins and malformed login-start responses. Every remaining Refused construction in the module uses a local literal, and the UI callers show only Display output, so no evidenced Critical, High or Medium defect remains.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 1m 10s
Token count: Unavailable

Pass	🔴 Critical	🟠 High	🟡 Medium	⚪ Low
🔒 Security	0	0	0	0
✅ Accuracy	0	0	0	0
🧰 Quality & Maintainability	0	0	0	0
Total	0	0	0	0

Key Findings

No material defects found.

Coverage

• Files reviewed:
  • crates/codexbar-providers/src/codex_app_server.rs
  • crates/codexbar-providers/tests/codex_error_privacy.rs
• Context read:
  • crates/codexbar-providers/src/lib.rs
  • crates/codexbar-providers/src/codex.rs
  • crates/codexbar-providers/Cargo.toml
  • crates/codexbar-app/src/codex_sign_in.rs
  • crates/codexbar-providers/tests/claude_quota_reset.rs
  • AGENTS.md
• Verified:
  • Compare base_commit.sha is 66764b6f65964d1422e63aa5c575333b5bafb0e0, the last commit is b4641be8189acbfef409342a3e305fa7783ef2a7, and the head is 2 commits ahead and 0 behind
  • PR base and head SHAs from the pulls API match the reserved SHAs
  • request maps any error member to refused() without reading the error payload (codex_app_server.rs:159)
  • SignIn::finish failure uses only the fixed message 'The sign-in didn't complete.' (codex_app_server.rs:267)
  • No remaining reference to MAX_ERROR or the old refused(Option<&Value>) signature at head
  • Every AppServerError::Refused construction in the module uses a string literal (lines 120, 267, 276)
  • begin_sign_in returns Failed without copying the result payload when authUrl is missing, matching the new malformed-response test
  • The integration test uses only public items (pub mod codex_app_server, pub fn over, pub trait Transport) and needs no extra feature
  • Test trait bounds hold: unwrap_err on finish and renew has T=(), and let-else avoids needing Debug on SignIn
  • codex_sign_in.rs shows errors via err.to_string() (lines 292, 314, 366), so the UI receives only local text
  • CodexProvider::renew discards the renewal error via is_ok(), so it has no dependence on the removed Codex text
  • The new test file follows the AGENTS.md placement rule for cross-module provider tests and uses snake_case behavior names
• Not verified:
  • Runtime behavior and the CI or metrics gate results the PR describes; execution is forbidden
  • Whether other logging paths outside the files read record raw app-server messages; a code search covered only the default branch index, not the exact head
  • Whether users lose useful diagnostics now that Codex's own error text is dropped; that is a product judgment, not a checked defect
• Tests run: None. Reviewer execution is disabled.

---

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head b4641be8189acbfef409342a3e305fa7783ef2a7 • SFL v0