YAGNI/security audit: remove dead code+submodules, harden CLI input, validate spawn-agent skill - #16
Merged
Merged
Conversation
…-templates) Neither submodule was referenced anywhere in code, the Makefile, CI, or docs beyond a project-structure tree diagram in README.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dead code Security audit findings: - q CLI: reject branch names outside [A-Za-z0-9._/-] (blocks flag injection, absolute paths, path traversal) and task text with control characters, across every command that accepts --branch/--task. print_agent_status additionally verifies the resolved path stays inside $AGENTS_HOME. - config/entrypoint.sh: escape task text and commit messages before interpolating them into status.json, so quotes/newlines in a task description can no longer produce invalid JSON. YAGNI cleanup: - Drop the unused "rich" direct dependency (Typer already pulls it in transitively; nothing in this package imports it). - Drop a needless from __future__ import annotations and a one-use agents_app alias. - Remove plans/testing-ci-acceptance-tdd.md — the plan it describes is already implemented and undocumented elsewhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…efix - SKILL.md: add a "Step 0" that requires assigning BRANCH/TASK through a single-quoted heredoc before it ever touches a shell-evaluated command string, so user task text can't trigger host-side $(...)/backtick expansion when inlined into container run / make invocations. - evals/: replace the hardcoded "qubits-team" project-name prefix with a generic <project-name> (basename of git root) derivation, so the evals stay valid if the repo is cloned under a different directory name. Re-validated with a fresh iteration-3 eval run (11 evals, with_skill vs. pre-edit baseline): 100%/100% pass rate, delta +0.00 — no regression, and the new Step 0 guidance is followed correctly in the sampled responses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Records the multi-agent audit that produced the other commits on this branch: confirmed/refuted findings, what was fixed vs. deliberately deferred, and the e2e/skill-eval verification that followed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Multi-agent audit of the repo (dead code / YAGNI, security in
app/cli/src/, spawn-agent skill), plus the follow-up work requested to close out the deferred items. Full narrative inlog.md.model/gemma3-finetunningandapp/agents-templates— neither was referenced anywhere outside a README tree diagram.qCLI now rejects malformed/malicious--branchvalues (blocks flag injection, absolute paths, path traversal) and--taskvalues containing control characters, on every command that accepts them.print_agent_statusverifies the resolved status-file path stays inside$AGENTS_HOME.config/entrypoint.shnow JSON-escapes task text and commit messages before writingstatus.json, so a quote or newline in a task description can't produce invalid JSON.richdirect dependency, a needlessfrom __future__ import annotations, a one-use alias, and a stale completed plan doc.SKILL.mdnow requires assigningTASK/BRANCHvia a single-quoted heredoc (prevents host-side$(...)/backtick expansion when a user's task text gets inlined into a shell-evaluated command), and the eval fixtures no longer hardcode thequbits-teamproject name. Re-validated with a fresh eval run (iteration-3): 11/11 evals, 100%/100% pass rate vs. the pre-edit baseline, delta +0.00 — no regression, and the new heredoc guidance is followed correctly.~/.claude/skills/spawn-agent/(the global, non-versioned skill install) to match this repo's copy — it had drifted and was missing theAGENT_MODELsection and the new heredoc guidance.Deliberately deferred / not in this PR: the OAuth token remaining visible in
container run's host process argv (config/Makefile:100,130) — a fix was designed (--env-file+ amktemp/trap-cleaned temp file, confirmed supported by Apple Container CLI) but paused at the user's request; seelog.mdfor the exact diff if picked back up later.Test plan
uv run pytest(unit + acceptance,app/cli/) — 135 passeduv run ruff check .— cleanmake mutation-ci-threshold— 95.2% (178/187), threshold 70%shellspec --shell bash(entrypoint,config/) — 59 examples, 0 failuresSTACKAI_E2E=1 uv run pytest tests/e2e -v -m e2e(real containers) — 2 passed (Claude agent round-trip + PI agent round-trip against a rebuiltclaude-pi:ubuntuimage and a live localmlx_lm.server).claude/skills/spawn-agent-workspace/iteration-3/, gitignored) — 100%/100%, delta +0.00 vs. iteration-2 baseline🤖 Generated with Claude Code