Skip to content

feat(core): pass in-process RPC bearer via internal handle, not process env#2709

Open
oxoxDev wants to merge 12 commits into
tinyhumansai:mainfrom
oxoxDev:feat/core-token-internal-transport
Open

feat(core): pass in-process RPC bearer via internal handle, not process env#2709
oxoxDev wants to merge 12 commits into
tinyhumansai:mainfrom
oxoxDev:feat/core-token-internal-transport

Conversation

@oxoxDev
Copy link
Copy Markdown
Contributor

@oxoxDev oxoxDev commented May 26, 2026

Summary

  • Stop publishing the in-process core's per-launch RPC bearer token as a process-global environment variable inside the Tauri shell.
  • Pass the bearer to the embedded server through an internal Option<Arc<String>> handoff instead.
  • Standalone CLI / Docker / cloud-deploy paths that use OPENHUMAN_CORE_TOKEN as configuration are unchanged.

Problem

Post-#1061 the core runs as an in-process tokio task inside the Tauri shell — it is not a child sidecar anymore. The Tauri shell, however, still hands the per-launch RPC bearer to the embedded core by setting OPENHUMAN_CORE_TOKEN as a process-global env var (a leftover from the sidecar era):

// app/src-tauri/src/core_process.rs (pre-change)
std::env::set_var("OPENHUMAN_CORE_TOKEN", self.rpc_token.as_str());

A process-global env var is the wrong transport for an in-process secret. The bearer is then visible to:

  • any other process running as the same UID on macOS (sysctl(KERN_PROCARGS2) / ps eww -p <pid> — no TCC prompt, no entitlement, no Accessibility / Full Disk Access requirement);
  • any other process running as the same UID on Linux without hidepid configured (/proc/<pid>/environ).

The bearer check itself is correct (constant-time, matches a per-launch hex secret). The problem is purely the transport.

docs/SECURITY_AUDIT.md lines 167 + 186 already track this as an open item ("Review OPENHUMAN_CORE_TOKEN lifetime and exposure scope").

