P1: trust boundary: decide what gets scrubbed, gated, escaped, and confined before untrusted text reaches a shell or a page

[macanderson]

What decision this asks for

Each item below is a single ruling: how much authority do we let a model-driven subprocess, an untrusted server, or an untrusted string keep once it crosses a boundary? The fixes are already written or fully specified — what is missing is the call, because on every one of these surfaces tightening breaks a legitimate workflow (a Django dev server that needs DJANGO_SECRET_KEY, a monorepo that legitimately declares ../ members, an SVG attribute value that happens to contain word:) and loosening reopens the hole.

Decide strict-vs-permissive per surface, once, and let the doc wording follow from the choice. Two items are load-bearing and should be answered first: item 1 (run_tests interpolates a model-supplied filter straight into bash -c — a real security finding) and items 8–10 (the env-scrub suffix policy, where two of the proposals contradict each other about whether to widen at all).

Spot-checked against origin/main at the time of filing: the run_tests filter interpolation (stella-tools/src/project.rs:236,257), the SVG references_external //-substring test (stella-media/src/svg.rs:344-346), the is_sensitive_env_name suffix list (_API_KEY/_TOKEN/_PASSWORD/_SECRET only — STRIPE_SECRET_KEY still passes), expand_member_pattern's unconfined root.join(base) (stella-tools/src/scripts.rs:1115), the "rs" => recognized_dir || name.contains("_test.") witness arm (stella-pipeline/src/witness.rs:509), the .ok().flatten() manifest call sites, and the absence of headless_scope_bypass from the settings overlay take! list all reproduce as described.

Items

Shell / subprocess authority

