Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Broker-spawned workers no longer inherit the broker's own credentials from its environment; each worker receives only its own agent token and the workspace credentials the broker delegates to it.
- Integration subscription setup, listing and retirement use workspace authentication even when a spawned worker also has an agent token, preventing misleading “Workspace key required” failures.
- A `teams.json` agent whose `cli` carries an inline `--model`/`-m` now records the model the harness actually runs. The inline override becomes the spawn's effective model before the relay skill prefix is chosen, so worker listings, spawn events, telemetry and small-model guidance describe the running model rather than the superseded pin.
- `agent-relay fleet config|enable|disable|inherit` now exit successfully as hidden compatibility no-ops instead of failing on the removed workspace rollout API.
Expand Down
21 changes: 9 additions & 12 deletions crates/broker/src/snippets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,16 +247,13 @@ async fn probe_agent_relay_mcp_command_with_timeout(
command: &AgentRelayMcpCommand,
timeout: Duration,
) -> io::Result<()> {
let mut child = Command::new(&command.command)
let mut probe = Command::new(&command.command);
// Tool discovery is local and must not rotate an agent identity or
// print credentials while diagnosing a broken MCP executable.
crate::spawner::remove_inherited_relay_credentials(&mut probe);
let mut child = probe
.args(&command.args)
// Tool discovery is local and must not rotate an agent identity or
// print credentials while diagnosing a broken MCP executable.
.env("RELAY_SKIP_BOOTSTRAP", "1")
.env_remove("RELAY_API_KEY")
.env_remove("RELAY_WORKSPACE_KEY")
.env_remove("AGENT_RELAY_WORKSPACE_KEY")
.env_remove("RELAY_AGENT_TOKEN")
.env_remove("RELAY_WORKSPACES_JSON")
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::null())
Expand Down Expand Up @@ -1954,7 +1951,7 @@ fn grok_manual_mcp_add_cmd(cli: &str) -> String {

async fn remove_grok_mcp_servers(exe: &str) {
for server_name in [AGENT_RELAY_MCP_SERVER, LEGACY_RELAYCAST_SERVER] {
let mut cmd = Command::new(exe);
let mut cmd = crate::spawner::scrubbed_command(exe);
cmd.args(["mcp", "remove", server_name])
.stdin(Stdio::null())
.stdout(Stdio::null())
Expand Down Expand Up @@ -1982,7 +1979,7 @@ async fn configure_grok_mcp(

remove_grok_mcp_servers(&exe).await;

let mut mcp_cmd = Command::new(&exe);
let mut mcp_cmd = crate::spawner::scrubbed_command(&exe);
mcp_cmd.args(grok_mcp_add_args(
api_key,
base_url,
Expand Down Expand Up @@ -2068,7 +2065,7 @@ async fn configure_gemini_droid_mcp(
/// Remove all known relay MCP server names from the gemini/droid shared config.
async fn remove_gemini_droid_mcp_servers(exe: &str) {
for server_name in [AGENT_RELAY_MCP_SERVER, LEGACY_RELAYCAST_SERVER] {
let mut cmd = Command::new(exe);
let mut cmd = crate::spawner::scrubbed_command(exe);
cmd.args(["mcp", "remove", server_name])
.stdin(Stdio::null())
.stdout(Stdio::null())
Expand Down Expand Up @@ -2135,7 +2132,7 @@ async fn spawn_mcp_add(
cli: &str,
manual_cmd: &str,
) -> Result<std::process::Output> {
let mut mcp_cmd = Command::new(exe);
let mut mcp_cmd = crate::spawner::scrubbed_command(exe);
mcp_cmd
.args(add_args)
.stdin(Stdio::null())
Expand Down
70 changes: 67 additions & 3 deletions crates/broker/src/spawner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ const RELAY_ATTEST_GIT_CONFIG_COUNT: &str = "RELAY_ATTEST_GIT_CONFIG_COUNT";
const RELAY_ATTEST_GIT_CONFIG_INDEX: &str = "RELAY_ATTEST_GIT_CONFIG_INDEX";
const RELAY_ATTEST_BROKER_HOOK_PATH: &str = "RELAY_ATTEST_BROKER_HOOK_PATH";

#[cfg(test)]
pub(crate) use relay_pty::credentials::INHERITED_RELAY_CREDENTIAL_ENV_KEYS;
/// Relay-owned credentials no spawned worker or spawn-time helper inherits
/// from the broker's environment; see [`relay_pty::credentials`]. Every worker
/// spawn strips this list first and then injects only what that worker is meant
/// to hold: its own `RELAY_AGENT_TOKEN`, its own result callback token, and the
/// workspace credentials the broker explicitly delegates to agents through its
/// worker environment.
pub(crate) use relay_pty::credentials::{remove_inherited_relay_credentials, scrubbed_command};

/// Standard client-side git hooks (see githooks(5)). `core.hooksPath` is a
/// single directory that replaces git's entire hook lookup, not just
/// `prepare-commit-msg` — so the broker must install a forwarder under every
Expand Down Expand Up @@ -372,6 +382,7 @@ impl Spawner {
.stdout(Stdio::inherit())
.stderr(Stdio::inherit());

remove_inherited_relay_credentials(&mut cmd);
let mut child_env = env_vars.to_vec();
if attestation_env_present(&child_env) {
match self.commit_hooks_dir() {
Expand Down Expand Up @@ -577,11 +588,64 @@ mod tests {

use super::{
add_broker_hooks_path, attestation_env_present, git_config_count_with_inherited,
spawn_env_vars, terminate_child, with_commit_attestation_env, write_broker_git_hooks,
Spawner, RELAY_ATTEST_AGENT_ID, RELAY_ATTEST_JTI, RELAY_ATTEST_SESSION_ID,
RELAY_ATTEST_SPONSOR_ID,
remove_inherited_relay_credentials, spawn_env_vars, terminate_child,
with_commit_attestation_env, write_broker_git_hooks, Spawner,
INHERITED_RELAY_CREDENTIAL_ENV_KEYS, RELAY_ATTEST_AGENT_ID, RELAY_ATTEST_JTI,
RELAY_ATTEST_SESSION_ID, RELAY_ATTEST_SPONSOR_ID,
};

#[test]
fn inherited_relay_credentials_cover_broker_node_and_workspace_secrets() {
for key in [
"RELAY_BROKER_API_KEY",
"RELAY_NODE_TOKEN",
"RELAY_AGENT_IDENTITY_KEY",
"RELAY_AGENT_TOKEN",
"AGENT_RELAY_RESULT_TOKEN",
"RELAY_API_KEY",
"RELAY_WORKSPACE_KEY",
"AGENT_RELAY_WORKSPACE_KEY",
"RELAY_WORKSPACES_JSON",
] {
assert!(
INHERITED_RELAY_CREDENTIAL_ENV_KEYS.contains(&key),
"{key} must be stripped from worker environments"
);
}
}

#[test]
fn remove_inherited_relay_credentials_strips_every_key_and_later_env_wins() {
let mut command = Command::new("true");
remove_inherited_relay_credentials(&mut command);
command.env("RELAY_AGENT_TOKEN", "worker-own-token");

let envs: std::collections::HashMap<String, Option<String>> = command
.as_std()
.get_envs()
.map(|(key, value)| {
(
key.to_string_lossy().into_owned(),
value.map(|v| v.to_string_lossy().into_owned()),
)
})
.collect();
for key in INHERITED_RELAY_CREDENTIAL_ENV_KEYS {
if *key == "RELAY_AGENT_TOKEN" {
continue;
}
assert_eq!(envs.get(*key), Some(&None), "{key} should be removed");
}
assert_eq!(
envs.get("RELAY_AGENT_TOKEN"),
Some(&Some("worker-own-token".to_string())),
"a worker's own token set after the scrub must survive"
);
// Non-relay environment is left to normal inheritance.
assert!(!envs.contains_key("PATH"));
assert!(!envs.contains_key("ANTHROPIC_API_KEY"));
}

fn git(repo: &Path, args: &[&str], env: &[(String, String)]) -> std::process::Output {
StdCommand::new("git")
.current_dir(repo)
Expand Down
81 changes: 73 additions & 8 deletions crates/broker/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ use crate::{
runtime::headless_provider_cli_name,
spawner::{
add_broker_hooks_path, attestation_env_present, is_valid_attestation_value,
resolve_commit_hooks_dir, terminate_child, with_commit_attestation_env,
RELAY_ATTEST_AGENT_ID, RELAY_ATTEST_JTI, RELAY_ATTEST_SESSION_ID, RELAY_ATTEST_SPONSOR_ID,
remove_inherited_relay_credentials, resolve_commit_hooks_dir, terminate_child,
with_commit_attestation_env, RELAY_ATTEST_AGENT_ID, RELAY_ATTEST_JTI,
RELAY_ATTEST_SESSION_ID, RELAY_ATTEST_SPONSOR_ID,
},
};

Expand Down Expand Up @@ -1208,6 +1209,11 @@ impl WorkerRegistry {
// attested child through Command's inherited environment.
command.env_remove(key);
}
// Every runtime (PTY, headless provider, app-server, native sidecar)
// reaches this point with a command that inherits the broker's
// environment. Drop relay-owned credentials from it; the worker's own
// credentials are injected below.
remove_inherited_relay_credentials(&mut command);
Comment on lines +1212 to +1216

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Scrub credentials before running spawn-time CLIs

When spawning Grok, Gemini, or Droid, build_mcp_args runs their mcp remove/add subprocesses in snippets.rs, while Codex can run its app-server or model probe, all before execution reaches this scrub. Those provider processes therefore still inherit RELAY_BROKER_API_KEY, RELAY_NODE_TOKEN, and RELAY_AGENT_IDENTITY_KEY, exposing the credentials this patch intends to withhold even though the final worker command is clean. Apply the same scrub to every spawn-time Command, or sanitize the environment passed into these helpers before invoking them.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 30fc1af. The credential list moved into relay_pty::credentials, with a scrubbed_command(program) constructor. Every spawn-time helper now builds its command through it:

  • the grok and gemini/droid mcp add / mcp remove subprocesses (snippets.rs)
  • codex debug models (worker.rs)
  • codex session pre-creation via app-server (relay-pty/codex_session.rs)

The broker re-exports the same helper, so worker spawns and helpers share one list. A new unit test asserts that scrubbed_command removes every key and that an explicit value set afterwards still wins. Results: relay-pty 253 passed, broker 1309 passed, clippy -D warnings clean on both libs.

child_env.retain(|(key, _)| {
!matches!(
key.as_str(),
Expand Down Expand Up @@ -2650,12 +2656,8 @@ async fn codex_debug_models_output(resolved_cli: &str) -> std::io::Result<std::p
let mut attempt: u32 = 0;
loop {
attempt += 1;
match Command::new(resolved_cli)
.arg("debug")
.arg("models")
.output()
.await
{
let mut command = crate::spawner::scrubbed_command(resolved_cli);
match command.arg("debug").arg("models").output().await {
Err(err)
if err.kind() == std::io::ErrorKind::ExecutableFileBusy
&& attempt < MAX_ATTEMPTS =>
Expand Down Expand Up @@ -2913,6 +2915,69 @@ mod tests {
}
}

#[cfg(any(target_os = "linux", target_os = "macos"))]
#[tokio::test]
async fn spawned_worker_holds_only_its_own_and_delegated_relay_credentials() {
let dir = tempfile::tempdir().expect("worker cwd");
// Record only the NAMES of relay credential variables the worker sees.
let keys = crate::spawner::INHERITED_RELAY_CREDENTIAL_ENV_KEYS.join(" ");
let script = format!(
"for k in {keys}; do if printenv \"$k\" >/dev/null; then echo \"$k\"; fi; done > names.tmp && mv names.tmp names.txt; sleep 30"
);
let mut spec = sleeping_native_worker("credential-worker", None);
spec.harness_config = Some(ResolvedHarnessConfig::Native(NativeHarnessConfig {
command: "sh".to_string(),
args: vec!["-c".to_string(), script],
cwd: Some(dir.path().to_string_lossy().into_owned()),
env: None,
session_id: "session-credential-worker".to_string(),
metadata: None,
}));
// The broker explicitly delegates a workspace key through its worker
// environment; that delegation must survive the inherited scrub.
let mut registry = make_registry(vec![(
"RELAY_API_KEY".to_string(),
"rk_live_delegated".to_string(),
)]);

registry
.spawn(
spec,
None,
None,
Some("at_live_worker_own".to_string()),
false,
None,
None,
None,
None,
)
.await
.expect("credential worker should spawn");

let names_path = dir.path().join("names.txt");
let mut names = None;
for _ in 0..50 {
if let Ok(value) = std::fs::read_to_string(&names_path) {
names = Some(value);
break;
}
tokio::time::sleep(Duration::from_millis(100)).await;
}
registry
.release("credential-worker")
.await
.expect("release credential worker");

let mut observed: Vec<String> = names
.expect("worker recorded its credential names")
.lines()
.map(str::to_string)
.collect();
observed.sort();
assert_eq!(observed, vec!["RELAY_AGENT_TOKEN", "RELAY_API_KEY"]);
}

#[cfg(any(target_os = "linux", target_os = "macos"))]
#[tokio::test]
async fn spawned_worker_process_uses_requested_cwd() {
Expand Down
4 changes: 2 additions & 2 deletions crates/relay-pty/src/codex_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use anyhow::{bail, Context, Result};
use serde_json::{json, Value};
use tokio::{
io::{AsyncBufReadExt, AsyncWriteExt, BufReader, Lines},
process::{ChildStdin, ChildStdout, Command},
process::{ChildStdin, ChildStdout},
time::timeout,
};

Expand Down Expand Up @@ -47,7 +47,7 @@ async fn create_resumable_codex_thread_inner(
client_version: &str,
) -> Result<String> {
let thread_cwd = cwd.canonicalize().unwrap_or_else(|_| cwd.to_path_buf());
let mut command = Command::new(codex_bin);
let mut command = crate::credentials::scrubbed_command(codex_bin);
command
.arg("app-server")
.arg("--listen")
Expand Down
78 changes: 78 additions & 0 deletions crates/relay-pty/src/credentials.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//! Relay-owned credentials a spawned process must not inherit from the
//! spawning process's environment. Every agent worker and every spawn-time
//! helper (MCP registration, model probes, session pre-creation) strips this
//! list first; a worker then receives only what it is meant to hold through an
//! explicit `Command::env`, which still wins after the scrub.

use tokio::process::Command;

pub const INHERITED_RELAY_CREDENTIAL_ENV_KEYS: &[&str] = &[
// The broker's local HTTP API key (set on the broker process at startup).
"RELAY_BROKER_API_KEY",
// The node's control-plane credential.
"RELAY_NODE_TOKEN",
// The broker's own registration identity proof.
"RELAY_AGENT_IDENTITY_KEY",
// Whoever launched the broker; each worker gets its own.
"RELAY_AGENT_TOKEN",
"AGENT_RELAY_RESULT_TOKEN",
// Workspace credentials. Re-added from the broker's explicit worker
// environment when the broker delegates them; never taken from ambient
// environment.
"RELAY_API_KEY",
"RELAY_WORKSPACE_KEY",
"AGENT_RELAY_WORKSPACE_KEY",
"RELAY_WORKSPACES_JSON",
];

/// Remove [`INHERITED_RELAY_CREDENTIAL_ENV_KEYS`] from a command's inherited
/// environment. Call before applying the command's own environment: a later
/// `Command::env` for the same key still wins.
pub fn remove_inherited_relay_credentials(command: &mut Command) {
for key in INHERITED_RELAY_CREDENTIAL_ENV_KEYS {
command.env_remove(key);
}
}

/// A `Command` for `program` that does not inherit relay credentials. Use it for
/// every process the broker starts on an agent's behalf, including spawn-time
/// helpers that never become the agent (MCP registration, model probes).
pub fn scrubbed_command(program: impl AsRef<std::ffi::OsStr>) -> Command {
let mut command = Command::new(program);
remove_inherited_relay_credentials(&mut command);
command
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn scrubbed_command_removes_every_relay_credential() {
let mut command = scrubbed_command("true");
command.env("RELAY_AGENT_TOKEN", "own-token");
let envs: std::collections::HashMap<_, _> = command
.as_std()
.get_envs()
.map(|(key, value)| {
(
key.to_string_lossy().into_owned(),
value.map(|v| v.to_owned()),
)
})
.collect();
for key in INHERITED_RELAY_CREDENTIAL_ENV_KEYS {
if *key == "RELAY_AGENT_TOKEN" {
continue;
}
assert_eq!(envs.get(*key), Some(&None), "{key} must be removed");
}
// An explicit value set after construction still wins.
assert_eq!(
envs.get("RELAY_AGENT_TOKEN"),
Some(&Some("own-token".into()))
);
// Everything else is left to normal inheritance.
assert!(!envs.contains_key("PATH"));
}
}
1 change: 1 addition & 0 deletions crates/relay-pty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
pub mod ansi;
pub mod codex_session;
pub mod crash_insights;
pub mod credentials;
pub mod detection;
pub mod inject;
pub mod pty;
Expand Down
5 changes: 5 additions & 0 deletions tests/e2e/fleet/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ omitted/null heartbeat-load compatibility added in relaycast#307.
| delivery seq/dedup | per-agent deliveries carry strictly monotonic `agent_seq` (no duplicates); a resync from a mid-cursor replays only the tail (`gap_detected: false`, no duplicate seqs) — the exactly-once cursor the node-restart reconcile relies on |
| mailbox TTL | an undelivered message dead-letters after a short TTL **and the sender is notified** (`delivery.failed` naming the target) |

`worker-env.test.ts` boots a single `env-probe` node and asserts that a spawned
worker sees only its own `RELAY_AGENT_TOKEN` plus the workspace credentials the
broker delegates — never the broker API key, node token, or broker identity key.
The probe records variable names only, never values.

### Coverage notes (intentionally not re-asserted here)

- **overflow reject-new**: `belowDepthCapSql` rejects new deliveries past the per-agent
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/fleet/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const REPO_ROOT = path.resolve(HERE, '..', '..', '..');
export const NODE_A_FILE = path.join(HERE, 'nodes', 'node-a.ts');
export const NODE_B_FILE = path.join(HERE, 'nodes', 'node-b.ts');
export const CLOUD_ENROLLED_NODE_FILE = path.join(HERE, 'nodes', 'cloud-enrolled.ts');
export const ENV_PROBE_NODE_FILE = path.join(HERE, 'nodes', 'env-probe.ts');

const CLI_ENTRY = path.join(REPO_ROOT, 'packages', 'cli', 'dist', 'cli', 'index.js');

Expand Down
Loading
Loading