diff --git a/.claude/skills/spawn-agent/SKILL.md b/.claude/skills/spawn-agent/SKILL.md index ec4b3aa..c9d52ac 100644 --- a/.claude/skills/spawn-agent/SKILL.md +++ b/.claude/skills/spawn-agent/SKILL.md @@ -120,6 +120,31 @@ Work autonomously, read the codebase as needed, and commit any changes. ## Spawning an agent +**Step 0: Assign BRANCH and TASK safely — never inline user text** + +User-provided task text must NEVER be typed literally inside a double-quoted +shell argument: `$(...)`, backticks and `$VAR` expand on the **host** shell +before the container ever sees them. Always assign through a single-quoted +heredoc first, then expand only the variable: + +```bash +# SAFE: single-quoted delimiter ('EOF') disables all expansion of the body +TASK=$(cat <<'EOF' + +EOF +) +BRANCH=feat/my-feature # only [A-Za-z0-9._/-], no leading '-' or '/', no '..' +``` + +```bash +# UNSAFE — never do this (host expands $(...) inside double quotes): +container run ... --task "Fix the bug in $(parse_user_input)" +``` + +Branch names must match `[A-Za-z0-9._/-]`, start with an alphanumeric, and +contain no `..` — the `q` CLI rejects anything else, and raw `container run` +invocations must follow the same rule. + **Step 1: Check env vars** ```bash test -n "$CLAUDE_CONTAINER_OAUTH_TOKEN" || echo "ERROR: export CLAUDE_CONTAINER_OAUTH_TOKEN=" diff --git a/.claude/skills/spawn-agent/evals/evals.json b/.claude/skills/spawn-agent/evals/evals.json index 76e2b60..c5d5e0d 100644 --- a/.claude/skills/spawn-agent/evals/evals.json +++ b/.claude/skills/spawn-agent/evals/evals.json @@ -32,11 +32,11 @@ { "id": 3, "prompt": "Show me what agents are currently running. Also list the worktrees that exist.", - "expected_output": "Claude runs container list filtered by the project prefix (qubits-team), and also lists the worktrees directory. Shows the results in a readable format.", + "expected_output": "Claude runs container list filtered by the project prefix (basename of the git root, e.g. stackai), and also lists the worktrees directory. Shows the results in a readable format.", "files": [], "expectations": [ "Runs `container list` (not `container ps` or other docker-style commands)", - "Filters output by project prefix (e.g., grep qubits-team)", + "Filters output by project prefix (basename of git root, e.g., grep stackai)", "Also shows worktrees on disk (ls on WORKTREES_DIR or equivalent)", "Does NOT attempt to spawn a new agent" ] diff --git a/.claude/skills/spawn-agent/evals/list_and_monitor.md b/.claude/skills/spawn-agent/evals/list_and_monitor.md index 84f873e..bf72aae 100644 --- a/.claude/skills/spawn-agent/evals/list_and_monitor.md +++ b/.claude/skills/spawn-agent/evals/list_and_monitor.md @@ -13,7 +13,7 @@ User says: ## Expected behavior (list) 1. Skill triggers -2. Runs: `container list 2>/dev/null | grep "qubits-team"` +2. Runs: `container list 2>/dev/null | grep ""` (basename of git root, e.g. `stackai`) 3. Also shows worktrees on disk (with status from `status.json` if available) 4. Presents output in a readable format to the user @@ -29,7 +29,7 @@ User says: 1. Skill triggers 2. Reads `status.json` from `$AGENTS_HOME/feat/jwt-auth/.agent/status.json` for quick status 3. If more detail needed, reads container logs or persisted `.agent/agent.log` -4. Sanitizes container name correctly: `feat/jwt-auth` → `qubits-team-feat-jwt-auth` +4. Sanitizes container name correctly: `feat/jwt-auth` → `-feat-jwt-auth` 5. **Reads and summarizes** the output — does NOT just dump raw logs 6. Tells user: agent is working on X, currently at step Y, last action was Z diff --git a/.claude/skills/spawn-agent/evals/multi_agent.md b/.claude/skills/spawn-agent/evals/multi_agent.md index 36d3fc4..893c1b4 100644 --- a/.claude/skills/spawn-agent/evals/multi_agent.md +++ b/.claude/skills/spawn-agent/evals/multi_agent.md @@ -22,7 +22,7 @@ User says: 4. Launches 3 `container run -d` commands sequentially 5. Lists all 3 containers at the end with: ```bash - container list | grep "qubits-team" + container list | grep "" # basename of git root, e.g. stackai ``` 6. Tells user how to monitor each one diff --git a/.claude/skills/spawn-agent/evals/spawn_feature.md b/.claude/skills/spawn-agent/evals/spawn_feature.md index 6c9b99f..4494a18 100644 --- a/.claude/skills/spawn-agent/evals/spawn_feature.md +++ b/.claude/skills/spawn-agent/evals/spawn_feature.md @@ -16,7 +16,7 @@ User says: 2. Determines agent type = **feature** 3. Detects git root from current directory 4. Builds task prompt for feature type mentioning "JWT authentication in the API" -5. Sanitizes branch: `feat/jwt-auth` → container name `qubits-team-feat-jwt-auth` +5. Sanitizes branch: `feat/jwt-auth` → container name `-feat-jwt-auth` (e.g. `stackai-feat-jwt-auth`) 6. Checks `CLAUDE_CONTAINER_OAUTH_TOKEN` is set (warns if not) 7. Runs `container run -d --rm ...` with: - `--worktree feat/jwt-auth` diff --git a/.claude/skills/spawn-agent/evals/stop_agent.md b/.claude/skills/spawn-agent/evals/stop_agent.md index a724f4c..8412327 100644 --- a/.claude/skills/spawn-agent/evals/stop_agent.md +++ b/.claude/skills/spawn-agent/evals/stop_agent.md @@ -14,7 +14,7 @@ User says: 1. Skill triggers 2. Sanitizes branch for container name: `feat-jwt-auth` -3. Runs: `container stop qubits-team-feat-jwt-auth` +3. Runs: `container stop -feat-jwt-auth` (e.g. `stackai-feat-jwt-auth`) 4. If user asked to clean worktree, also runs: ```bash git -C worktree remove --force /feat/jwt-auth diff --git a/.gitmodules b/.gitmodules deleted file mode 100644 index 39d6041..0000000 --- a/.gitmodules +++ /dev/null @@ -1,6 +0,0 @@ -[submodule "model/gemma3-finetunning"] - path = model/gemma3-finetunning - url = https://github.com/deimagjas/machinelearning -[submodule "app/agents-templates"] - path = app/agents-templates - url = https://github.com/deimagjas/agents-templates diff --git a/CLAUDE.md b/CLAUDE.md index 5c4be47..3a099ff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -114,8 +114,8 @@ The rules below cannot be enforced by Ruff and must be applied by hand: ## Architecture - **`config/`** — Container infrastructure: `Dockerfile.wolfi` (production, multi-stage: Rust tool compilation → runtime with Claude CLI, Node, Python), `entrypoint.sh` (credential injection + worktree creation + su-exec privilege drop), `Makefile` (orchestration) -- **`app/cli/`** — Python CLI (`q`) using Typer+Rich that wraps Makefile targets. Entry point registered as `q` in pyproject.toml. Commands delegate to `make` via `utils.run_make()` -- **`.claude/skills/`** — Host-side Claude Code skills for multi-agent orchestration (spawn-agent, spawn-agent-workspace) +- **`app/cli/`** — Python CLI (`q`) using Typer that wraps Makefile targets. Entry point registered as `q` in pyproject.toml. Commands delegate to `make` via `utils.run_make()` +- **`.claude/skills/`** — Host-side Claude Code skills for multi-agent orchestration (spawn-agent); `spawn-agent-workspace/` is the gitignored eval-output workspace (see `docs/agents/evals.md`) - **`docs/agents/`** — All project documentation (container reference, CLI, setup/auth, skill architecture, evals) ### Key concepts diff --git a/README.md b/README.md index 4c10196..899714f 100644 --- a/README.md +++ b/README.md @@ -147,8 +147,7 @@ make stop-agent BRANCH=feat/oauth2 # stop when done ``` stackai/ ├── app/ -│ ├── cli/ # Python CLI (q command) -│ └── agents-templates/ # Agent template examples +│ └── cli/ # Python CLI (q command) ├── config/ │ ├── Dockerfile.wolfi # Production image (ARM64, glibc) │ ├── Dockerfile # CI image (Alpine, amd64) @@ -161,8 +160,7 @@ stackai/ │ ├── setup.md # Authentication guide │ ├── evals.md # Evaluation framework │ └── cli.md # CLI command reference -├── iac/ # Infrastructure as Code -└── model/ # ML fine-tuning experiments +└── iac/ # Infrastructure as Code ``` ## How it works diff --git a/app/agents-templates b/app/agents-templates deleted file mode 160000 index b715e47..0000000 --- a/app/agents-templates +++ /dev/null @@ -1 +0,0 @@ -Subproject commit b715e4799cb528aa2cab9acee2fc5a8b6b3fc24b diff --git a/app/cli/pyproject.toml b/app/cli/pyproject.toml index f29f2d1..d099d75 100644 --- a/app/cli/pyproject.toml +++ b/app/cli/pyproject.toml @@ -2,7 +2,7 @@ name = "qubits-cli" version = "0.1.0" requires-python = ">=3.13" -dependencies = ["typer>=0.12", "rich>=13"] +dependencies = ["typer>=0.12"] [project.scripts] q = "container_cli.main:app" diff --git a/app/cli/src/container_cli/commands/agents.py b/app/cli/src/container_cli/commands/agents.py index c99c850..0965692 100644 --- a/app/cli/src/container_cli/commands/agents.py +++ b/app/cli/src/container_cli/commands/agents.py @@ -5,7 +5,13 @@ import typer from container_cli.targets import Target -from container_cli.utils import check_token, print_agent_status, run_make +from container_cli.utils import ( + check_token, + print_agent_status, + run_make, + validate_branch, + validate_task, +) app = typer.Typer(help="Agent lifecycle commands") @@ -23,6 +29,8 @@ def spawn( ] = None, ) -> None: """Spawn a detached headless agent container.""" + validate_branch(branch) + validate_task(task) check_token() make_vars: dict[str, str] = {"BRANCH": branch, "TASK": task} if cpus is not None: @@ -47,6 +55,7 @@ def logs( branch: Annotated[str, typer.Option("--branch", help="Agent branch name")], ) -> None: """Show logs for a branch agent.""" + validate_branch(branch) run_make(Target.LOGS_AGENT, {"BRANCH": branch}) @@ -55,6 +64,7 @@ def follow( branch: Annotated[str, typer.Option("--branch", help="Agent branch name")], ) -> None: """Follow live streaming logs for a branch agent.""" + validate_branch(branch) run_make(Target.FOLLOW_AGENT, {"BRANCH": branch}, tty=True) @@ -63,6 +73,7 @@ def stop( branch: Annotated[str, typer.Option("--branch", help="Agent branch name")], ) -> None: """Stop a branch agent container.""" + validate_branch(branch) run_make(Target.STOP_AGENT, {"BRANCH": branch}) @@ -71,6 +82,7 @@ def status( branch: Annotated[str, typer.Option("--branch", help="Agent branch name")], ) -> None: """Show agent status from persisted status.json file.""" + validate_branch(branch) print_agent_status(branch, label="status") @@ -79,4 +91,5 @@ def summary( branch: Annotated[str, typer.Option("--branch", help="Agent branch name")], ) -> None: """Show structured lifecycle events for a branch agent.""" + validate_branch(branch) run_make(Target.SUMMARY_AGENT, {"BRANCH": branch}) diff --git a/app/cli/src/container_cli/commands/pi_agents.py b/app/cli/src/container_cli/commands/pi_agents.py index 09bb976..7a9f281 100644 --- a/app/cli/src/container_cli/commands/pi_agents.py +++ b/app/cli/src/container_cli/commands/pi_agents.py @@ -8,14 +8,12 @@ build.py are not modified — pi commands live under their own subapp. """ -from __future__ import annotations - from typing import Annotated import typer from container_cli.targets import Target -from container_cli.utils import print_agent_status, run_make +from container_cli.utils import print_agent_status, run_make, validate_branch, validate_task app = typer.Typer(help="PI agent lifecycle (local mlx_lm.server backend)") @@ -63,6 +61,8 @@ def spawn( The mlx_lm.server must be running on the host. Check with: uv run iac server status """ + validate_branch(branch) + validate_task(task) typer.echo( "[pi] reminder: ensure mlx_lm.server is running (`uv run iac server status` from /iac)" ) @@ -91,6 +91,7 @@ def logs( branch: Annotated[str, typer.Option("--branch", help="PI agent branch name")], ) -> None: """Show logs for a PI agent (live container or persisted log).""" + validate_branch(branch) run_make(Target.LOGS_PI_AGENT, {"BRANCH": branch}) @@ -99,6 +100,7 @@ def follow( branch: Annotated[str, typer.Option("--branch", help="PI agent branch name")], ) -> None: """Follow live streaming logs for a PI agent.""" + validate_branch(branch) run_make(Target.FOLLOW_PI_AGENT, {"BRANCH": branch}, tty=True) @@ -107,6 +109,7 @@ def stop( branch: Annotated[str, typer.Option("--branch", help="PI agent branch name")], ) -> None: """Stop a PI agent container.""" + validate_branch(branch) run_make(Target.STOP_PI_AGENT, {"BRANCH": branch}) @@ -115,4 +118,5 @@ def status( branch: Annotated[str, typer.Option("--branch", help="PI agent branch name")], ) -> None: """Show PI agent status from persisted status.json file.""" + validate_branch(branch) print_agent_status(branch, label="pi-status") diff --git a/app/cli/src/container_cli/main.py b/app/cli/src/container_cli/main.py index dbb0ff8..6952ac2 100644 --- a/app/cli/src/container_cli/main.py +++ b/app/cli/src/container_cli/main.py @@ -5,7 +5,6 @@ from container_cli.commands import agents, build, network, pi_agents, run app = typer.Typer(name="q", help="Container management CLI for Claude agent containers") -agents_app = agents.app # Register top-level commands from build module app.command("build")(build.build) @@ -24,7 +23,7 @@ app.command("spawn")(agents.spawn) # Register agents sub-app -app.add_typer(agents_app, name="agents") +app.add_typer(agents.app, name="agents") # Register PI agent sub-app (extension — local mlx_lm backend, no Claude token) app.add_typer(pi_agents.app, name="pi") diff --git a/app/cli/src/container_cli/utils.py b/app/cli/src/container_cli/utils.py index b004724..617e49f 100644 --- a/app/cli/src/container_cli/utils.py +++ b/app/cli/src/container_cli/utils.py @@ -2,6 +2,7 @@ import json import os +import re import subprocess from pathlib import Path @@ -9,6 +10,11 @@ from container_cli.targets import Target +# Branch names must start with an alphanumeric and use only safe characters, +# so a value can never be parsed as a flag (leading `-`), an absolute path +# (leading `/`), or a shell word boundary once it reaches make/git. +_BRANCH_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._/-]*") + def find_git_root() -> Path: """Return the absolute path of the repository root. @@ -51,6 +57,40 @@ def agents_home() -> Path: return find_git_root().parent / ".worktrees" +def validate_branch(branch: str) -> None: + """Reject branch names that could be misinterpreted by make, git, or the shell. + + Args: + branch: Candidate git branch name received from the CLI. + + Raises: + typer.Exit: With code 1 when the name is empty, contains `..`, or has + characters outside `[A-Za-z0-9._/-]` (or does not start with an + alphanumeric character). + """ + if not branch or ".." in branch or not _BRANCH_RE.fullmatch(branch): + typer.echo(f"[error] invalid branch name: {branch!r}", err=True) + raise typer.Exit(1) + + +def validate_task(task: str) -> None: + """Reject task descriptions that could smuggle control characters to the host. + + Args: + task: Task prompt text received from the CLI. + + Raises: + typer.Exit: With code 1 when the task is empty or contains control + characters (newlines, carriage returns, tabs, NUL, DEL). + """ + if not task or any(ord(char) < 32 or ord(char) == 127 for char in task): + typer.echo( + "[error] invalid task: must be non-empty and contain no control characters", + err=True, + ) + raise typer.Exit(1) + + def check_token() -> None: """Verify that the Claude container OAuth token is exported. @@ -100,9 +140,14 @@ def print_agent_status(branch: str, *, label: str) -> None: label: Tag used in the not-found messages (e.g. `status`, `pi-status`). Raises: - typer.Exit: With code 1 when no status file exists for the branch. + typer.Exit: With code 1 when the branch resolves outside the worktrees + directory or no status file exists for the branch. """ - status_file = agents_home() / branch / ".agent" / "status.json" + base = agents_home().resolve() + status_file = (agents_home() / branch / ".agent" / "status.json").resolve() + if base not in status_file.parents: + typer.echo(f"[{label}] invalid branch path: {branch!r}", err=True) + raise typer.Exit(1) if not status_file.exists(): typer.echo(f"[{label}] No status file found for branch '{branch}'.") typer.echo(f"[{label}] Expected at: {status_file}") diff --git a/app/cli/tests/acceptance/features/input_validation.feature b/app/cli/tests/acceptance/features/input_validation.feature new file mode 100644 index 0000000..b88525f --- /dev/null +++ b/app/cli/tests/acceptance/features/input_validation.feature @@ -0,0 +1,46 @@ +Feature: Input validation for branch and task arguments + As a user of the q CLI + I want malformed or malicious branch/task values rejected before reaching make + So that shell injection and path traversal cannot reach the host + + Background: + Given the make runner is ready + + Scenario: Spawn rejects a branch with shell metacharacters + Given the CLAUDE_CONTAINER_OAUTH_TOKEN is set + When I run "q spawn --branch 'feat;rm -rf x' --task implement-x" + Then the command exits with an error + And the output contains "invalid branch" + And the make runner was not invoked + + Scenario: Spawn rejects a branch with path traversal + Given the CLAUDE_CONTAINER_OAUTH_TOKEN is set + When I run "q spawn --branch ../../escape --task implement-x" + Then the command exits with an error + And the output contains "invalid branch" + And the make runner was not invoked + + Scenario: Spawn rejects a task containing control characters + Given the CLAUDE_CONTAINER_OAUTH_TOKEN is set + When I run spawn with a task containing a control character + Then the command exits with an error + And the output contains "invalid task" + And the make runner was not invoked + + Scenario: Agent status rejects a branch that escapes the worktrees directory + When I run "q agents status --branch ../../../etc" + Then the command exits with an error + And the output contains "invalid branch" + And the make runner was not invoked + + Scenario: Stop rejects a branch that begins with a dash + When I run "q agents stop --branch=-evil" + Then the command exits with an error + And the output contains "invalid branch" + And the make runner was not invoked + + Scenario: PI spawn rejects a branch with shell metacharacters + When I run "q pi spawn --branch 'pi;evil' --task implement-x" + Then the command exits with an error + And the output contains "invalid branch" + And the make runner was not invoked diff --git a/app/cli/tests/acceptance/steps/input_validation_steps.py b/app/cli/tests/acceptance/steps/input_validation_steps.py new file mode 100644 index 0000000..6d2bbac --- /dev/null +++ b/app/cli/tests/acceptance/steps/input_validation_steps.py @@ -0,0 +1,19 @@ +from pytest_bdd import scenarios, then, when + +from container_cli.main import app +from tests.acceptance.steps.common_steps import * # noqa: F401, F403 + +scenarios("../features/input_validation.feature") + + +@when("I run spawn with a task containing a control character") +def _spawn_with_control_char_task(invocation_context) -> None: + invocation_context.result = invocation_context.runner.invoke( + app, ["spawn", "--branch", "feat/ok", "--task", "do this\nrm -rf x"] + ) + + +@then("the make runner was not invoked") +def _make_runner_not_invoked(invocation_context) -> None: + for name, mock in invocation_context.mocks.items(): + assert not mock.called, f"run_make mock {name!r} was invoked: {mock.call_args_list}" diff --git a/app/cli/tests/test_utils.py b/app/cli/tests/test_utils.py index efa26e8..d4fd276 100644 --- a/app/cli/tests/test_utils.py +++ b/app/cli/tests/test_utils.py @@ -14,6 +14,8 @@ makefile_dir, print_agent_status, run_make, + validate_branch, + validate_task, ) # ---------- find_git_root ---------- @@ -155,6 +157,66 @@ def test_fallback_path_name(self, monkeypatch: pytest.MonkeyPatch): assert agents_home() == Path("/home/user/.worktrees") +# ---------- validate_branch ---------- + + +class TestValidateBranch: + @pytest.mark.parametrize( + "branch", + ["feat/foo", "pi/refactor", "fix/issue-42", "release/v1.2.3", "main", "feat_underscore"], + ) + def test_accepts_normal_branch_names(self, branch: str): + validate_branch(branch) # should not raise + + @pytest.mark.parametrize( + "branch", + [ + "", + "feat;rm -rf x", + "feat && evil", + "feat foo", + "feat`id`", + "feat$(id)", + "feat\nbar", + "../escape", + "feat/../../escape", + "-evil", + "--upload-pack=evil", + "/absolute", + ".hidden", + ], + ) + def test_rejects_malicious_or_malformed_names(self, branch: str, capsys): + with pytest.raises(typer.Exit) as exc_info: + validate_branch(branch) + assert exc_info.value.exit_code == 1 + err = capsys.readouterr().err + assert "invalid branch" in err + + +# ---------- validate_task ---------- + + +class TestValidateTask: + @pytest.mark.parametrize( + "task", + ["implement feature X", "rename ambiguous helpers", "fix bug #42 (edge-case: 'quotes')"], + ) + def test_accepts_normal_task_text(self, task: str): + validate_task(task) # should not raise + + @pytest.mark.parametrize( + "task", + ["", "line1\nline2", "task\rcarriage", "tab\there", "nul\x00byte"], + ) + def test_rejects_empty_or_control_characters(self, task: str, capsys): + with pytest.raises(typer.Exit) as exc_info: + validate_task(task) + assert exc_info.value.exit_code == 1 + err = capsys.readouterr().err + assert "invalid task" in err + + # ---------- print_agent_status ---------- @@ -185,3 +247,27 @@ def test_label_tags_the_not_found_message( with pytest.raises(typer.Exit): print_agent_status("feat-x", label="pi-status") assert "[pi-status]" in capsys.readouterr().out + + def test_rejects_branch_escaping_agents_home( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys + ): + monkeypatch.setenv("AGENTS_HOME", str(tmp_path / "worktrees")) + outside = tmp_path / "outside" / ".agent" / "status.json" + outside.parent.mkdir(parents=True) + outside.write_text('{"phase": "completed"}') + with pytest.raises(typer.Exit) as exc_info: + print_agent_status("../outside", label="status") + assert exc_info.value.exit_code == 1 + captured = capsys.readouterr() + assert "invalid branch" in captured.err + assert "completed" not in captured.out + + def test_allows_branch_with_slash_inside_agents_home( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys + ): + monkeypatch.setenv("AGENTS_HOME", str(tmp_path)) + status_file = tmp_path / "feat" / "foo" / ".agent" / "status.json" + status_file.parent.mkdir(parents=True) + status_file.write_text('{"phase": "working"}') + print_agent_status("feat/foo", label="status") + assert "working" in capsys.readouterr().out diff --git a/app/cli/uv.lock b/app/cli/uv.lock index c344cce..64234ac 100644 --- a/app/cli/uv.lock +++ b/app/cli/uv.lock @@ -455,7 +455,6 @@ name = "qubits-cli" version = "0.1.0" source = { editable = "." } dependencies = [ - { name = "rich" }, { name = "typer" }, ] @@ -468,10 +467,7 @@ dev = [ ] [package.metadata] -requires-dist = [ - { name = "rich", specifier = ">=13" }, - { name = "typer", specifier = ">=0.12" }, -] +requires-dist = [{ name = "typer", specifier = ">=0.12" }] [package.metadata.requires-dev] dev = [ diff --git a/config/entrypoint.sh b/config/entrypoint.sh index 7f4ba60..8459b9e 100644 --- a/config/entrypoint.sh +++ b/config/entrypoint.sh @@ -90,12 +90,25 @@ setup_agent_perms() { chown -R agent:agent "$WORKTREE_PATH" } +json_escape() { + # Escape backslashes, double quotes and control characters so interpolated + # values (task text, commit messages) always produce valid JSON. + local s="$1" + s=${s//\\/\\\\} + s=${s//\"/\\\"} + s=${s//$'\t'/\\t} + s=${s//$'\r'/\\r} + s=${s//$'\n'/\\n} + printf '%s' "$s" +} + write_status() { local phase="$1"; shift local now now=$(date -u +"%Y-%m-%dT%H:%M:%SZ") ( printf '{"phase":"%s","branch":"%s","task":"%s","started_at":"%s"}\n' \ - "$phase" "$WORKTREE_BRANCH" "$AGENT_TASK" "${AGENT_STARTED_AT:-${now}}" \ + "$phase" "$(json_escape "$WORKTREE_BRANCH")" "$(json_escape "$AGENT_TASK")" \ + "${AGENT_STARTED_AT:-${now}}" \ > "${AGENT_DIR}/status.json" ) 2>/dev/null || true } @@ -166,9 +179,9 @@ run_agent() { "exit_code": %d, "commits": %s, "last_commit": "%s" -}\n' "$final_phase" "$WORKTREE_BRANCH" "$AGENT_TASK" \ +}\n' "$final_phase" "$(json_escape "$WORKTREE_BRANCH")" "$(json_escape "$AGENT_TASK")" \ "$AGENT_STARTED_AT" "$finished_at" "$duration_secs" \ - "$exit_code" "$commit_count" "$last_commit" \ + "$exit_code" "$commit_count" "$(json_escape "$last_commit")" \ > "$AGENT_DIR/status.json" ) 2>/dev/null || true emit_marker "$final_phase" "EXIT_CODE=${exit_code}" "COMMITS=${commit_count}" "DURATION=${duration_secs}s" diff --git a/docs/agents/cli.md b/docs/agents/cli.md index f4e1ae8..3519c42 100644 --- a/docs/agents/cli.md +++ b/docs/agents/cli.md @@ -13,7 +13,7 @@ ```bash cd app/cli -uv sync # installs typer, rich, and the q entry point +uv sync # installs typer and the q entry point uv run q --help # verify installation ``` @@ -83,6 +83,13 @@ q spawn --branch feat/quick-fix --task "Tighten the readme" --model sonnet Requires `CLAUDE_CONTAINER_OAUTH_TOKEN` to be set. +> **Input validation:** every command that takes `--branch` rejects names +> outside `[A-Za-z0-9._/-]`, names not starting with an alphanumeric, and any +> name containing `..` — this blocks flag injection, absolute paths, and path +> traversal before the value reaches `make`. `--task` must be non-empty and +> contain no control characters (newlines, tabs, NUL). `status` additionally +> verifies the resolved path stays inside `$AGENTS_HOME`. + > **Agent model:** the agent runs `opus` unless `--model` says otherwise. It > does **not** inherit the host's `~/.claude/settings.json` `model` preference — > that file is copied into the container for credentials only, and a headless diff --git a/log.md b/log.md new file mode 100644 index 0000000..92debb7 --- /dev/null +++ b/log.md @@ -0,0 +1,117 @@ +# Auditoría YAGNI + seguridad — 2026-06-12 + +Auditoría multi-agente (workflow de 20 agentes: 4 finders en paralelo + verificación +adversarial de cada hallazgo). Resultado: 16 hallazgos brutos → **14 confirmados, 2 refutados**. +Todo lo aplicado quedó verificado con la suite completa. + +## Verificación final + +| Gate | Resultado | +|---|---| +| `uv run pytest` (unit + acceptance) | 135 passed (100 previos + 35 nuevos de validación) | +| `uv run ruff check .` | limpio | +| `make mutation-ci-threshold` | 95.2% (178/187), umbral 70% | +| `shellspec --shell bash` (entrypoint) | 59 examples, 0 failures | + +## Código muerto eliminado (YAGNI) + +| Qué | Dónde | Evidencia | +|---|---|---| +| Dependencia directa `rich>=13` | `app/cli/pyproject.toml` | Ningún módulo de `container_cli/` ni sus tests importa `rich`; Typer ya lo trae transitivamente (sigue en `uv.lock` como dep de typer). Docs actualizadas: `CLAUDE.md` («Typer+Rich» → «Typer») y `docs/agents/cli.md`. | +| `from __future__ import annotations` | `commands/pi_agents.py:11` | `requires-python >= 3.13`; toda anotación usada (`str \| None`) es nativa. Los 4 módulos hermanos no lo usan. | +| Alias `agents_app = agents.app` | `main.py:8` | Indirección de un solo uso; ahora se registra directo `app.add_typer(agents.app, ...)`, igual que `pi_agents.app`. | +| `plans/testing-ci-acceptance-tdd.md` | `plans/` (carpeta completa) | Plan ya implementado (gate de mutación y filosofía TDD viven en CLAUDE.md); cero referencias en README, docs, Makefile o CI. | +| `.claude/skills/spawn-agent-workspace/iteration-1/` | (no versionado) | Artefacto de evals superado por `iteration-2/`. Se conservan `iteration-2/` y `skill-snapshot/` — son la línea base de regresión más reciente (única corrida con los evals PI 9–11). | +| `.DS_Store` y `__pycache__` sueltos | repo completo | Limpieza local; ninguno estaba trackeado, `.gitignore` ya los cubre. | + +## Documentación desactualizada corregida + +- **`CLAUDE.md`**: `spawn-agent-workspace` aparecía listado como skill; es el workspace + (gitignorado) de salida de evals. Redactado corregido. +- **`.claude/skills/spawn-agent/evals/`** (5 archivos): prefijo de contenedor hardcodeado + `qubits-team` (nombre antiguo del proyecto) → regla derivada `` (basename + del git root, p. ej. `stackai`). Afectaba `evals.json`, `spawn_feature.md`, + `list_and_monitor.md`, `stop_agent.md`, `multi_agent.md`. + ⚠️ **Pendiente**: re-ejecutar los evals del skill (`/skill-creator:skill-creator run evals…`) + según CLAUDE.md — el runner de evals no está disponible en esta sesión. + +## Código inseguro corregido + +### 1. Validación de entrada en el CLI (`--branch` / `--task`) — severidad ALTA +`q spawn --branch 'foo; rm -rf x'` llegaba sin sanitizar a la recipe del Makefile, donde +el shell del host la expande (inyección de comandos); un branch `../../x` permitía +path traversal en `print_agent_status` (`utils.py`). + +Fix (TDD: Gherkin → unit → implementación): +- `utils.validate_branch()`: solo `[A-Za-z0-9._/-]`, debe empezar por alfanumérico + (bloquea `-flag` y `/abs`), rechaza `..` y vacío. Cableado en los 11 comandos que + reciben `--branch` (agents: spawn/logs/follow/stop/status/summary; pi: spawn/logs/follow/stop/status). +- `utils.validate_task()`: rechaza vacío y caracteres de control (`\n`, `\r`, `\t`, NUL, DEL). +- `print_agent_status()`: guard de contención — el path resuelto debe quedar dentro de + `$AGENTS_HOME` (defensa en profundidad contra traversal). +- Tests nuevos: `tests/acceptance/features/input_validation.feature` (6 escenarios) + + `TestValidateBranch`/`TestValidateTask`/2 tests de traversal en `test_utils.py`. + +### 2. Inyección JSON en `status.json` — `config/entrypoint.sh` +`$AGENT_TASK` y `$last_commit` se interpolaban sin escapar en el `printf` del JSON +(líneas 97 y 159): una tarea o mensaje de commit con `"` producía JSON inválido y +rompía `q agents status` (`json.loads`). Añadida `json_escape()` (escapa `\`, `"`, +`\t`, `\r`, `\n`) aplicada a branch, task y last_commit. Shellspec verde. + +### 3. Skill `spawn-agent` — regla de asignación segura (SKILL.md) +Riesgo verificado: si el agente que sigue el skill escribe el texto del usuario +**literalmente** dentro de comillas dobles (`--task "...$(...)..."`), el host expande +`$(...)`/backticks antes de llegar al contenedor. Añadido «Step 0» al flujo de spawn: +asignar `TASK` vía heredoc con delimitador entre comillas simples (`<<'EOF'`, sin +expansión) y la misma regla de charset de branch que aplica el CLI. + +## Hallazgos REFUTADOS por la verificación adversarial (sin cambios) + +- **«`chmod go+x /root` expone credenciales al usuario agent»** — falso: `x` solo da + traversal, no lectura; `cp` sin `-p` hereda el modo del origen enmascarado por umask + (verificado empíricamente: fuente 600 → copia 600), y `~/.claude.json` del host es 600. +- **«Credenciales copiadas con permisos world-readable por umask»** — falso por la misma + semántica de `cp`: umask solo puede quitar bits, nunca añadirlos. + +## Decisiones diferidas (requieren al dueño del repo / cambio coordinado) + +2. **Fix de fondo del sink TASK en `config/Makefile:132,284`** (`--task "$(TASK)"` se + re-interpola en la recipe). La validación del CLI mitiga el vector, pero el sink sigue: + la corrección correcta es pasar TASK por env var o archivo en vez de argv de make, y + exige tocar `entrypoint.sh` (parsea `--task` de argv) + re-ejecutar `make e2e-test` + con contenedores reales. No aplicado en caliente a propósito. +3. **Token visible en argv de `container run`** (`-e CLAUDE_CODE_OAUTH_TOKEN=…`, + `config/Makefile:100,130`) — visible en `ps` del host mientras el contenedor vive. + Diseño de fix ya elaborado (`--env-file` + archivo temporal `mktemp`/`trap`, confirmado + que Apple Container CLI soporta `--env-file`) pero **explícitamente pausado a pedido + del usuario el 2026-07-27** — se mantiene el mecanismo actual sin cambios por ahora. + +## Seguimiento — 2026-07-27 + +Resolución de los puntos diferidos 1 y 4, más ejecución de los e2e: + +1. **Submódulos eliminados.** `git submodule deinit -f` + `git rm -f` + limpieza de + `.git/modules/` para `model/gemma3-finetunning` y `app/agents-templates`; `.gitmodules` + quedó vacío y se eliminó. `README.md` (diagrama de árbol) actualizado. Verificado: + `git submodule status` vacío, sin referencias residuales en código/docs/CI. +2. **Tests e2e ejecutados** (`app/cli/tests/e2e/`, ambos con contenedores reales): + - `test_pi_agent_e2e.py` — confirmado que lanza el modelo local (`mlx_lm.server`). + Antes de correrlo se reconstruyó `claude-pi:ubuntu` (`make build-pi`); la imagen no + fija versión del paquete `@earendil-works/pi-coding-agent` y el build usa + `--no-cache`, así que el rebuild ya trae la última versión de npm sin tocar el + Dockerfile. + - `test_claude_agent_e2e.py` — corrido también a pedido del usuario, gasta créditos + reales de la API de Anthropic. + - Resultado: **2 passed** en ~3.5 min. Sin contenedores huérfanos tras la limpieza + automática de los tests. +4. **Evals del skill `spawn-agent` re-ejecutados** (iteration-3, vía el flujo del plugin + skill-creator) para validar los cambios de la sesión anterior (Step 0 de asignación + segura de TASK/BRANCH, genericización del prefijo de proyecto en `evals/`). Nota + importante: existe una copia **global** del skill en `~/.claude/skills/spawn-agent/` + que diverge de la copia del repo (le falta el Step 0 y el bloque `AGENT_MODEL`) — hay + que apuntar explícitamente a la ruta del repo al re-ejecutar evals, no a la ruta + genérica de `CLAUDE.md`. Resultado: **11/11 evals, 100% pass rate en ambas + configuraciones (with_skill vs. baseline pre-edición), delta +0.00** — sin regresión. + El nuevo Step 0 se ve aplicado correctamente en las respuestas (heredoc single-quoted + para TASK). Benchmark en + `.claude/skills/spawn-agent-workspace/iteration-3/benchmark.json` (gitignorado). diff --git a/model/gemma3-finetunning b/model/gemma3-finetunning deleted file mode 160000 index 5270901..0000000 --- a/model/gemma3-finetunning +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 52709012f5bb318839c306bd3edba461af03c6e1 diff --git a/plans/testing-ci-acceptance-tdd.md b/plans/testing-ci-acceptance-tdd.md deleted file mode 100644 index 768fdf9..0000000 --- a/plans/testing-ci-acceptance-tdd.md +++ /dev/null @@ -1,258 +0,0 @@ -# Plan: Mutation CI + Acceptance Tests + CLAUDE.md TDD flow - -## Contexto - -El CI carece de enforcement en tests de mutación. No existe una capa que verifique -el comportamiento del CLI desde la perspectiva del usuario (Gherkin/BDD). Además, -se establece una nueva filosofía de desarrollo: los **acceptance tests son la fuente -de verdad** de la aplicación y el **flujo TDD** (acceptance → unit → implementación) -queda documentado en `CLAUDE.md`. - -Este plan ha sido **revisado contra el estado real del código** antes de implementarse. -Las correcciones técnicas y decisiones cerradas están al final. - ---- - -## Parte 1 — Tests de mutación en CI (umbral 70%) - -**Objetivo**: fallar el pipeline si el kill rate cae por debajo del 70%. -Umbral conservador para permitir estabilidad semántica durante refactoring. -Score actual: 98.4% — pasará holgadamente. - -### Implementación inline (sin script `.py` separado) - -El check de threshold se implementa **directamente como target del Makefile** -con `python -c` para evitar crear `app/cli/scripts/`. - -### Archivos - -| Archivo | Cambio | -|---------|--------| -| `app/cli/Makefile` | Target `mutation-ci-threshold`: `mutmut export-cicd-stats \| python -c ''`, sale ≠ 0 si killed/total < 0.70 | -| `app/cli/pyproject.toml` | Comentar `# threshold = 70` en `[tool.mutmut]` (intención documentada) | -| `.github/workflows/ci.yml` | Job `mutation-tests` con `needs: [test-cli]`, artifact `mutants/` | - -### Boceto del target - -```makefile -mutation-ci-threshold: mutation-run - @uv run mutmut export-cicd-stats | python -c '\ -import json, sys; \ -d = json.load(sys.stdin); \ -killed, total = d["killed"], d["total"]; \ -score = killed/total if total else 0; \ -sys.exit(0 if score >= 0.70 else (print(f"FAIL: {score:.1%} < 70%") or 1))' -``` - -### CI job structure - -``` -mutation-tests: - needs: [test-cli] - steps: checkout → uv sync → make mutation-ci-threshold - artifact: mutants/ (always, 14 días) -``` - ---- - -## Parte 2 — Acceptance tests (pytest-bdd, solo local) - -**Objetivo**: verificar comportamiento del CLI desde la perspectiva del usuario, -expresado en Gherkin. Son **locales únicamente** — GitHub Actions no tiene Apple -Container CLI. Son el gate de calidad antes de PR. - -### Acceptance tests como fuente de verdad - -Los acceptance tests definen el comportamiento contratado de la aplicación. -No se modifican sin acuerdo explícito. Los unit tests y la implementación -deben estar al servicio de los acceptance tests, no al revés. - -### Qué distingue acceptance de unit tests - -Unit tests llaman funciones Python directamente con `mock_run_make`. -Acceptance tests invocan el CLI como usuario via `CliRunner.invoke(app, [...])`, -expresados en lenguaje de negocio. El mock de `run_make` se mantiene (sin -container real), pero el entry point es la interfaz pública del CLI. - -### Estructura de directorios - -``` -app/cli/tests/ -└── acceptance/ - ├── __init__.py - ├── conftest.py # fixture invocation_context: patches run_make en los 4 - │ # módulos + find_git_root + AGENTS_HOME cleanup; expone - │ # mocks + CliRunner + tmp_path - ├── features/ - │ ├── spawn.feature # 3 escenarios: token válido, sin token, con recursos - │ ├── agents.feature # 4 escenarios: list, status ok, status missing, stop - │ ├── build.feature # 3 escenarios: defaults, imagen custom, clean-all - │ └── network.feature # 2 escenarios: defaults, subnet custom - └── steps/ - ├── __init__.py - ├── common_steps.py # Given/When/Then compartidos - ├── spawn_steps.py # llama scenarios("../features/spawn.feature") - ├── agents_steps.py # llama scenarios("../features/agents.feature") - ├── build_steps.py # llama scenarios("../features/build.feature") - └── network_steps.py # llama scenarios("../features/network.feature") -``` - -### Decisiones técnicas clave (revisadas) - -- **`CliRunner()` sin `mix_stderr`** — Click 8.3.1 removió ese flag; `result.output` ya - mezcla stdout y stderr cuando se usa `typer.echo`. -- **`invocation_context` fixture** activa/desactiva patches vía yield; expone: - - `runner` (`CliRunner()`) - - `mocks` (dict de `run_make` mocks por módulo) - - `git_root` (tmp_path como root patcheado en `find_git_root`) - - `monkeypatch` (para gestionar env vars) - - Limpia `AGENTS_HOME` al inicio (`monkeypatch.delenv("AGENTS_HOME", raising=False)`) -- **No mockear `check_token`** — el escenario "sin token" se valida por ausencia real - de la env var `CLAUDE_CONTAINER_OAUTH_TOKEN`. -- **`agents status` no usa `run_make`** — lee `status.json` desde el filesystem. - El escenario "status ok" debe **crear** físicamente - `/../.worktrees//.agent/status.json` antes del `When`. - El escenario "status missing" simplemente no crea el archivo y assertea `exit_code == 1`. -- **`"the make runner is ready"` step es no-op** — la fixture ya activó los mocks. -- **`clean/clean-all/clean-network` llaman `run_make("target")` sin segundo arg; - `build`/`network` llaman `run_make("target", {})`** — distinción importante para asserts. - -### Comandos no cubiertos (deuda técnica) - -`agents logs`, `agents follow`, `agents summary` también usan `run_make` pero -quedan fuera de los 12 escenarios iniciales. Documentar para iteración futura. - -### Cambios en archivos existentes - -| Archivo | Cambio | -|---------|--------| -| `app/cli/pyproject.toml` | `pytest-bdd>=8` en dev deps; `testpaths = ["tests", "tests/acceptance"]` | -| `app/cli/Makefile` | Targets `acceptance-test`, `test-all`, `eval-skills`, `local-qa` | - -### Eval target - -Los evals del skill `spawn-agent` (scenarios LLM-graded en `evals.json`) se -invocan via Claude Code CLI. **Solo cubre `spawn-agent`** — cuando se añadan -más skills, se extenderá manualmente. - -```makefile -eval-skills: - claude -p "/skill-creator:skill-creator run evals for the spawn-agent skill at ~/.claude/skills/spawn-agent/" - -local-qa: acceptance-test eval-skills -``` - ---- - -## Parte 3 — Actualización de CLAUDE.md - -**Objetivo**: documentar la nueva filosofía de testing y el flujo TDD. - -La sección "Skill evals" en CLAUDE.md (líneas 86-94) ya existe y se mantiene. -La nueva sección "Testing philosophy" se añade **separada** y referencia a -"Skill evals" sin duplicar contenido. - -### Sección a agregar: "Testing philosophy" - -#### Acceptance tests — fuente de verdad - -Los acceptance tests (Gherkin en `tests/acceptance/features/`) definen el -comportamiento contratado de la aplicación. Son ejecutados localmente con -`make acceptance-test`. No se agregan al CI. - -**Regla**: no modificar un acceptance test sin acuerdo explícito. Toda nueva -funcionalidad comienza con un acceptance test. - -#### Flujo TDD (3 leyes) - -Cuando se implementa una nueva feature o se corrige un bug: - -1. **Escribe el acceptance test** en Gherkin que describa el comportamiento esperado -2. **Sigue las 3 leyes de TDD** para los unit tests: - - Ley 1: No escribir código de producción sin tener un unit test que falle - - Ley 2: No escribir más unit test del necesario para que falle (basta con que compile) - - Ley 3: No escribir más código de producción del necesario para que el test pase -3. **Repite** el ciclo rojo → verde → refactor hasta que el acceptance test pase - -Este flujo garantiza cobertura desde el contrato externo (acceptance) hasta la -implementación interna (unit), con tests de mutación como red de seguridad. - ---- - -## Estrategia de implementación - -**Las tres partes se implementan inline en esta conversación, secuencialmente.** -No se lanzan spawn-agents paralelos. - -Razón: durante los acceptance tests (Parte 2) y los evals del skill `spawn-agent` -(Parte 3), el flujo lanzará contenedores Apple Container reales para validar el -comportamiento. Si además existiera un agente paralelo trabajando en -`feat/mutation-ci`, el host quedaría con 3+ contenedores activos simultáneos y -se podría agotar la memoria. Implementación serial → menor riesgo de OOM y -diagnóstico más limpio si algo falla. - -### Orden - -1. Parte 1 (Mutation CI) — bajo riesgo, aislado al Makefile/CI/pyproject. -2. Parte 2 (Acceptance tests) — el grueso del trabajo; verificar localmente con `make acceptance-test`. -3. Parte 3 (CLAUDE.md) — actualización documental, una vez que Parte 2 funciona. -4. Validación final con `make local-qa` (acceptance + eval-skills). - ---- - -## Verificación (post-merge) - -```bash -# Acceptance tests (local) -cd app/cli && make acceptance-test - -# Mutation check (debe pasar: 98.4% > 70%) -make mutation-ci-threshold - -# Todos los tests (unit + acceptance) -make test-all - -# Skill evals -make eval-skills - -# QA completo pre-PR -make local-qa -``` - ---- - -## Decisiones cerradas - -| Decisión | Valor | Razón | -|----------|-------|-------| -| Threshold de mutación | **70%** | Holgura para refactor sin frenar CI | -| Cobertura `eval-skills` | **Solo `spawn-agent`** | Único skill con `evals.json` activo; extender manualmente | -| Ubicación del check de threshold | **Inline en Makefile** | Evita crear `app/cli/scripts/` | -| Estrategia de ejecución | **Serial inline**, sin spawn-agents paralelos | Evitar OOM al lanzar contenedores en Parte 2 + Parte 3 | -| `CliRunner` flag | **Sin `mix_stderr`** | Click 8.3.1 removió el flag; `result.output` ya mezcla | - ---- - -## Funciones existentes a reutilizar - -- `container_cli.utils.run_make` (`utils.py:32`) — entry point a Make. -- `container_cli.utils.find_git_root` (`utils.py:8`) — usado en `_agents_home()`. -- `container_cli.utils.check_token` (`utils.py`) — control real del flujo "sin token". -- `tests/conftest.py:17-28` — fixture `mock_run_make` (modelo a copiar). -- `tests/conftest.py:39-42` — fixture `env_with_token`. -- Targets `mutation-run`/`mutation-show`/`mutation-results` en `app/cli/Makefile`. - ---- - -## Archivos afectados - -| Acción | Archivo | -|--------|---------| -| Crear | `app/cli/tests/acceptance/conftest.py` | -| Crear | `app/cli/tests/acceptance/features/*.feature` (×4) | -| Crear | `app/cli/tests/acceptance/steps/*.py` (×5) | -| Crear | `app/cli/tests/acceptance/__init__.py`, `steps/__init__.py` | -| Modificar | `app/cli/pyproject.toml` | -| Modificar | `app/cli/Makefile` | -| Modificar | `.github/workflows/ci.yml` | -| Modificar | `CLAUDE.md` |