• run_tests interpolates the model-supplied filter straight into the bash -c command line — FIXED in fix(stella-tools): stop run_tests interpolating a model-chosen filter into bash -c #624 (per-token shell quoting via a SafeFilter newtype; preserves the multi-token contract the deferral was protecting). Original deferral: format!("cargo test --workspace {filter}") and format!("{pm} run {script} -- {filter}"). Stop interpolating it into the shell line. Deferred because the issue's own Fix disclaims both routes: quoting breaks the documented multi-token filter contract (tests.join(" ") from impact selection relies on shell splitting), and argv composition is a cross-function signature change. Flagged in the source issue as "a real security finding that deserves its own ticket". stella-tools/src/project.rs:236,257 — severity high. (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603)
• Gate or sandbox the start_process family. It is pushed into the default tool set unconditionally while only Bash sits behind the bash flag. Whichever posture is chosen, the safety promises in bash.rs:5-9 and settings.rs:557-560 have to be rewritten to match. Deferred because the Fix says "pick one and state it in both doc comments" — an operator-facing security-posture decision, and rewriting the operator's risk sentence without the posture chosen would make the docs wrong in a new way. stella-tools/src/registry/process_tools.rs:23, stella-tools/src/bash.rs, stella-tools/src/settings.rs — severity medium. (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603)
• Extend the command.started fence to the other command-executing default tools — run_lint, format_code, diagnostics, screenshot, ci_status, and the repo_* tools. Their commands still never reach ToolRegistry::command_line_for / the blocking command.started policy chain, so an extension policy that denies command execution cannot see them. Deferred as "out of scope and not the same defect": those use exec::run_argv (direct argv exec, no shell) with argv composed by Stella from the scripts index, and widening here is a drive-by refactor with a much larger event-surface blast radius. stella-tools/src/registry.rs — severity medium. (audit stella-tools: build_project/run_tests/verify_done reach bash -c outside the command.started policy chain #548, deferred by fix(stella-tools): fence build_project/run_tests/verify_done on command.started #584)
• command.started is not byte-exact for run_tests with scope: "impacted". The gate-time resolver (resolve_command_for_gate) deliberately does not re-run impact::select_impacted, so the gated command line is the un-narrowed command for the call's kind/filter — the filter half can read wider than what actually executed. Deferred because the only faithful fix is to resolve once and thread the resolved command through to execution, a ToolRegistry/Tool contract change the source issue does not ask for (and run_script's shipped precedent deliberately avoids it); re-running select_impacted at gate time would double the cost and could diverge from the execute-time selection. stella-tools/src/project.rs, stella-tools/src/registry.rs — severity low. (audit stella-tools: build_project/run_tests/verify_done reach bash -c outside the command.started policy chain #548, deferred by fix(stella-tools): fence build_project/run_tests/verify_done on command.started #584)
• Confine expand_member_pattern with resolve_within_root so a workspace-member pattern read from Cargo.toml / package.json / pnpm-workspace.yaml cannot index a directory outside the root. Deferred because the Fix disclaims it: some real monorepos legitimately declare ../ members, and silently dropping them is a behaviour change maintainers should choose. stella-tools/src/scripts.rs:1115 (issue cites :1136) — severity medium. (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603) — landed in fix(security): scrub ambient authority, close the SVG data: gap, confine workspace members #695. The ruling was "confine, visibly": each glob expansion is confined too, not just the glob base, and skipped ../ members are not silent — they ride a ScriptIndex::out_of_root_members counter into render_list.

Environment scrub policy (these three overlap on one const and two of them contradict each other — answer as one ruling)

• Widen the credential suffix policy to add _SECRET_KEY, _PRIVATE_KEY, _ACCESS_KEY, _APIKEY, and consider SSH_AUTH_SOCK as an exact entry, so spellings like STRIPE_SECRET_KEY stop passing through to every model-controlled subprocess. Deferred because the Fix disclaims it: widening the scrub is a policy change that breaks real workflows (a Django dev server started via start_process needs DJANGO_SECRET_KEY). stella-tools/src/subprocess_env.rs:83 — severity medium. (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603) — landed in fix(security): scrub ambient authority, close the SVG data: gap, confine workspace members #695: _SECRET_KEY, _PRIVATE_KEY, _ACCESS_KEY, _APIKEY added, plus SSH_AUTH_SOCK as an exact entry, with STELLA_SUBPROCESS_ENV_ALLOW (exact names only, never read from a command's [env] overrides) as the escape hatch that makes widening safe.
• Extend is_sensitive_env_name with _KEY, _CREDENTIALS, _CREDENTIAL, _PAT, _AUTH suffixes plus an AMBIENT_AUTHORITY_ENV_VARS set. Same const as the item above and strictly broader — and the two contradict each other about whether to widen at all. _KEY/_AUTH deliberately overmatch (SSH_KEY_PATH, PUBLIC_KEY, LICENSE_KEY) and would silently strip env from every model-launched subprocess. stella-tools/src/subprocess_env.rs:82 — severity medium. (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603) — landed in fix(security): scrub ambient authority, close the SVG data: gap, confine workspace members #695, resolving the contradiction with the row above: _CREDENTIALS, _CREDENTIAL, _PAT added; bare _KEY and _AUTH deliberately not added (they overmatch PUBLIC_KEY/LICENSE_KEY/SSH_KEY_PATH/AUTH_URL), pinned by a test so a future widening has to argue with a red test.
• Extend SystemGitCli::run's env_remove list to scrub the git config/command-injection family — GIT_CONFIG_COUNT/GIT_CONFIG_KEY_n/GIT_CONFIG_VALUE_n, GIT_SSH_COMMAND, GIT_EXTERNAL_DIFF, GIT_PROXY_COMMAND — or better, move the decision into stella_tools::subprocess_env alongside the credential scrub so stella-tools' and stella-cli's own git invocations get the same treatment. Deferred because the Fix explicitly declined ("env policy is another crate's concern"), scrubbing GIT_SSH_COMMAND breaks deploy-key auth setups that legitimately set it, and it is unverifiable locally (SystemGitCli spawns real git, so a witness needs process-global std::env::set_var, unsafe in edition 2024 and racy under the parallel harness). Graded "inconsistent rather than dangerous today". stella-fleet/src/git.rs:97 — severity low. (audit Batched audit cleanup: area:fleet — 14 items #562, deferred by chore(stella-fleet): apply the area:fleet audit batch #593) — landed in fix(security): scrub ambient authority, close the SVG data: gap, confine workspace members #695, taking the "or better" option: the family moved into the shared stella_tools::subprocess_env scrub under a separate is_ambient_authority_env_name predicate, so stella-tools' and stella-cli's own git invocations get identical treatment from one list, and GIT_CONFIG_KEY_n / GIT_CONFIG_VALUE_n are prefix-matched.

Untrusted text reaching a page

• Rewrite the exported dashboard's barChart/statsTable/insights renderers to build DOM with document.createElement + textContent instead of innerHTML string concatenation. The landed PR only added an esc() helper around interpolations. Deferred as a structurally safer design but a full rewrite of three rendering functions, well beyond the source issue's remediation text — no drive-by refactors. stella-cli/src/export.rs — severity low. (audit stella-cli: /export builds DOM via innerHTML from agent- and MCP-chosen strings #552, deferred by fix(stella-cli): escape untrusted text at the /export dashboard innerHTML sinks #581)
• Audit the same untrusted-string-into-innerHTML pattern in the observatory page (52 innerHTML sites — re-counted 2026-07-26; the issue's original 48 is stale), left unfixed and unaudited. Deferred as a different crate not cited by the source issue, whose data sources and threat model (loopback-only, read-only server) were unverified there — it needs its own audit. stella-observatory/src/assets/index.html — severity medium. (audit stella-cli: /export builds DOM via innerHTML from agent- and MCP-chosen strings #552, deferred by fix(stella-cli): escape untrusted text at the /export dashboard innerHTML sinks #581)
• Decide whether the exported /export dashboard HTML gets a Content-Security-Policy meta tag or other page-level hardening. Deferred as not requested by the source issue; a CSP on a file:// artifact is a behavioral change to a shipped artifact and the owner's call. stella-cli/src/export.rs — severity low. (audit stella-cli: /export builds DOM via innerHTML from agent- and MCP-chosen strings #552, deferred by fix(stella-cli): escape untrusted text at the /export dashboard innerHTML sinks #581)
• The {watermark} interpolation and the other Rust-side HTML interpolations in the exported dashboard remain unescaped. Deferred as not part of the source finding — the watermark is a store timestamp, and changing it would widen scope without a demonstrated attacker-controlled path. stella-cli/src/export.rs — severity low. (audit stella-cli: /export builds DOM via innerHTML from agent- and MCP-chosen strings #552, deferred by fix(stella-cli): escape untrusted text at the /export dashboard innerHTML sinks #581)
• Close the SVG sanitizer's data:-URI and style-attribute gap. Rule 6 (references_external) tests only for // and javascript:, and the style attribute has no rule at all (only the <style> element is dropped), so style="background-image:url(data:image/svg+xml;base64,…onload=…)" and fill="url(data:…)" survive sanitization — re-opening the off-host/active-content vector the moment a sanitized artifact is inlined into HTML. Remediation: add "style" to the deny branch in keep_attribute, replace the // substring test with a scheme test (<ascii-alpha>[a-z0-9+.-]*:, allowing bare #fragment), add hostile-corpus tests for style="…url(data:…)", fill="url(data:…)" and a data:text/html payload, and update the rule list plus "Known limits". Deferred because tightening a sanitizer changes behavior on a path stella-tools' media tests exercise, and the prescribed scheme test over all attribute values has real false-positive risk on legitimate SVG (any value containing word:) that the attack-only hostile corpus would not catch. Security-sensitive: half-doing it is worse than not doing it. stella-media/src/svg.rs:337,344-346 (limits already recorded at :51-58) — severity medium. (audit Batched audit cleanup: area:media — 10 items #565, deferred by chore(stella-media): fix the live-smoke gate, preview labels, branding #591) — landed in fix(security): scrub ambient authority, close the SVG data: gap, confine workspace members #695: style added to the denied-attribute branch (dropped outright rather than URL-filtered — parsing CSS to decide is a second parser's worth of attack surface), and the // substring test replaced with a real scheme test applied to url(…) arguments too.

Serve / observatory HTTP posture (one dial: how strict is the private sidecar?)

• Unguessable turn ids. Deferred because getrandom is not a dependency of this crate and adding one is a workspace decision that AGENTS.md requires per-dependency justification for; the property is already documented on serve at server.rs:124-125. stella-serve/src/server.rs:222, AGENTS.md — severity medium. (audit Batched audit cleanup: area:core (1/2) — 31 items #559, deferred by chore(audit): batched area:core cleanup (7 of 31 items) #590)
• Support HTTP/1.1 keep-alive for the result POSTs instead of one request per connection. Deferred and self-disclaimed: it requires leftover-byte and pipelining discipline in a hand-rolled parser, and the code's own comment at http.rs:86-90 currently relies on one-request-per-connection to make smuggling safe. stella-serve/src/http.rs:123 — severity low. (audit Batched audit cleanup: area:core (2/2) — 31 items #560, deferred by chore(audit): apply the area:core (2/2) batch — 13 of 31 findings #595)
• Make the Bearer auth-scheme match case-insensitive per RFC 7235. Deferred as a conscious decision: it widens what authenticates, strictness is defensible for a private sidecar, and loosening an authentication check is an owner call regardless of what the RFC permits. stella-serve/src/http.rs:38 — severity low. (audit Batched audit cleanup: area:core (2/2) — 31 items #560, deferred by chore(audit): apply the area:core (2/2) batch — 13 of 31 findings #595) — landed in audit(round 2): 155 fixes from the 17-dimension re-audit — 79 → 80 #700: Request::bearer matches the scheme with eq_ignore_ascii_case, per RFC 7235 §2.1.
• Decide the observatory's 500 error response text — left unchanged when the security headers landed; the source issue's "keep it or reduce the detail it leaks" is still an unresolved choice. stella-observatory/src/lib.rs — severity low. (audit Batched audit cleanup: area:core (2/2) — 31 items #560, deferred by chore(audit): apply the area:core (2/2) batch — 13 of 31 findings #595)

Gates that silently do nothing

• Surface the swallowed StorageManifest::load error. All four call sites discard the parse error with .ok().flatten(), so a single TOML typo silently drops every layer declaration, boundary, intent sentence and redirect, and the pre-write gate stops enforcing configured boundaries with no message anywhere. Deferred because both offered routes are closed: the Fix disclaims the first (adding a field to the public IndexStats struct is a public API change), and the second is unavailable (tracing is not a workspace dependency and appears nowhere in stella-graph, so a tracing::warn! means adding a dependency to satisfy a cleanup ticket). stella-graph/src/manifest.rs:101, call sites store.rs:167, store.rs:200, graph.rs:379, lib.rs:86 — severity medium. (audit Batched audit cleanup: area:graph — 15 items #563, deferred by perf(stella-graph): bound the audit's unbounded loops, scans, and walks #600) — half done. The graph.rs / lib.rs call sites landed in perf/reliability/accuracy: eight audit-backlog fixes, reconciled onto main's new durability contract #703: assembly stays best-effort, but the reason now rides on StorageSnapshot::manifest_error and is reported by stella storage and at gate seeding. The two store.rs call sites (now store.rs:228 and store.rs:261) are still .ok().flatten(), so this row stays open.
• Add take!(headless_scope_bypass) to the settings overlay macro list, together with a restore_project_headless_bypass gate. The field is confirmed absent from the overlay list, so the setting is genuinely dead — but applying half of it opens a repo-controlled scope-review bypass, and choosing the gate (trust.hooks vs authority.project_prompts_allowed) is a security-policy decision. stella-cli/src/settings.rs:434 — severity medium. (audit Batched audit cleanup: area:cli — 30 items #557, deferred by chore(stella-cli): apply the area:cli audit batch (14 of 30 items) #597)
• Invert prove_process_free_surface's deny-list into an allowlist — assert registry.schemas() is a subset of an explicit process-free allowlist so any newly added tool fails closed by default. The landed PR only replaced the bogus deny-list names and pinned them with a test. Deferred because the inversion changes the proof's failure mode for every future tool and is a maintainer's call. stella-cli/src/enterprise_telemetry.rs:411, stella-cli/src/enterprise_telemetry_tests.rs — severity medium. (audit Batched audit cleanup: area:cli — 30 items #557, deferred by chore(stella-cli): apply the area:cli audit batch (14 of 30 items) #597)
• Tighten is_witness_test_path. It accepts any .rs file whose name contains _test. regardless of directory, so src/backdoor_test.rs passes as a witness artifact even though cargo can only run integration tests from tests/. validate_witness_invocation's required cargo test --test <stem> then fails forever, the flip never happens, the run burns its full revision budget, and a judge may still PASS on the diff — so the stray file is adopted into the user's real tree. The fix requires recognized_dir outright for the "rs" arm, dropping the bare name.contains("_test.") fallback. Deferred because it tightens an acceptance predicate (a behavior change) and the existing test table needs a new case; what counts as an acceptable witness artifact is a safety-relevant contract. stella-pipeline/src/witness.rs:509 — severity medium. (audit Batched audit cleanup: area:pipeline — 15 items #567, deferred by test(pipeline): name the work-item test modules for what they pin #594) — landed in fix(security): scrub ambient authority, close the SVG data: gap, confine workspace members #695: the "rs" arm now requires recognized_dir and the bare name.contains("_test.") fallback is gone; other languages keep their filename forms (their runners genuinely collect by filename), pinned by a per-language shape table, and witness_prompt now tells the author role where Rust tests must live.

Why these are together

Every item is the same shape of decision made on a different surface: something untrusted — a model-supplied string, a repo-supplied config value, an agent-chosen label, an inherited environment variable, a caller on loopback — is about to cross into a shell, a subprocess environment, a filesystem walk, a DOM, or an acceptance gate, and someone has to rule on how much authority it keeps. The engineering is done or specified in each case; what blocks them is that the strict answer breaks a real workflow and the permissive answer leaves the hole open. Batching them lets the posture be set coherently in one pass instead of drifting per-PR, and the env-scrub items in particular cannot be resolved independently because two of them propose contradictory widths on the same constant.

_{Deferred during the #546 audit remediation. Put Closes #<this issue> in the PR description and as a commit trailer when it lands.}

[macanderson]

Item 1 (the run_tests filter reaching bash -c) is fixed in #624.

The deferral's reasoning was sound but rested on one assumption: that quoting meant wrapping the whole filter in a single pair of quotes, which would indeed have collapsed the documented multi-token contract. Quoting each token separately preserves the token count exactly while stripping every token's shell meaning, so neither of the two objections applies — no contract break, no cross-function signature change.

The other 20 items in this issue still need their rulings.

[macanderson]

Triage 2026-07-26 — 1 of 21 landed; two counts drifted and one citation is dead

Item 1 is genuinely fixed by PR #624 (82885ef0): stella-tools/src/project.rs:234 pub(crate) struct SafeFilter(String) with from_input (per-token quoting, :237-249) and from_tokens (:251-262); test_command takes only a SafeFilter, so the type is the enforcement and the multi-token contract survives. Already ticked in the body.

The other 20 all reproduce at 558474c9. Three corrections for whoever picks this up:

1. Item 10's count drifted up. rg -c innerHTML stella-observatory/src/assets/index.html → 52, not 48. (Separately, stella-cli/src/export.rs has 11 innerHTML sites; the issue names three renderer functions but no number.)
2. Item 2 cites a file that does not exist. stella-tools/src/settings.rs:557-560 — fd -g settings.rs returns only stella-cli/src/settings.rs and stella-tui/src/views/settings.rs. The claim itself holds and is better cited at stella-tools/src/bash.rs:6-12 ("Opt-in, never ambient… the default tool surface has no shell at all"), against stella-tools/src/registry/process_tools.rs:23 where StartProcess is pushed unconditionally while Bash sits behind if bash { at :37-38.
3. Item 19's diagnosis is wrong in a way that matters for the fix. headless_scope_bypass is not "genuinely dead" — it is in config.rs:49's ROOT_FIELDS validator and is read at agent/engine.rs:152 via headless_scope_bypass_on(). What is broken is only the project-over-user overlay path: it is absent from AgentEngineConfig::overlay's take! list (stella-cli/src/settings.rs:439-454), so a value set in a project settings file never merges up. Same security consequence, different fix.

Everything else verified as described — including stella-media/src/svg.rs:323-347 (keep_attribute has arms for on* and href/xlink:href then references_external, but no "style" attribute arm; the "style" at :316 is the <style> element drop, which is precisely the gap named), the sequential format!("turn-{}", counter.fetch_add(…)) at stella-serve/src/server.rs:247, and the case-sensitive .strip_prefix("Bearer ") at stella-serve/src/http.rs:38-40.