Skip to content

docs(research): suites are safe to print; project-pack.py echoes operator env values (#345) - #439

Open
tucktuck101 wants to merge 1 commit into
launchpadfrom
research/345-test-output-disclosure
Open

docs(research): suites are safe to print; project-pack.py echoes operator env values (#345)#439
tucktuck101 wants to merge 1 commit into
launchpadfrom
research/345-test-output-disclosure

Conversation

@tucktuck101

Copy link
Copy Markdown
Collaborator

Summary

Answers #345 by inducing real failures in the unrun suites and reading what they print. The suites are safe — project-intelligence reads no environment at all, and test_project_pack.py is deliberately hermetic. But the tool one of them tests, project-pack.py, writes the operator's real environment values into its rendered output and sends that to stdout by default. The tests never reach it; anything running the projector in CI would.

Related issue

Closes #345

Issue type

Task


Agent provenance

Field Value
Harness / provider Claude Code
Model claude-opus-5[1m]
Session reference N/A - the harness exposes no shareable run id
Initiating human @tucktuck101

Objective

Add launchpad/Research/345-test-output-disclosure.md recording what the currently-unrun cohort Python suites emit on failure and whether any of it is unsafe for a public CI log.

Impacted components

launchpad/Research/345-test-output-disclosure.md

Approach and rejected alternatives

Induced failures by mutating each production module rather than by editing assertions into the test files. A mutation produces a genuine failure through the code's own paths, which is what #345 asked to see; hand-writing assert False would have produced output shaped by my edit rather than by the suite.

Then classified the output by kind — absolute path, quoted source line, repr of asserted values — because only the third is variable, and reasoning about the risk means reasoning about what those values can hold. That is what led to checking environment access across all twelve files, which is where the real finding came from.

Rejected reporting "the suites are safe" and stopping. The question behind #345 is whether wiring this directory into a public log is safe, and the honest answer required following the tool the suite tests, not only the suite.

Verification

Command run:

git rev-parse HEAD
# per module: mutate first comparison, run its suite, restore
python3 -m unittest test_graph test_memory test_semantic_index test_symbol test_indexer
grep -rn "os.environ\|getenv\|BUZZ_\|TOKEN\|SECRET\|KEY" launchpad/project-intelligence/*.py
grep -n "os.environ\|getenv\|TOKEN\|SECRET\|api_key\|password" launchpad/agents/*.py
sed -n '179,206p' launchpad/agents/project-pack.py
grep -n "stderr" launchpad/project-intelligence/investigator.py
git status --short

Raw output:

######## test_graph (module graph.py mutated)          Ran 14 tests   OK    exit=0
######## test_memory (module memory.py mutated)        Ran 33 tests   OK    exit=0
######## test_semantic_index (mutated)                 Ran 27 tests   FAILED (failures=1)  exit=1
######## test_symbol (module symbol.py mutated)        Ran 2 tests    OK    exit=0
######## test_indexer (module indexer.py mutated)      Ran 7 tests    OK    exit=0

# the one real failure, complete
FAIL: test_a_zero_vector_has_zero_similarity_not_a_division_error (test_semantic_index.EmbedAndCosineSimilarityTest...)
Traceback (most recent call last):
  File "/Users/jeff/group-build-project/buzz__worktrees/testing/launchpad/project-intelligence/test_semantic_index.py", line 171, ...
    self.assertEqual(cosine_similarity(empty, non_empty), 0.0)
AssertionError: 1.0 != 0.0

# environment access across project-intelligence -- only comments and one fixture string
graph.py:46:    source: str  # qualified_name of a Symbol, ... ("config:KEY",
graph.py:61:    corresponding Symbol, namespaced as "config:KEY" / "doc:PATH" /
graph.py:108:                # Namespaced ("config:KEY"), not the bare key ...
test_semantic_index.py:51:    signature="pub fn encode_v2_code(secret: &[u8; V2_SECRET_LEN]) -> String",

# launchpad/agents/project-pack.py -- reads the live environment
project-pack.py:231:        buzz_bin = find_buzz_binary(REPO_ROOT, os.environ)
project-pack.py:235:        kept, skipped = apply_operator_precedence(pairs, os.environ)

# and writes the operator's value into the rendered output
            skipped.append((key, value, environ[key]))
    for key, resolved_value, operator_value in skipped:
        lines.append(
            f"# skipped {key}: operator env already sets it to "
            f"{shlex.quote(operator_value)} (pack would have projected "
            f"{shlex.quote(resolved_value)})"
        )
        sys.stdout.write(rendered)

# same shape, smaller
investigator.py:137:        raise RuntimeError(f"search_text: grep exited {result.returncode}: {result.stderr!r}")

$ git status --short
(empty - all mutations restored)
  • Tests or checks were run and the raw output is pasted above
  • The diff is confined to the scope of the linked issue
  • No secrets, keys, tokens or hostnames were added to tracked files

Not verified

Only one induced failure produced output. Four mutations were not caught, so there is one real failure sample rather than five. Other failure modes — an exception inside a module rather than a failed assertion — would print differently and were not observed.

test_investigator was excluded from the mutation run, and it is the suite most likely to print something unexpected, since it spawns real subprocesses including cargo test (#329).

I did not run the projector, with real or fake environment. The disclosure path is read from source, not observed; running it with a synthetic variable set would confirm it in seconds.

The suites that already run in CI were not auditedlaunchpad/scripts/ and launchpad/review-agent/. #279 covers one instance in the former.

No judgement that the projector's behaviour is wrong. Writing an operator's own value back to that operator's own terminal may be entirely intended; what is established is that the output is unsafe to route into a public log.

The four surviving mutations are weak evidence about test strength — they were mechanical, and several probably landed on lines no test reaches.

Security implications

This is the security-relevant finding of the document, so stated plainly.

project-pack.py reads os.environ and, for any variable the pack projects that the operator has already set, writes the operator's real value into the rendered env file as a comment — then writes that file to stdout unless --out is passed. The projector's domain includes credentials: CLAUDE.md's Agent CLI section documents BUZZ_PRIVATE_KEY and BUZZ_AUTH_TAG as the auth environment the ACP harness injects.

No leak is claimed. The tests do not reach this path, no CI job runs the projector today, and no tracked file contains a secret. What exists is a code path whose output is unsafe to print in a public log — the same shape as #279, in a different file, in the directory criterion 3 is about to wire into CI. Nothing in this PR changes that behaviour; it records it.

Escalations

  1. project-pack.py's operator-value echo needs a decision and is not mine to take. The skip comment's stated purpose is to explain why a variable was skipped, which arguably needs only the name — but whether the value is load-bearing for operators should be judged by someone who knows the workflow. I have deliberately not filed a separate issue, since the routing (public issue versus private advisory) is a judgement call under launchpad/AGENTS.md §8 and this is a code path rather than an exposed secret.
  2. Criterion 3 should probably not wire the projector into CI until item 1 is settled, even though the test_project_pack.py suite itself is safe. That distinction is easy to lose when a directory is added wholesale.
  3. investigator.py:137 is the same defect class as task: stop raw stderr from an abnormal gitleaks exit reaching public CheckResult.detail #279 and I would fold it into that thread rather than file it separately.

…ator env values (#345)

Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>

@serina-mcfall serina-mcfall left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker — the credential claim is not supported by the code, and it is the note's entire severity argument

The projector's own domain includes credentials — CLAUDE.md's Agent CLI section documents BUZZ_PRIVATE_KEY and BUZZ_AUTH_TAG as the auth environment the ACP harness injects.

The echo only fires for a key that is both projected by the pack and present in the operator's environment. That projected set is closed, and no credential is in it:

$ sed -n '117,160p' launchpad/agents/project-pack.py
  pairs = [(k, v) for k, v in persona.get("runtime_env_vars", [])]
  pairs.append(("BUZZ_ACP_AGENT_COMMAND", runtime))
  pairs.append(("BUZZ_ACP_AGENT_ARGS", "acp"))
  # ... plus at most one BUZZ_ACP_MCP_COMMAND

$ sed -n '367,400p' crates/buzz-persona/src/resolve.rs | grep -oE '"[A-Z_]+"' | sort -u
"BUZZ_AGENT_MODEL"
"BUZZ_AGENT_PROVIDER"
"GOOSE_CONTEXT_LIMIT"
"GOOSE_MODEL"
"GOOSE_PROVIDER"
"GOOSE_TEMPERATURE"

runtime_env_vars is itself closed — six keys, none of them a credential, and no mechanism for a pack to declare an arbitrary one. So BUZZ_PRIVATE_KEY and BUZZ_AUTH_TAG never enter pairs, which means key in environ is never evaluated for them and they can never reach skipped. Provider API keys are explicitly out of scope by the pack's own design (the-professor/README.md:141-142).

CLAUDE.md does document those two variables. What it does not say — and what the note infers — is that the projector projects them.

Why this blocks. In a disclosure note the threat model is the deliverable. This claim is what converts "an operator sees their own value on their own terminal" into "a credential in a public log", and it carries Escalations 1 and 2 and Recommendation 3. It is the same defect class as the one upheld on #405: a conclusion attributed to a source that does not support it, in a document whose genre is precisely "trust this, it was checked."

The underlying finding is real and survives. project-pack.py does echo operator environment values into a comment written to stdout. The remediation is unchanged. Only the characterisation of what can leak needs correcting — and the honest version ("whatever a persona's model, provider, temperature and context-limit resolve to") is still worth writing down.

High — "the suites are safe to print" is universal; the disqualifying suite is admitted 150 lines later

The Finding headline is "The suites are safe to print", justified by "everything these suites assert over is derived from the repository — and the repository is public."

True for five of six. test_investigator.py spawns real subprocesses (subprocess.run at 119, 218, 270, 274; Popen at 251) against investigator.py, which shells out to rql, grep and cargo capturing stdout/stderr. That output is machine-derived, not repository-derived, so the stated justification does not cover it.

The note knows — "test_investigator was excluded from the mutation run… it is the most likely of the six to print something unexpected" — but that sits in "Confidence and what was not checked", at the bottom. Meanwhile Recommendation 1 says "wire the six project-intelligence suites into CI without redaction work": six, including the unchecked one, and the one whose module the note itself flags at investigator.py:137 as a #279-class stderr-into-output path.

High — a bracketed substitution changes what the quoted docstring asserts

The note quotes test_project_pack.py as saying it "does not exercise inspect_pack()/find_buzz_binary() against a [real environment]", and concludes the file "is safe for a stronger reason: it says so on purpose."

The actual docstring says "against a real buzz binary … so they run anywhere without a cargo build." The stated reason is build portability, not environment hermeticity. The bracket replaces the one word carrying the claim.

The conclusion is independently true — there is no os.environ/getenv in the file and the call sites pass literal dicts — so this is High rather than Blocker. But "it says so on purpose" should go; cite the literal-dict call sites instead, which are actual proof.

Medium — the note publishes an instance of the disclosure class it defines

The verbatim failure block contains /Users/jeff/group-build-project/buzz__worktrees/testing/…, and the note's own item 1 classifies an absolute path exposing a developer's home directory as a disclosure category.

I considered this as a Blocker and decided against it: no credential is present, and the identity is already public in this repository's own commit metadata. What is newly published is a local worktree layout — real, but not the class the rule exists to stop. One-line fix, and eliding it demonstrates the note's own point.

What is right

  • The core mechanism is correct, and every line number is accurate at the pinned SHA. apply_operator_precedence records the live value at 181, render_env_file interpolates it at 199-204, main writes to stdout at 244. The one-line offset from the current tree is explained by the apostrophe fix in 709c7709. That is better citation hygiene than most notes here.
  • The environment-access enumeration reproduces byte-for-byte across all 12 files — three comments and one fixture, and the claim is enumerative, not sampled.
  • "No CI job runs the projector today" holds.
  • test_project_pack.py really is environment-hermetic.
  • The "Not checked" section is specific and falsifiable rather than decorative.

Reviewed at head 81c0cb48c. The closed key set was verified by me directly against project-pack.py and resolve.rs.

🤖 Review drafted by Claude Code (claude-opus-5) for @serina-mcfall.

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

Labels

by:agent Filed or authored by an AI agent, not a human

Projects

None yet

Development

Successfully merging this pull request may close these issues.

task: find out what the cohort's unrun Python suites print on failure before wiring them into public CI logs

2 participants