Solution

  1. src/core/auth.rs — add a new in-memory init path:
    pub fn init_rpc_token_with_value(token: &str) -> anyhow::Result<()>
    Sets the RPC_TOKEN OnceLock directly. No env read, no file read. The existing init_rpc_token env+file fallback for the standalone CLI / Docker / cloud is untouched.
  2. src/core/jsonrpc.rs — extend run_server_embedded_with_ready + run_server_inner to accept Option<Arc<String>>. When the embedded entry is called from the Tauri shell, pass Some(rpc_token); the new helper seeds RPC_TOKEN before any reader runs. When None (standalone CLI / Docker / cloud), fall back to the existing init_rpc_token(workspace_dir) behavior.
  3. app/src-tauri/src/core_process.rs — delete the std::env::set_var call; pass the token in via the extended embedded-entry signature. The token already lives in CoreProcessHandle.rpc_token: Arc<String> and is reachable in-memory; no env crossing is needed.
  4. src/core/jsonrpc.rs approval-gate session_id reader (:1843-1857) — replace std::env::var("OPENHUMAN_CORE_TOKEN") with crate::core::auth::get_rpc_token(). Same value, sourced from the in-memory RPC_TOKEN post-init. Audit-grouping continuity preserved.
  5. scripts/test-webhook-flow.sh — replace the ps eww token-extraction branch with a read of the existing debug-only token file at ${TMPDIR:-/tmp}/openhuman-e2e-rpc-token (already written by core_process.rs under #[cfg(debug_assertions)], mode 0600). Release-build users continue to set the env var themselves, matching the script's existing precedence.
  6. docs/SECURITY_AUDIT.md lines 167 + 186 — mark the audit-item resolved.

Init ordering verified

The approval-gate auth::get_rpc_token() read at src/core/jsonrpc.rs:1843 runs inside bootstrap_core_runtime(...) which is awaited from run_server_inner after init_rpc_token_with_value(...) / init_rpc_token(...). Same-task ordering — no race. The bearer is always populated by the time the read happens.

Preserved unchanged (legitimate env-as-config surfaces)

  • init_rpc_token env + file fallback at src/core/auth.rs — standalone CLI / Docker / cloud (fly secrets set OPENHUMAN_CORE_TOKEN=...) keep working byte-identically.
  • Public-bind warning at src/core/jsonrpc.rs:1417 — operator-supplied config, kept reading env directly.
  • Pairing diagnostics at src/openhuman/security/pairing.rs:19, :290, :320-348 — operator-supplied config, kept reading env directly.
  • Scripts that intentionally rely on env-as-config (print-core-token.sh, docker-entrypoint-core.sh, test-proactive-welcome.sh, test-subconscious-ticks.sh).
  • Tests at src/core/jsonrpc_tests.rs:125, src/openhuman/inference/http/tests.rs:31, tests/inference_provider_e2e.rs:32 (these exercise the CLI / standalone path).
  • core_rpc_token Tauri command at app/src-tauri/src/lib.rs:89-93 — already pulls from state.inner().rpc_token(), not env, so no change required.

Submission Checklist

  • cargo check --workspace clean
  • cargo check --manifest-path app/src-tauri/Cargo.toml clean
  • cargo fmt --all -- --check clean
  • cargo clippy --workspace --no-deps — zero new warnings on changed lines
  • cargo clippy --manifest-path app/src-tauri/Cargo.toml --no-deps — clean on core_process
  • cargo test --manifest-path app/src-tauri/Cargo.toml --lib core_process — 27 / 27 pass (including new ensure_running_does_not_publish_token_to_env regression guard)
  • cargo test --test json_rpc_e2e --no-run — compiles clean
  • N/A: i18n strings — no UI surface touched
  • N/A: frontend typecheck / lint / vitest — no TS surface touched
  • Release notes — bumped docs/SECURITY_AUDIT.md (item 6 marked resolved)

Note: cargo test --lib core::auth cannot run end-to-end on main right now because src/openhuman/config/ops_tests.rs:631 fails E0063: missing fields ... AutonomySettingsPatch on a clean 87f8ef47 (pre-existing, unrelated to this PR). cargo check --tests --lib compiles my auth.rs tests cleanly. A trivial ..Default::default() fix unblocks the full matrix.

Impact

  • Security: removes the leak primitive that lets any same-UID process recover the local RPC bearer via ps eww / KERN_PROCARGS2 / /proc/<pid>/environ. The local RPC surface is the agent control plane, so this is a meaningful local-attack-surface reduction.
  • Compatibility: standalone CLI (openhuman-core serve), Docker (docker-entrypoint-core.sh), and cloud-deploy (fly secrets set ...) paths are unchanged. They keep using OPENHUMAN_CORE_TOKEN as configuration via the existing init_rpc_token env + file fallback.
  • Dev workflow: scripts/test-webhook-flow.sh now reads the per-launch token from the debug-only mode-0600 file the Tauri shell already writes under #[cfg(debug_assertions)], instead of grepping ps eww output. No behavior change for release-build users.
  • Manual macOS verification: after launch, ps eww -p $(pgrep -x OpenHuman) | grep OPENHUMAN_CORE_TOKEN returns no matches; core_rpc_token Tauri command still returns the live bearer; authenticated RPC calls still succeed.

Related

  • Sibling defense-in-depth hardening — wallet-side prepared-quote owner binding lands in feat(wallet): bind prepared transaction quotes to originating chat session #2708.
  • Resolves an internal audit item tracked in docs/SECURITY_AUDIT.md (lines 167 + 186).
  • Follow-up out of scope: the standalone CLI's ~/.openhuman/core.token 0600 file has a similar latent footprint but is operator-managed configuration and lives outside this PR's scope.

AI Authored PR Metadata

  • AI authored: yes — implementation produced by a Claude-driven worktree-dev agent following a plan derived via sequential-thinking MCP.
  • Human review: all seven commits manually reviewed; tests run locally.

Summary by CodeRabbit

  • Security Improvements

    • Desktop app now keeps the core bearer token in-memory (not exposed via process env), reducing token exposure.
  • Documentation

    • Docs and guides updated to explain the in-memory token flow and clarify env-var tokens still apply for CLI/docker/cloud.
  • Tests

    • Added regression test ensuring desktop startup does not publish the token to the process environment.
  • Developer Tools / Scripts

    • Test webhook flow updated to use a debug token file and clearer debug guidance.

Review Change Stack

oxoxDev added 7 commits May 26, 2026 23:32
Same-process callers (the Tauri host now that the core runs in-process) can
seed the per-process RPC bearer directly into the OnceLock without writing
to OPENHUMAN_CORE_TOKEN in the environment. Avoiding the env crossing keeps
the bearer off /proc/<pid>/environ (Linux) and out of sysctl KERN_PROCARGS2
/ ps eww -p <pid> (macOS) where any same-UID process could otherwise read
it without entitlement.

Idempotent — second calls no-op, matching init_rpc_token. Module rustdoc
updated to enumerate all three init paths (in-memory / env / file).
…dded entry

Extend run_server_embedded_with_ready and the private run_server_inner to
accept an optional in-memory bearer token. When Some, run_server_inner
seeds the auth subsystem via init_rpc_token_with_value and skips the env
read; when None, behaviour is unchanged (init_rpc_token reads
OPENHUMAN_CORE_TOKEN from the env or generates a fresh token to
{workspace_dir}/core.token — the legitimate operator-supplied path for
CLI / docker / cloud deployments).

Existing call sites pass None to preserve current behaviour; the Tauri
shell switches to Some(handle.rpc_token.clone()) in the next commit.
The per-launch RPC bearer is now handed to the in-process core via the
in-memory rpc_token argument of run_server_embedded_with_ready, not by
setting OPENHUMAN_CORE_TOKEN on the process environment.

Sidecar-era env-var transport was a leftover from PR tinyhumansai#1061. With the
core in-process there is no child to receive the env, and sharing the
bearer via env placed it within reach of any same-UID process that
could read /proc/<pid>/environ (Linux) or sysctl KERN_PROCARGS2 /
ps eww -p <pid> (macOS).

CLI / docker / cloud env-as-config remains intact via the None branch
of run_server_inner — operators setting OPENHUMAN_CORE_TOKEN in
docker-compose / fly secrets / systemd units are unaffected.
… token

Replace std::env::var("OPENHUMAN_CORE_TOKEN") with
crate::core::auth::get_rpc_token() at the approval-gate session_id init.
Same bearer value, no env crossing — the OnceLock is the single source
of truth populated by init_rpc_token / init_rpc_token_with_value earlier
in run_server_inner, well before bootstrap_core_runtime runs.

Minimal swap only: session_id construction (token-as-id vs ephemeral
UUID) is unchanged; follow-up will redesign that surface.
…of ps eww

Replace the third resolution branch (ps eww -p <core_pid> grep for
OPENHUMAN_CORE_TOKEN=) with a read of the debug-only e2e token file at
${TMPDIR:-/tmp}/openhuman-e2e-rpc-token (mode 0600, only written under
#[cfg(debug_assertions)] from core_process.rs). The previous branch was
the only consumer of the env-var token-on-process-args surface; with the
Tauri shell no longer setting that env var, it would always return empty.

Env-var and workspace core.token branches retained for caller-supplied
(.env / docker) and standalone CLI workflows respectively.
Two new tests:

- core/auth#init_rpc_token_with_value_seeds_and_is_idempotent: confirms
  the in-memory handoff path populates the same OnceLock that
  get_rpc_token reads, and that a second call is a no-op (matches
  init_rpc_token semantics — flipping the bearer mid-life would 401
  every in-flight client).
- core/auth#init_rpc_token_with_value_rejects_empty: empty / whitespace
  tokens error rather than seeding the OnceLock with junk.
- app/src-tauri/core_process_tests#ensure_running_does_not_publish_token_to_env:
  after a successful ensure_running, OPENHUMAN_CORE_TOKEN must remain
  unset on the process env. Guards against the leak reintroducing
  itself through copy-paste or unrelated env-var work.
- SECURITY_AUDIT.md item 6 + checklist: env-var transport replaced with
  in-memory handoff on the desktop shell. CLI / docker / cloud
  env-as-config preserved as the documented operator surface.
- CLAUDE.md / AGENTS.md: wording updated where it previously described
  injecting OPENHUMAN_CORE_TOKEN before spawn — now describes
  in-memory handoff via run_server_embedded_with_ready(rpc_token:
  Some(_)). Operator-facing docker / cloud guidance unchanged.
- .env.example: Tauri-desktop line clarified — the shell does not read
  this variable; it generates its own per-launch bearer and hands it
  to the in-process core in memory.
@oxoxDev oxoxDev requested a review from a team May 26, 2026 18:40
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

📝 Walkthrough

Walkthrough

Move Tauri desktop RPC bearer handling from process-environment variables to an in-memory handoff: add a caller-seeded initializer, extend embedded server startup to accept an optional in-memory token, pass the token from the Tauri host in-memory, and update tests, scripts, and docs.

Changes

Desktop in-memory token handoff

Layer / File(s) Summary
Core in-memory token initializer
src/core/auth.rs
New init_rpc_token_with_value seeds the RPC bearer directly from caller-supplied strings with trimming/validation and idempotency. Tests verify empty-rejection and that re-initialization does not overwrite an already-seeded token.
JSON-RPC server embedded-core token parameter
src/core/jsonrpc.rs
run_server_embedded_with_ready and internal run_server_inner now accept rpc_token: Option<Arc<String>>; during bootstrap, prefer the provided token via init_rpc_token_with_value, falling back to env/file-based init. Approval-gate reads active bearer from auth subsystem instead of directly inspecting OPENHUMAN_CORE_TOKEN.
Tauri desktop token capture and in-memory passing
app/src-tauri/src/core_process.rs
Remove OPENHUMAN_CORE_TOKEN environment-variable setup from ensure_running; instead clone the internal token into token_for_core and pass it in-memory to run_server_embedded_with_ready(rpc_token: Some(_)). Updated doc comments explain the in-memory handoff mechanism.
Test: ensure_running does not set OPENHUMAN_CORE_TOKEN
app/src-tauri/src/core_process_tests.rs
New regression test verifies CoreProcessHandle::ensure_running() does not pollute the process environment with OPENHUMAN_CORE_TOKEN; samples env var before/after spawn with guard-based cleanup.
Configuration, scripts, and documentation updates
.env.example, scripts/test-webhook-flow.sh, AGENTS.md, CLAUDE.md, docs/SECURITY_AUDIT.md, src/openhuman/inference/http/tests.rs
.env.example clarifies the variable should be left blank on desktop. test-webhook-flow.sh now resolves the token from OPENHUMAN_CORE_TOKEN or the freshest of core.token and a debug e2e token file. Contributor docs updated to document in-memory handoff, token-file fallback for debugging, and mark prior env-exposure security concern resolved while noting env-as-config remains valid for non-desktop deployments.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • graycyrus
  • senamakel

"🐇
A token born fresh for every start,
Kept in memory, safe and smart,
The shell whispers it into the core,
No env to leak, no open door,
Hopping secrets kept secure."

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: moving RPC bearer token passing from process environment to internal in-memory handle for in-process core execution.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added feature Net-new user-facing capability or product behavior. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. working A PR that is being worked on by the team. labels May 26, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/SECURITY_AUDIT.md`:
- Line 167: Update Section 3 to remove the blanket statement that the per-launch
bearer is stored in OPENHUMAN_CORE_TOKEN and instead split the token-flow by
deployment: state that the Tauri/desktop shell no longer exports
OPENHUMAN_CORE_TOKEN to the process environment and performs an in-memory
handoff (use run_server_embedded_with_ready(rpc_token: Some(_)) as the
mechanism), while CLI/docker/cloud deployments continue to accept the bearer via
the OPENHUMAN_CORE_TOKEN env var (env-as-config). Ensure the wording references
OPENHUMAN_CORE_TOKEN and run_server_embedded_with_ready so readers can reconcile
the desktop vs. non-desktop behaviors.

In `@scripts/test-webhook-flow.sh`:
- Around line 17-25: The current token-resolution logic allows a stale workspace
core.token to take precedence over the debug-only TMP token, so update the
resolution order in scripts/test-webhook-flow.sh: first honor
OPENHUMAN_CORE_TOKEN env var, then check for and prefer the debug-only token
file (${TMPDIR:-/tmp}/openhuman-e2e-rpc-token) if it exists and is readable, and
only then fall back to the workspace core.token file; implement this by
reordering the existence/read checks and corresponding assignments (look for the
token-resolution block and variables OPENHUMAN_CORE_TOKEN, the
TMPDIR/openhuman-e2e-rpc-token path, and core.token) so a running Tauri debug
app's live bearer is used when present.

In `@src/core/auth.rs`:
- Around line 158-176: Update the rustdoc comments that still reference the old
Tauri shell env var so they match the new desktop init path: search for and edit
the docs around the constants and functions CORE_TOKEN_ENV_VAR and
init_rpc_token in this file to describe the current desktop boot flow (in-memory
handoff via init_rpc_token_with_value / CoreProcessHandle) instead of saying the
Tauri shell sets OPENHUMAN_CORE_TOKEN, and ensure the module header,
CORE_TOKEN_ENV_VAR docblock, and the init_rpc_token docblock consistently
describe the single current initialization path.

In `@src/core/jsonrpc.rs`:
- Around line 1286-1301: The host-binding security warning logic currently only
checks the OPENHUMAN_CORE_TOKEN env var, so callers passing an in-memory bearer
via rpc_token to run_server_embedded_with_ready are still warned; update the
explicit-token detection to treat rpc_token.is_some() as an explicit auth
configuration (i.e., change the has_explicit_token check used in
run_server_embedded_with_ready and the equivalent check in the related embedded
run_server path) so that when rpc_token is Some(_) and a non-loopback host is
bound, no false public-bind warning is emitted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0bb665c9-e503-4890-9d2e-5652cd1ecbe4

📥 Commits

Reviewing files that changed from the base of the PR and between 87f8ef4 and 7d6c349.

📒 Files selected for processing (9)
  • .env.example
  • AGENTS.md
  • CLAUDE.md
  • app/src-tauri/src/core_process.rs
  • app/src-tauri/src/core_process_tests.rs
  • docs/SECURITY_AUDIT.md
  • scripts/test-webhook-flow.sh
  • src/core/auth.rs
  • src/core/jsonrpc.rs

Comment thread docs/SECURITY_AUDIT.md
Comment thread scripts/test-webhook-flow.sh Outdated
Comment thread src/core/auth.rs
Comment thread src/core/jsonrpc.rs
Copy link
Copy Markdown
Contributor

@graycyrus graycyrus left a comment

Choose a reason for hiding this comment

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

@oxoxDev hey! the code looks solid — this is a clean and well-scoped security fix. the approach (OnceLock in-memory handoff, explicit idempotency, removing the env crossing entirely from the Tauri path, regression test) is exactly right. once CI is green i'll approve.

One minor thing I noticed that CodeRabbit didn't catch:

[minor] app/src-tauri/src/core_process_tests.rsenv_during_spawn assertion is redundant and its comment is misleading

In ensure_running_does_not_publish_token_to_env, both env_after and env_during_spawn are captured back-to-back via std::env::var(...) after ensure_running().await returns — no task is spawned in between, and both reads happen at the same instant. The comment says "peek midway via spawning a tiny check task" but no task is actually spawned. The two assertions will always pass or fail together, making env_during_spawn a dead variable. The core regression guard (env_after.is_none()) is correct and sufficient — the second assertion doesn't add coverage. Worth cleaning up so the test is honest about what it proves.

Fix the CI failures and this is good to go.

@graycyrus
Copy link
Copy Markdown
Contributor

@oxoxDev unresolved review feedback — please address before we review.

@senamakel senamakel self-assigned this May 28, 2026
oxoxDev added 4 commits May 28, 2026 11:22
…sport

Update §3 Core RPC Auth to describe two transport shapes:
- Desktop/Tauri: bearer held in CoreProcessHandle.rpc_token: Arc<String>,
  handed to embedded server via internal handle. Not on process env.
- Standalone CLI / Docker / cloud: bearer from OPENHUMAN_CORE_TOKEN env
  var or {workspace}/core.token file (operator-supplied config).

Keeps the audit doc internally consistent with §6 item 6 (already
marked resolved) and the new init path.
When both files exist, the previous precedence (workspace/core.token first,
debug e2e file second) silently shadowed the live debug bearer with a stale
standalone token left over from an earlier 'openhuman core run' session.

New resolution order:
  1. OPENHUMAN_CORE_TOKEN env var — always wins (explicit op config)
  2. Freshest by mtime between:
       - $workspace/core.token (standalone CLI writer)
       - ${TMPDIR:-/tmp}/openhuman-e2e-rpc-token (Tauri debug writer)

Also prints a one-line debug log to stderr telling the user which source
was picked, so token-mismatch failures are diagnosable at a glance.

mtime resolved via BSD/GNU stat with a graceful fallback. Existing
not-found error path preserved.
…th in-memory desktop path

CodeRabbit flagged the rustdoc on CORE_TOKEN_ENV_VAR / init_rpc_token still
claiming the Tauri shell sets OPENHUMAN_CORE_TOKEN. That contradicted the
module header and init_rpc_token_with_value docs which already describe the
in-memory handoff correctly.

Rewrite both to state plainly:
- The Tauri desktop shell does NOT set OPENHUMAN_CORE_TOKEN; it uses
  init_rpc_token_with_value for an in-memory handoff.
- OPENHUMAN_CORE_TOKEN remains the operator-supplied configuration surface
  for standalone CLI / Docker / cloud only.

Also retire the lingering 'Tauri-managed' wording on the env-path log line
and inline comment inside init_rpc_token (env path is now operator-supplied).
…lic-bind check

The public-bind safety warning is supposed to fire only when the RPC server
binds on a non-loopback address WITHOUT an explicit operator-supplied
bearer. With the new internal handoff, an embedded caller (the Tauri shell)
can pass the bearer in-memory via the `rpc_token: Option<Arc<String>>`
parameter — that is also an explicit-token configuration, but the existing
check only looked at $OPENHUMAN_CORE_TOKEN.

Extend the check so 'explicit token' means any of:
  - in-memory bearer supplied to run_server_inner via the rpc_token param
  - OPENHUMAN_CORE_TOKEN set in the process environment (CLI/docker/cloud)
  - auth subsystem already seeded (live get_rpc_token belt-and-suspenders)

Prevents a false security warning when the Tauri shell is providing the
bearer in-memory but a developer has opted into a non-loopback bind via
OPENHUMAN_CORE_HOST for LAN testing.
@oxoxDev
Copy link
Copy Markdown
Contributor Author

oxoxDev commented May 28, 2026

@graycyrus — CodeRabbit's four inline items addressed in 040d4c4..361638e: docs split (SECURITY_AUDIT.md desktop vs CLI/docker/cloud transport), freshest-bearer resolution in test-webhook-flow.sh, rustdoc alignment in src/core/auth.rs (CORE_TOKEN_ENV_VAR + init_rpc_token), and in-memory bearer in the has_explicit_token public-bind check in src/core/jsonrpc.rs. Ready for re-review when you have a slot. (5 CI failures remain and are pre-existing main breakage being unblocked by PR #2710; they'll turn green once #2710 merges and I rebase.)

@oxoxDev oxoxDev requested a review from graycyrus May 28, 2026 05:57
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 28, 2026
Copy link
Copy Markdown
Contributor

@graycyrus graycyrus left a comment

Choose a reason for hiding this comment

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

@oxoxDev the four follow-up commits look good — docs/SECURITY_AUDIT.md now cleanly separates the desktop in-memory path from CLI/docker/cloud env-as-config, the mtime-based freshest-wins logic in test-webhook-flow.sh correctly handles the stale-standalone-token case, the rustdoc on CORE_TOKEN_ENV_VAR and init_rpc_token is now accurate, and the three-pronged has_explicit_token check (in-memory || env || already-initialized) is a solid defensive improvement.

All CodeRabbit threads are resolved. Still blocked on CI — Rust Core Tests + Quality, Windows Rust Core Tests, Rust Core Coverage, and Build & smoke-test core image are failing. Once those are green I'll approve.

CI was hitting a 1/9795 test failure where
test_chat_completions_with_bearer_not_rejected_as_auth_error returned
401 because the new auth-test
init_rpc_token_with_value_seeds_and_is_idempotent (added in
a58250b) seeded the shared RPC_TOKEN OnceLock with its own bearer
before this test ran in the same lib-test binary. Test order is
non-deterministic, so the failure is a race, not a deterministic break
of either test.

ensure_test_rpc_auth now returns the bearer actually installed in
RPC_TOKEN (whichever sibling test seeded it first). The failing test
reads its bearer from that return value instead of the
TEST_RPC_TOKEN constant. The other tests in this file that intentionally
omit the Authorization header are unaffected.

No behavior change to the auth subsystem itself. test-only fix.
@oxoxDev
Copy link
Copy Markdown
Contributor Author

oxoxDev commented May 28, 2026

@graycyrus quick status:

  • coderabbitai re-reviewed and approved at 2026-05-28T06:01Z after I pushed addresses for all four of its earlier inline notes (docs/SECURITY_AUDIT.md §3 split, scripts/test-webhook-flow.sh freshest-mtime resolution, src/core/auth.rs rustdoc consistency, and src/core/jsonrpc.rs has_explicit_token in-memory branch). Each is acknowledged via <review_comment_addressed> on the original threads.
  • CI's earlier failure was a RPC_TOKEN OnceLock race between the new init_rpc_token_with_value_seeds_and_is_idempotent test and the older test_chat_completions_with_bearer_not_rejected_as_auth_error (whichever ran first sealed the OnceLock and the other read a stale value). Just pushed da53de53 making ensure_test_rpc_auth token-agnostic — fix is test-only, no auth-subsystem behavior change.

Re-review when you have a moment 🙏

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/openhuman/inference/http/tests.rs (1)

31-43: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add debug/trace diagnostics to the changed auth-init helper flow.

This helper changed behavior and is now the source-of-truth token provider for tests, but it still has no grep-friendly debug/trace breadcrumbs for init/ready states.

♻️ Proposed update
 fn ensure_test_rpc_auth() -> String {
     static INIT: Once = Once::new();
+    log::trace!("[rpc] ensure_test_rpc_auth: entry");
+
     INIT.call_once(|| {
+        log::debug!("[rpc] ensure_test_rpc_auth: initializing test RPC auth");
         // SAFETY: test-only init; we serialize via `Once`, and live_routing_e2e
         // uses its own env lock + a different token value so the two test
         // binaries don't collide (they run in separate processes anyway).
         unsafe { std::env::set_var(CORE_TOKEN_ENV_VAR, TEST_RPC_TOKEN) };
 
         let tmp = tempfile::tempdir().expect("tempdir for token file");
         crate::core::auth::init_rpc_token(tmp.path()).expect("init rpc auth token for http tests");
     });
-    crate::core::auth::get_rpc_token()
+
+    let token = crate::core::auth::get_rpc_token()
         .expect("rpc bearer must be installed after ensure_test_rpc_auth")
-        .to_string()
+        .to_string();
+    log::debug!(
+        "[rpc] ensure_test_rpc_auth: ready (token_len={})",
+        token.len()
+    );
+    token
 }

As per coding guidelines, Rust changes should include debug/trace diagnosis logs with stable prefixes and must never log secrets (only redacted metadata).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/inference/http/tests.rs` around lines 31 - 43, The test auth
helper ensure_test_rpc_auth lacks grep-friendly diagnostics; add
tracing::debug!/trace! calls (use a stable prefix like "[test-auth]") at key
points in ensure_test_rpc_auth: before setting CORE_TOKEN_ENV_VAR (log the env
var name and redact the token value), after successfully calling
crate::core::auth::init_rpc_token (log the tmp path and success), and after
get_rpc_token returns (log that a token was retrieved without printing the
secret, e.g., length or redacted marker). Also emit an error-level trace when
init_rpc_token or get_rpc_token fail, including error details but never the
token itself; reference the functions/vars ensure_test_rpc_auth,
CORE_TOKEN_ENV_VAR, TEST_RPC_TOKEN, init_rpc_token, and get_rpc_token so the
logs are clearly tied to this flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/openhuman/inference/http/tests.rs`:
- Around line 31-43: The test auth helper ensure_test_rpc_auth lacks
grep-friendly diagnostics; add tracing::debug!/trace! calls (use a stable prefix
like "[test-auth]") at key points in ensure_test_rpc_auth: before setting
CORE_TOKEN_ENV_VAR (log the env var name and redact the token value), after
successfully calling crate::core::auth::init_rpc_token (log the tmp path and
success), and after get_rpc_token returns (log that a token was retrieved
without printing the secret, e.g., length or redacted marker). Also emit an
error-level trace when init_rpc_token or get_rpc_token fail, including error
details but never the token itself; reference the functions/vars
ensure_test_rpc_auth, CORE_TOKEN_ENV_VAR, TEST_RPC_TOKEN, init_rpc_token, and
get_rpc_token so the logs are clearly tied to this flow.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 37c6b734-6343-418f-aeba-18d6844ec04f

📥 Commits

Reviewing files that changed from the base of the PR and between 361638e and da53de5.

📒 Files selected for processing (1)
  • src/openhuman/inference/http/tests.rs

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

Labels

feature Net-new user-facing capability or product behavior. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants