Conversation
Several runtime surfaces relied on implicit trust or static policy where request-specific authority was needed. Require browser request proofs and single-use stream tickets, descendant ownership for transcript continuation, and approval for task gate recording. Enforce strict fleet known-host checks and reject unsupported fingerprint settings. Refuse discovered plugin name collisions and keep explicit overrides bound to their configured names. Use cryptographic bridge action tokens and scope release metadata credentials to the HTTPS GitHub API origin, refusing non-HTTPS metadata redirects. Validation (Cargo used CARGO_BUILD_JOBS=4 and CARGO_NET_OFFLINE=true): - cargo test -p codewhale-tui --lib -- runtime_surface_hardening_ fleet_host_ssh_command fleet_host_ssh_config tools::plugin::tests tools::tasks::tests::task_shell_wait_keeps runtime_api::web::tests runtime_api::auth::tests --test-threads=1 --nocapture test result: ok. 35 passed; 0 failed; 0 ignored; 0 measured; 13439 filtered out; finished in 0.09s - Final focused plugin diagnostics and browser bootstrap fixture check: cargo test -p codewhale-tui --lib -- runtime_surface_hardening_plugin_collisions_preserve_registered_tools web_bootstrap_sets_strict_cookie_once_and_preserves_v1_auth --test-threads=1 --nocapture test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 13472 filtered out; finished in 0.05s Browser bootstrap: needs socket run; its fixture explicitly skipped because the sandbox refuses loopback binding. Pure request-proof and ticket checks are included in the 35-test run above. - npm test --prefix integrations/bridge-core: 17 tests, 17 pass, 0 fail. - node --test crates/tui/tests/runtime_web_client.test.mjs npm/codewhale/test/release-assets.test.js: 53 tests, 53 pass, 0 fail. - The three focused JavaScript regressions each failed when their respective production protection was temporarily removed, then the fixes were restored. - cargo fmt --all; sh scripts/sync-changelog.sh; git diff --check. - OpenSSH offline configuration inspection confirmed strict host-key checking and the configured known-hosts file, including a path with spaces. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a9e4d8bc2
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Cannot load plugin tool '{}': name is already registered; use an explicit tool override", | ||
| crate::safe_label::SafeLabel::identifier(tool.name()) | ||
| ); | ||
| continue; |
There was a problem hiding this comment.
When a plugin declares a built-in or duplicate tool name, this branch only emits a tracing event and continues, while load_plugins returns () and configure_plugin_tools consequently reports successful initialization with that plugin silently absent. This makes a self-contained configuration error depend on whether the user inspects tracing output instead of failing startup/load as promised; return an error through the initialization path when a collision is detected.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
- Recover browser request proofs on cookie-authenticated same-origin or direct page loads; retain origin-scoped sessionStorage and document recovery limits. Keep bounded independent stream tickets for tabs connecting together. - Place IgnoreUnknown before KnownHostsCommand so OpenSSH 7.x clients retain strict known-host checks without rejecting the newer option. - Correct the SSH authentication example and protocol field documentation; document the host_key_fingerprint migration to known_hosts in the changelog. - Retry transient stream-ticket failures with capped backoff, stop on HTTP 401/403, and discard superseded or stopped connection attempts. Validation (all Cargo commands used CARGO_BUILD_JOBS=4 CARGO_NET_OFFLINE=true): - cargo test -p codewhale-tui --lib -- runtime_surface_review_ runtime_surface_hardening_ssh_requires_known_host_verification fleet_host_ssh_command fleet_host_ssh_config runtime_api::web::tests runtime_api::auth::tests --test-threads=1 test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 13461 filtered out; finished in 0.01s - node --test crates/tui/tests/runtime_web_client.test.mjs tests 42; pass 42; fail 0. - The proof recovery, independent-ticket and SSH compatibility checks fail when their respective production changes are removed. The corrected docs regression run against the old example reports: test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 13476 filtered out; finished in 0.06s It fails with the expected unsupported host_key_fingerprint error. - Each focused JavaScript regression fails without its corresponding change: page proof recovery, transient ticket retry, and pending-stream ownership. Each reports tests 1; pass 0; fail 1. All changes were restored afterward. - cargo fmt --all; sh scripts/sync-changelog.sh; sh scripts/sync-changelog.sh --check; git diff --check: passed. - OpenSSH 10.0p2 ssh -G checks preserve strict checking and a known-hosts path containing spaces, and confirm IgnoreUnknown handling. No native 7.x run. - web_bootstrap_sets_strict_cookie_once_and_preserves_v1_auth: needs socket run; the sandbox cannot bind the loopback listener. - cargo clippy -p codewhale-tui --all-targets -- -D warnings (run once): exit 101; could not compile `codewhale-tui` (lib test) due to 19 previous errors. All 19 diagnostics are clippy::too_many_arguments. Each reported function signature is unchanged from both the starting HEAD and HEAD^; these are pre-existing, unrelated signatures. No lint allowances were added. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+19
to
+27
| ### Security | ||
|
|
||
| - Harden local runtime browser sessions, fleet SSH trust, agent continuation | ||
| ownership, task gate approval, plugin tool registration, bridge action tokens, | ||
| and release metadata credential forwarding. Browser sessions recover across | ||
| reloads and new tabs, and stream tickets retry after transient failures. | ||
| Fleet SSH known-host checks support OpenSSH 7.x and later. SSH host configs | ||
| using `host_key_fingerprint` must migrate to `known_hosts` with verified host | ||
| keys; the unsupported fingerprint field now fails at configuration load. |
Contributor
There was a problem hiding this comment.
🔍 Changelog entries precede the merge
The repository reserves changelog edits for a batched commit on main. These branch edits can conflict with other pending PRs.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+571
to
+575
| Web fetches require the session cookie plus an origin-scoped request proof; | ||
| streams use a fresh single-use ticket. The initial redirect carries the proof | ||
| in a fragment, which the client removes and saves in origin-scoped | ||
| `sessionStorage`. On reload or in a second tab, an authenticated `GET /` also | ||
| embeds the proof in a meta tag when `Sec-Fetch-Site` is `same-origin` or `none` |
Contributor
There was a problem hiding this comment.
🔍 Bootstrap redirect documentation conflicts
The earlier bootstrap description still says the browser redirects to /; the new flow redirects to a proof-bearing fragment.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+264
to
+277
| if headers | ||
| .get("sec-fetch-site") | ||
| .is_some_and(|site| site == "same-origin" || site == "none") | ||
| && web.matches_session_cookie(headers.get(header::COOKIE).and_then(|v| v.to_str().ok())) | ||
| && let Some(proof) = web | ||
| .request_proof | ||
| .lock() | ||
| .unwrap_or_else(|p| p.into_inner()) | ||
| .as_deref() | ||
| { | ||
| html = html.replace( | ||
| "name=\"codewhale-web-request\" content=\"\"", | ||
| &format!("name=\"codewhale-web-request\" content=\"{proof}\""), | ||
| ); |
Contributor
There was a problem hiding this comment.
🟥 Direct navigation exposes the browser API proof
An authenticated direct navigation to / embeds the API proof when Sec-Fetch-Site is none. This makes the proof available to any same-user local browser navigation that can read the returned page.
Was this helpful? React with 👍 or 👎 to provide feedback.
Hmbown
pushed a commit
that referenced
this pull request
Sep 27, 2026
crates/tui/src/runtime_api/web.rs: #6601 (already merged) replaced the file-local constant_time_eq with the shared codewhale_core::secret_eq::constant_time_eq; #6681 kept the local helper and widened secured_asset's body parameter to impl IntoResponse. Took #6681's secured_asset signature and dropped the local helper, so every runtime_api surface keeps using the one shared comparison. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tightens several runtime and tool trust boundaries:
Local: focused codewhale-tui tests 35 passed; 0 failed (and 15 passed with real loopback sockets); bridge-core 17 passed; runtime web client + release-assets 53 passed; 0 failed.
🤖 Generated with Claude Code