diff --git a/.env.quickstart.example b/.env.quickstart.example index a092448..6fe0fe6 100644 --- a/.env.quickstart.example +++ b/.env.quickstart.example @@ -221,6 +221,12 @@ ROSETTA_PLUGIN_PATH=/opt/rosetta-plugin # Example: playwright,figma MCP_SERVERS_ENABLED=playwright +# ALLOW_AGENT_SDKMANAGER +# What: Local quickstart escape hatch that lets agents add missing Android SDK +# packages through ensure-android-sdk-package only. Raw sdkmanager remains blocked. +# How: Keep true for quickstart; set false for hosted/shared environments. +ALLOW_AGENT_SDKMANAGER=true + # WORKSPACE_EXCLUDE_PATTERNS # What: Extra file paths excluded when syncing code into parallel workspaces. # These are added to the built-in exclusions (node_modules, .git, venvs, diff --git a/agents/IMPLEMENTATION.md b/agents/IMPLEMENTATION.md index 24730f6..ed320f5 100644 --- a/agents/IMPLEMENTATION.md +++ b/agents/IMPLEMENTATION.md @@ -30,6 +30,8 @@ ## Recently Completed (June 2026) +- **Mobile SDK artifact hardening (Jul 8)**: Workspace archives/sync now exclude predictable Android SDK install artifacts (`android-sdk-local`, `cmdline-tools.zip`, generated setup scripts), and seeded workspace `.gitignore` mirrors those patterns. The Bash PreToolUse guard blocks direct `sdkmanager` calls and execution of shell scripts that contain `sdkmanager`, preserving the operator-owned shared SDK contract (`init-mobile-sdk.sh`) instead of letting agents install SDK packages inside workspaces. Follow-up: local quickstart defaults `ALLOW_AGENT_SDKMANAGER=true`, which exposes only the narrow additive `ensure-android-sdk-package` wrapper; hosted/default settings stay false, raw `sdkmanager` stays blocked, and phase prompts now spend two concise lines on the supported Android SDK path. Tests: `uv run pytest test/services/test_agent_hooks.py test/test_tool_usage.py test/test_claude_code_setup.py test/core/test_config_paths.py test/scripts/test_ensure_android_sdk_package_script.py -q` from `backend` (236 passed). + - **TUI workspace notification dedupe (Jul 1)**: Fixed workspace drill-in causing repeated desktop "Workspace phase progressed" notifications every poll during KB init. `MilestoneTracker` now emits workspace progress only when `last_completed_phase` increases, so label-only differences between session-list and `/status` payloads stay silent. `WorkspaceMessagesScreen` feeds its `/status` poll into the shared per-run tracker, and the app-wide active-session watcher excludes the workspace screen's generation to avoid mixed payload sources. Tests: `test_tui_poller.py::TestMilestoneTracker::test_kb_init_phase_label_change_without_progress_is_silent`, `test_tui_app.py::TestWorkspaceDrillIn::test_workspace_screen_excludes_app_level_session_watcher`; broader TUI check: `uv run pytest tests/test_tui_app.py tests/test_tui_poller.py -v` from `mcp_server` (78 passed). - **MCP client setup screen in the TUI (Jun 30)**: New `ClientSetupScreen` (`mcp_server/tui/app.py`) + pure registry `mcp_server/tui/mcp_clients.py` that connect SpecFlow's local MCP server to the user's AI tool, replacing the post-onboarding "No active generation sessions." dead-end (shown first-run via `_startup_gate`, re-openable with `c` on Sessions/Dashboard). **Connect UX**: auto-detects installed clients (`shutil.which` + `~/.cursor`), one-key connect, honest per-client status (green = connected/verified, amber = added-can't-verify, red = failed, grey = not-connected / copy-only). Claude Code → `claude mcp remove`+`add-json -s user` then verify `claude mcp get`; Gemini → `gemini mcp add … -s user` (silent overwrite, trust-folder caveat); Cursor → merge `~/.cursor/mcp.json` (refuse-on-malformed, `.bak` only after confirm) + best-effort deeplink; universal "copy config" fallback. Cursor/Gemini have no read-back so they cap at "added — confirm in your client"; revisiting opens a Yes/No inspection that promotes to connected or demotes to failed. **Real per-client status persisted** in the global **`~/.specflow/config.json`** (`clients` section, values = TUI statuses) so an unverified add is never assumed connected across sessions — global because connecting a client is a machine-wide act (`-s user` / `~/.cursor`) and the TUI must reach it from any project. **`~/.specflow/config.json` is the SSOT for future global SpecFlow/MCP settings** — add new sections as sibling top-level keys via `mcp_clients._read_config`/`_write_config` (they preserve unknown keys); do not put global settings in the project-local `.specflow-local/`. **UI follow-ups (Jul):** modals use the app `Footer` ("ESC to close") instead of an inline hint; the works/doesn't-work inspection is an arrow-navigable `ListView` (not buttons); and on screen open a background probe runs `claude mcp get specflow` for verifiable clients — an already-registered server shows **verified without re-adding** (helps users who already have it), and a stale saved "connected" is cleared (`mcp_clients.forget_status`) if it's gone. One registry feeds both the screen and the `init` CLI hint (replaced the hardcoded `_IDE_REGISTRATION_HINT`). **Hang-safe `local_env.run_command`** (timeout + `proc.kill()` + reap) added because verify/probe commands like `claude mcp get` can block on a network socket with no output — the existing `_stream_subprocess` does an unbounded `await proc.wait()` and would freeze the exclusive Textual worker with no escape. Tests: `test_tui_mcp_clients.py` (pure: json forms, Gemini flag translation, deeplink encoding+no-raw-`+`, merge preserves siblings, status/marker), `test_tui_app.py::TestClientSetupScreen` (render/preselect, connect→verified/added-unverified, malformed-refuse-untouched, inspect confirm/reject/later, status persistence), `test_local_env.py::TestRunCommand` (timeout kills hung process, no zombie). 542 unit tests pass. diff --git a/agents/MEMORY.md b/agents/MEMORY.md index 9b7a8cc..0f9f352 100644 --- a/agents/MEMORY.md +++ b/agents/MEMORY.md @@ -10,3 +10,4 @@ Concise, generalized lessons (not a changelog — that is `agents/IMPLEMENTATION - Firestore emulator imports require the `*overall_export_metadata` file path; export destinations are directories, but `--import-data` must not point at the snapshot directory itself. - Global SpecFlow/TUI settings live in `~/.specflow/config.json` (SSOT) — read/write via `mcp_server/tui/mcp_clients.py` `_read_config`/`_write_config`, which preserve unknown top-level keys; add each new setting as its own top-level section. Do NOT put global settings in the project-local `.specflow-local/` (that dir is per-project runtime: `mcp-config.json`, `workspaces.json`, `init.log`). MCP-client connection status is stored globally because connecting a client is a machine-wide act (`claude/gemini mcp add -s user`, Cursor `~/.cursor/mcp.json`). - Run MCP server pytest commands from `mcp_server/` (`uv run pytest tests/...`); root-level `uv run pytest ...` may not expose a pytest executable because the repo has per-package `pyproject.toml` environments. +- Mobile SDKs are operator-provisioned into the shared cache (`init-mobile-sdk.sh`); if `ANDROID_HOME` is missing, agents must not install SDK packages into the workspace or commit/archive local SDK workarounds. Local quickstart may opt in to additive package installs only via `ensure-android-sdk-package`; raw `sdkmanager` stays blocked. diff --git a/backend/Dockerfile b/backend/Dockerfile index 78166ba..17f31c7 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -146,10 +146,10 @@ RUN CLAUDE_BIN="$(find /app/.venv -type f -path '*/claude_agent_sdk/_bundled/cla RUN mkdir -p /workspaces RUN mkdir -p /agent_logs -# Add helper script to initialize ALL heavy mobile SDKs (Android + Flutter template) into the -# persistent volume on demand. Run manually once per volume — one script to remember. +# Add helper scripts for mobile SDKs on the persistent volume. COPY scripts/init-mobile-sdk.sh /usr/local/bin/init-mobile-sdk.sh -RUN chmod +x /usr/local/bin/init-mobile-sdk.sh +COPY scripts/ensure-android-sdk-package.sh /usr/local/bin/ensure-android-sdk-package +RUN chmod +x /usr/local/bin/init-mobile-sdk.sh /usr/local/bin/ensure-android-sdk-package # Flutter/Dart PATH entrypoints: each lazily copies the shared template into the per-workspace # FLUTTER_ROOT on first invocation, then execs the real binary. Installed in /usr/local/bin so diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 173cb40..04fa2b4 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -97,6 +97,9 @@ class Settings(BaseSettings): # Isolated Workspace Model - Standards Configuration STANDARDS_SOURCE_PATH: str = "/standards_source" # Build-time location of standards in Docker image STANDARDS_DIR_NAME: str = "standards" # Directory name for standards in each workspace (becomes ./standards) + # Local quickstart escape hatch: agents may call ensure-android-sdk-package for + # additive-only installs into the shared Android SDK cache. Hosted/default stays false. + ALLOW_AGENT_SDKMANAGER: bool = False # Artifact Store Configuration # Archived generation outputs are stored at ARTIFACTS_BASE_PATH/{generation_id}/ @@ -131,6 +134,8 @@ class Settings(BaseSettings): "vendor", # Java / Maven "target", ".m2", + # Android SDK install artifacts must live in the shared cache, not workspace snapshots + "android-sdk-local", "cmdline-tools.zip", "setup-sdk.sh", # Build output (language-agnostic) "dist", "build", ".next", ".nuxt", ".output", "out", ".gradle", ".cache", # Standards directory copied during workspace prep — not user code diff --git a/backend/app/core/tool_usage.py b/backend/app/core/tool_usage.py index 5c1ecbf..28a5fec 100644 --- a/backend/app/core/tool_usage.py +++ b/backend/app/core/tool_usage.py @@ -248,7 +248,8 @@ def get_disallowed_tools() -> List[str]: # Java / Kotlin / Android tooling. # Multiple gradlew spellings are listed because each is a distinct literal prefix — # `sh gradlew` and `./gradlew` are different strings to the allowlist matcher. - # adb/avdmanager/emulator are deploy/QA-only (ANDROID_SDK_BASH_USAGE); sdkmanager is operator-only. + # adb/avdmanager/emulator are deploy/QA-only (ANDROID_SDK_BASH_USAGE). Raw sdkmanager is + # operator-only; local quickstart exposes additive installs through the narrow wrapper. "Bash(java:*)", "Bash(javac:*)", "Bash(mvn:*)", @@ -260,6 +261,7 @@ def get_disallowed_tools() -> List[str]: "Bash(bash ./gradlew:*)", "Bash(kotlin:*)", "Bash(kotlinc:*)", + "Bash(ensure-android-sdk-package:*)", # Flutter / Dart tooling "Bash(flutter:*)", "Bash(dart:*)", diff --git a/backend/app/prompts/agents_claude_code.py b/backend/app/prompts/agents_claude_code.py index 93fe7d7..564e33e 100644 --- a/backend/app/prompts/agents_claude_code.py +++ b/backend/app/prompts/agents_claude_code.py @@ -4,7 +4,6 @@ from app.core.mcp_config import mcp_prompt_hints from app.core.tool_usage import BASH_DEFAULT_TIMEOUT_MS, BASH_MAX_TIMEOUT_MS from app.prompts.mcp_workflow_registry import format_mcp_prune_llm_rules_section -from app.prompts.prompt_configs import base_awus, factors_markdown from app.schemas.agent import AgentResult from app.schemas.estimate import ComparativeAnalysis, EstimationSummary from app.schemas.planning import PhaseInfo @@ -1131,6 +1130,8 @@ def generate_phase_agent_template( - Builds: npm run build, vite build, next build, tsc --noEmit, webpack (no --watch) - Tests: npm test -- --run, jest (no --watch), pytest, go test, cargo test — these exit when done - Installs, lints, formatters: npm install, eslint, prettier, ruff, mypy +- Android SDK: raw `sdkmanager` is forbidden. If a local quickstart SDK package is missing, + run `ensure-android-sdk-package ''` only; never install an SDK in the workspace. """ # Prepend phase header diff --git a/backend/app/services/agent_hooks.py b/backend/app/services/agent_hooks.py index 27dee4d..ac8ae2b 100644 --- a/backend/app/services/agent_hooks.py +++ b/backend/app/services/agent_hooks.py @@ -1,5 +1,7 @@ import logging import re +import shlex +from pathlib import Path from typing import Any, List, Pattern, Tuple from claude_agent_sdk import ( @@ -161,6 +163,17 @@ def _gradle_blocklist() -> List[Tuple[str, str]]: ] +def _android_sdk_blocklist() -> List[Tuple[str, str]]: + """Android SDK package-management commands owned by operators, not agents.""" + return [ + ( + r"(?:^|[;&|]\s*)(?:[A-Za-z_][A-Za-z0-9_]*=\S+\s+)*(?:\S*/)?sdkmanager\b", + "`sdkmanager` is operator-only. The shared Android SDK is provisioned once " + "with `init-mobile-sdk.sh`; agents must not install SDK packages into a workspace.", + ), + ] + + def _shell_blocklist() -> List[Tuple[str, str]]: """Generic shell constructs that detach, follow, or loop forever.""" return [ @@ -208,6 +221,7 @@ def _ci_blocklist() -> List[Tuple[str, str]]: + _node_static_blocklist() + _watch_flag_blocklist() + _gradle_blocklist() + + _android_sdk_blocklist() + _shell_blocklist() + _ci_blocklist() ) @@ -239,12 +253,60 @@ def _ci_blocklist() -> List[Tuple[str, str]]: ), ] +_SCRIPT_SDKMANAGER_REASON = ( + "Running a workspace shell script that invokes `sdkmanager` side-steps the command " + "allowlist. The shared Android SDK is operator-provisioned with `init-mobile-sdk.sh`; " + "agents must report a missing SDK as an infrastructure blocker instead of installing one." +) + + +def _script_candidates(command: str) -> list[str]: + """Return shell script tokens that are being executed, not merely read or chmod'd.""" + try: + tokens = shlex.split(command) + except ValueError: + return [] + + candidates: list[str] = [] + for idx, token in enumerate(tokens): + previous = Path(tokens[idx - 1]).name if idx > 0 else "" + starts_command = idx == 0 or previous in {"&&", "||", ";", "|"} + if token.endswith(".sh") and ( + previous in {"bash", "sh"} or token.startswith("./") or starts_command + ): + candidates.append(token) + return candidates + + +def _script_path(token: str, cwd: str | None) -> Path | None: + if not cwd: + return None + path = Path(token) + if not path.is_absolute(): + path = Path(cwd) / path + return path + + +def _script_invokes_sdkmanager(command: str, cwd: str | None) -> bool: + for token in _script_candidates(command): + path = _script_path(token, cwd) + if path is None: + continue + try: + if path.is_file() and re.search(r"\bsdkmanager\b", path.read_text(encoding="utf-8")): + return True + except OSError: + continue + except UnicodeDecodeError: + continue + return False + def _is_bash_tool_input(tool_name: str, tool_input: dict[str, Any]) -> bool: return tool_name == "Bash" and isinstance(tool_input.get("command"), str) -def check_bash_command(command: str) -> Tuple[bool, str | None]: +def check_bash_command(command: str, cwd: str | None = None) -> Tuple[bool, str | None]: """Inspect a Bash command string. Returns (is_blocked, reason_or_none). Side-effect-free — safe to unit-test @@ -253,6 +315,8 @@ def check_bash_command(command: str) -> Tuple[bool, str | None]: for pattern, reason in _COMPILED_BLOCKLIST: if pattern.search(command): return True, reason + if _script_invokes_sdkmanager(command, cwd): + return True, _SCRIPT_SDKMANAGER_REASON return False, None @@ -271,7 +335,8 @@ async def _pre_tool_use_hook( return {} command: str = tool_input["command"] - blocked, reason = check_bash_command(command) + cwd = input_data.get("cwd") if isinstance(input_data.get("cwd"), str) else None + blocked, reason = check_bash_command(command, cwd=cwd) if not blocked: return {} diff --git a/backend/app/services/claude_code.py b/backend/app/services/claude_code.py index 845eccb..4486476 100644 --- a/backend/app/services/claude_code.py +++ b/backend/app/services/claude_code.py @@ -236,6 +236,7 @@ def setup_workspace_cache_directories(workspace_path: str) -> Dict[str, str]: } scalar_flags = { "MAVEN_OPTS": f"-Dmaven.repo.local={os.path.join(cache_base, 'maven', 'repository')}", + "ALLOW_AGENT_SDKMANAGER": "true" if settings.ALLOW_AGENT_SDKMANAGER else "false", } # Disable analytics for every CLI handed to agents: the sandbox has no egress (dead # latency + noisy errors), and analytics state written under $HOME can exhaust the pod rootfs. diff --git a/backend/app/utils/workspace_gitignore.py b/backend/app/utils/workspace_gitignore.py index e16caa6..8acab93 100644 --- a/backend/app/utils/workspace_gitignore.py +++ b/backend/app/utils/workspace_gitignore.py @@ -23,6 +23,9 @@ "vendor/", "target/", ".m2/", + "android-sdk-local/", + "cmdline-tools.zip", + "setup-sdk.sh", "dist/", "build/", ".next/", diff --git a/backend/scripts/ensure-android-sdk-package.sh b/backend/scripts/ensure-android-sdk-package.sh new file mode 100755 index 0000000..461c49e --- /dev/null +++ b/backend/scripts/ensure-android-sdk-package.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env sh +# Add missing Android SDK packages to the shared SDK cache. +# +# This is intentionally narrower than raw sdkmanager: +# - local quickstart must opt in with ALLOW_AGENT_SDKMANAGER=true +# - installs only into the shared ANDROID_SDK_ROOT +# - skips packages that already exist +# - serializes installs with a simple directory lock +set -eu + +if [ "${ALLOW_AGENT_SDKMANAGER:-false}" != "true" ]; then + echo "ensure-android-sdk-package is disabled. Set ALLOW_AGENT_SDKMANAGER=true for local quickstart." >&2 + exit 1 +fi + +: "${ANDROID_SDK_ROOT:?ANDROID_SDK_ROOT must be set}" +WORKSPACE_BASE_PATH="${WORKSPACE_BASE_PATH:-/workspaces}" +EXPECTED_ROOT="${WORKSPACE_BASE_PATH}/caches/common/android" +if [ "${ANDROID_SDK_ROOT}" != "${EXPECTED_ROOT}" ]; then + echo "Refusing to modify Android SDK outside shared cache: ${ANDROID_SDK_ROOT}" >&2 + echo "Expected: ${EXPECTED_ROOT}" >&2 + exit 1 +fi + +SDKMANAGER="${ANDROID_SDK_ROOT}/cmdline-tools/latest/bin/sdkmanager" +if [ ! -x "${SDKMANAGER}" ]; then + echo "sdkmanager not found at ${SDKMANAGER}. Run init-mobile-sdk.sh first." >&2 + exit 1 +fi + +if [ "$#" -eq 0 ]; then + echo "Usage: ensure-android-sdk-package [...]" >&2 + exit 2 +fi + +package_dir() { + case "$1" in + platform-tools) + printf '%s\n' "${ANDROID_SDK_ROOT}/platform-tools" + ;; + platforms\;android-[0-9]*) + printf '%s\n' "${ANDROID_SDK_ROOT}/platforms/${1#platforms;}" + ;; + build-tools\;[0-9]*) + printf '%s\n' "${ANDROID_SDK_ROOT}/build-tools/${1#build-tools;}" + ;; + cmake\;[0-9]*) + printf '%s\n' "${ANDROID_SDK_ROOT}/cmake/${1#cmake;}" + ;; + ndk\;[0-9]*) + printf '%s\n' "${ANDROID_SDK_ROOT}/ndk/${1#ndk;}" + ;; + *) + echo "Package not allowed for agent-managed additive install: $1" >&2 + exit 2 + ;; + esac +} + +missing="" +for package in "$@"; do + dir="$(package_dir "${package}")" + if [ -d "${dir}" ]; then + echo "Android SDK package already present: ${package}" + else + missing="${missing} ${package}" + fi +done + +if [ -z "${missing}" ]; then + exit 0 +fi + +LOCK_DIR="${ANDROID_SDK_ROOT}/.sdkmanager.lock" +attempt=0 +while ! mkdir "${LOCK_DIR}" 2>/dev/null; do + attempt=$((attempt + 1)) + if [ "${attempt}" -gt 120 ]; then + echo "Timed out waiting for Android SDK lock: ${LOCK_DIR}" >&2 + exit 1 + fi + sleep 1 +done +trap 'rmdir "${LOCK_DIR}" 2>/dev/null || true' EXIT INT TERM + +for package in ${missing}; do + dir="$(package_dir "${package}")" + if [ -d "${dir}" ]; then + echo "Android SDK package already present after lock: ${package}" + continue + fi + echo "Installing missing Android SDK package into shared cache: ${package}" + yes | "${SDKMANAGER}" --sdk_root="${ANDROID_SDK_ROOT}" "${package}" >/dev/null +done + +echo "Android SDK additive package check complete." diff --git a/backend/test/core/test_config_paths.py b/backend/test/core/test_config_paths.py index 0e21ffa..e370a26 100644 --- a/backend/test/core/test_config_paths.py +++ b/backend/test/core/test_config_paths.py @@ -62,6 +62,24 @@ def test_workspace_dir_env_sets_workspace_dir(self): assert s.WORKSPACE_DIR == "/my/workdir2" +class TestAgentSdkmanagerPolicy: + def test_hosted_default_disallows_agent_sdkmanager(self): + from app.core.config import Settings + + with pytest.MonkeyPatch.context() as mp: + mp.delenv("ALLOW_AGENT_SDKMANAGER", raising=False) + s = Settings() + assert s.ALLOW_AGENT_SDKMANAGER is False + + def test_env_can_enable_local_quickstart_agent_sdkmanager(self): + from app.core.config import Settings + + with pytest.MonkeyPatch.context() as mp: + mp.setenv("ALLOW_AGENT_SDKMANAGER", "true") + s = Settings() + assert s.ALLOW_AGENT_SDKMANAGER is True + + class TestExcludedArtifactPatternsAlias: def test_legacy_env_name_still_populates_field(self): """Legacy CODE_ARCHIVE_EXCLUDE_PATTERNS env still populates EXCLUDED_ARTIFACT_PATTERNS.""" @@ -82,6 +100,23 @@ def test_default_includes_git(self): s = Settings() assert ".git" in s.EXCLUDED_ARTIFACT_PATTERNS + @pytest.mark.parametrize( + "pattern", + [ + "android-sdk-local", + "cmdline-tools.zip", + "setup-sdk.sh", + ], + ) + def test_default_excludes_workspace_local_android_sdk_artifacts(self, pattern): + from app.core.config import Settings + + with pytest.MonkeyPatch.context() as mp: + mp.delenv("CODE_ARCHIVE_EXCLUDE_PATTERNS", raising=False) + mp.delenv("EXCLUDED_ARTIFACT_PATTERNS", raising=False) + s = Settings() + assert pattern in s.EXCLUDED_ARTIFACT_PATTERNS + class TestWorkspaceExcludePatternsParsing: @pytest.mark.parametrize( diff --git a/backend/test/scripts/test_ensure_android_sdk_package_script.py b/backend/test/scripts/test_ensure_android_sdk_package_script.py new file mode 100644 index 0000000..79b65aa --- /dev/null +++ b/backend/test/scripts/test_ensure_android_sdk_package_script.py @@ -0,0 +1,116 @@ +"""Tests for the additive Android SDK package wrapper.""" + +import os +import subprocess +from pathlib import Path + + +SCRIPT = Path(__file__).resolve().parents[2] / "scripts" / "ensure-android-sdk-package.sh" + + +def _base_env(tmp_path: Path, allow: str = "true") -> dict[str, str]: + sdk_root = tmp_path / "caches" / "common" / "android" + sdkmanager = sdk_root / "cmdline-tools" / "latest" / "bin" / "sdkmanager" + sdkmanager.parent.mkdir(parents=True) + sdkmanager.write_text( + "#!/usr/bin/env sh\n" + "echo \"$@\" >> \"${SDKMANAGER_LOG}\"\n" + "for arg in \"$@\"; do\n" + " case \"$arg\" in\n" + " platforms\\;android-*) mkdir -p \"${ANDROID_SDK_ROOT}/platforms/${arg#platforms;}\" ;;\n" + " build-tools\\;*) mkdir -p \"${ANDROID_SDK_ROOT}/build-tools/${arg#build-tools;}\" ;;\n" + " esac\n" + "done\n", + encoding="utf-8", + ) + sdkmanager.chmod(0o755) + return { + **os.environ, + "ALLOW_AGENT_SDKMANAGER": allow, + "WORKSPACE_BASE_PATH": str(tmp_path), + "ANDROID_SDK_ROOT": str(sdk_root), + "SDKMANAGER_LOG": str(tmp_path / "sdkmanager.log"), + } + + +def test_disabled_by_default_even_when_sdkmanager_exists(tmp_path: Path) -> None: + env = _base_env(tmp_path, allow="false") + + result = subprocess.run( + ["sh", str(SCRIPT), "platforms;android-33"], + env=env, + text=True, + capture_output=True, + check=False, + ) + + assert result.returncode == 1 + assert "disabled" in result.stderr + assert not (tmp_path / "sdkmanager.log").exists() + + +def test_skips_existing_package_without_calling_sdkmanager(tmp_path: Path) -> None: + env = _base_env(tmp_path) + (Path(env["ANDROID_SDK_ROOT"]) / "platforms" / "android-33").mkdir(parents=True) + + result = subprocess.run( + ["sh", str(SCRIPT), "platforms;android-33"], + env=env, + text=True, + capture_output=True, + check=False, + ) + + assert result.returncode == 0 + assert "already present" in result.stdout + assert not (tmp_path / "sdkmanager.log").exists() + + +def test_installs_missing_allowed_package_additively(tmp_path: Path) -> None: + env = _base_env(tmp_path) + + result = subprocess.run( + ["sh", str(SCRIPT), "platforms;android-33", "build-tools;33.0.2"], + env=env, + text=True, + capture_output=True, + check=False, + ) + + assert result.returncode == 0 + log = (tmp_path / "sdkmanager.log").read_text(encoding="utf-8") + assert "--sdk_root=" in log + assert "platforms;android-33" in log + assert "build-tools;33.0.2" in log + + +def test_refuses_disallowed_package_id(tmp_path: Path) -> None: + env = _base_env(tmp_path) + + result = subprocess.run( + ["sh", str(SCRIPT), "cmdline-tools;latest"], + env=env, + text=True, + capture_output=True, + check=False, + ) + + assert result.returncode == 2 + assert "not allowed" in result.stderr + assert not (tmp_path / "sdkmanager.log").exists() + + +def test_refuses_non_shared_sdk_root(tmp_path: Path) -> None: + env = _base_env(tmp_path) + env["ANDROID_SDK_ROOT"] = str(tmp_path / "workspace" / "android-sdk-local") + + result = subprocess.run( + ["sh", str(SCRIPT), "platforms;android-33"], + env=env, + text=True, + capture_output=True, + check=False, + ) + + assert result.returncode == 1 + assert "outside shared cache" in result.stderr diff --git a/backend/test/services/test_agent_hooks.py b/backend/test/services/test_agent_hooks.py index 9e596c2..b9c9fc4 100644 --- a/backend/test/services/test_agent_hooks.py +++ b/backend/test/services/test_agent_hooks.py @@ -55,6 +55,11 @@ ("tail --follow=name file.log", "tail -f"), ("while true; do echo hi; done", "infinite loop"), ("while :; do sleep 1; done", "infinite loop"), + ("sdkmanager --version", "operator-only"), + ( + "JAVA_HOME=/usr/lib/jvm/java-21-openjdk-arm64 /workspace/android/cmdline-tools/bin/sdkmanager --licenses", + "operator-only", + ), # Compound commands — the offending verb anywhere in the chain trips ("cd app && npm start", "npm start"), ("npm install && npm run dev", "npm run dev"), @@ -174,6 +179,9 @@ 'grep -rn "subprocess.run" .', "cat scripts/run.py | grep os.system", "rg child_process src/", + "grep -rn sdkmanager docs/", + "cat setup-sdk.sh | grep sdkmanager", + "ensure-android-sdk-package 'platforms;android-33'", # - running a script file (not inline) is governed by the allowlist, not flagged here "python manage.py migrate", "node server-build.js", @@ -230,6 +238,37 @@ def test_hook_passes_through_allowed_bash_call() -> None: assert output == {} +def test_blocks_shell_script_that_invokes_sdkmanager(tmp_path) -> None: + script = tmp_path / "setup-sdk.sh" + script.write_text("#!/bin/sh\nsdkmanager 'platforms;android-34'\n", encoding="utf-8") + + blocked, reason = check_bash_command("chmod +x setup-sdk.sh && bash setup-sdk.sh", cwd=str(tmp_path)) + + assert blocked + assert reason is not None + assert "side-steps the command allowlist" in reason + + +def test_allows_shell_script_without_sdkmanager(tmp_path) -> None: + script = tmp_path / "run-tests.sh" + script.write_text("#!/bin/sh\n./gradlew testDebugUnitTest\n", encoding="utf-8") + + blocked, reason = check_bash_command("bash run-tests.sh", cwd=str(tmp_path)) + + assert not blocked + assert reason is None + + +def test_allows_reading_shell_script_that_mentions_sdkmanager(tmp_path) -> None: + script = tmp_path / "setup-sdk.sh" + script.write_text("#!/bin/sh\nsdkmanager 'platforms;android-34'\n", encoding="utf-8") + + blocked, reason = check_bash_command(f"cat {script}", cwd=str(tmp_path)) + + assert not blocked + assert reason is None + + def test_hook_ignores_non_bash_tools() -> None: input_data = { "hook_event_name": "PreToolUse", diff --git a/backend/test/test_claude_code_setup.py b/backend/test/test_claude_code_setup.py index df68f2d..21ab4ad 100644 --- a/backend/test/test_claude_code_setup.py +++ b/backend/test/test_claude_code_setup.py @@ -102,6 +102,7 @@ def test_rosetta_plugin_env_reevaluates_disk_state_each_call(tmp_path: Path) -> "ASTRO_TELEMETRY_DISABLED", "STORYBOOK_DISABLE_TELEMETRY", "HOMEBREW_NO_ANALYTICS", + "ALLOW_AGENT_SDKMANAGER", } COMMON_PATH_KEYS = {"ANDROID_SDK_ROOT", "ANDROID_HOME"} @@ -114,6 +115,7 @@ def _call_with_mock_base(workspace_path: str, caches_base: Path): """Call setup_workspace_cache_directories with settings.WORKSPACE_BASE_PATH mocked.""" mock_settings = MagicMock() mock_settings.WORKSPACE_BASE_PATH = str(caches_base) + mock_settings.ALLOW_AGENT_SDKMANAGER = False with patch("app.services.claude_code.settings", mock_settings): return setup_workspace_cache_directories(workspace_path) @@ -182,6 +184,19 @@ def test_telemetry_opt_outs_present_and_not_dirs(self, tmp_path): assert result.get(key) == value, f"{key} should be {value!r}, got {result.get(key)!r}" assert not os.path.exists(key), f"{key} must not be created as a directory" + def test_agent_sdkmanager_disabled_by_default_in_agent_env(self, tmp_path): + result = _call_with_mock_base(str(tmp_path / "ws-01-1"), tmp_path) + assert result["ALLOW_AGENT_SDKMANAGER"] == "false" + + def test_agent_sdkmanager_opt_in_flows_to_agent_env(self, tmp_path): + mock_settings = MagicMock() + mock_settings.WORKSPACE_BASE_PATH = str(tmp_path) + mock_settings.ALLOW_AGENT_SDKMANAGER = True + with patch("app.services.claude_code.settings", mock_settings): + result = setup_workspace_cache_directories(str(tmp_path / "ws-01-1")) + + assert result["ALLOW_AGENT_SDKMANAGER"] == "true" + def test_idempotent_when_dirs_already_exist(self, tmp_path): _call_with_mock_base(str(tmp_path / "ws-01-1"), tmp_path) result = _call_with_mock_base(str(tmp_path / "ws-01-1"), tmp_path) diff --git a/backend/test/test_tool_usage.py b/backend/test/test_tool_usage.py index 3cf0f8e..0aa0dbe 100644 --- a/backend/test/test_tool_usage.py +++ b/backend/test/test_tool_usage.py @@ -153,6 +153,11 @@ def test_sdkmanager_is_operator_only(self): assert "Bash(sdkmanager:*)" not in bash_usage assert "Bash(sdkmanager:*)" not in ANDROID_SDK_BASH_USAGE + def test_android_sdk_additive_wrapper_allowed(self): + """Agents may use only the narrow additive wrapper, gated by env at runtime.""" + assert "Bash(ensure-android-sdk-package:*)" in bash_usage + assert "Bash(sdkmanager:*)" not in bash_usage + _BASH_RULE_RE = re.compile(r"^Bash\((?P.+):\*\)$") @@ -184,6 +189,7 @@ class TestBashAllowlistCoversRealCommands: "dart pub get", "kotlinc Main.kt -include-runtime -d main.jar", "kotlin -version", + "ensure-android-sdk-package 'platforms;android-33'", ] # Sensitive look-alikes that must stay OUTSIDE the generation allowlist: diff --git a/backend/test/utils/test_workspace_gitignore.py b/backend/test/utils/test_workspace_gitignore.py index defc799..2cbd669 100644 --- a/backend/test/utils/test_workspace_gitignore.py +++ b/backend/test/utils/test_workspace_gitignore.py @@ -36,6 +36,9 @@ def test_creates_gitignore_when_missing(tmp_path: Path) -> None: assert "SpecFlow workspace defaults" in content for entry in WORKSPACE_GITIGNORE_ENTRIES: assert entry in content + assert "android-sdk-local/" in content + assert "cmdline-tools.zip" in content + assert "setup-sdk.sh" in content def test_idempotent_when_all_patterns_present(tmp_path: Path) -> None: diff --git a/docker-compose.yml b/docker-compose.yml index 8f348f5..1c32ac5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -125,6 +125,9 @@ services: - LOCAL_USER_EMAIL=${LOCAL_USER_EMAIL} - LOCAL_USER_NAME=${LOCAL_USER_NAME} - POSTHOG_ENABLED=${POSTHOG_ENABLED:-true} + # Local quickstart may let agents add missing Android SDK packages through + # ensure-android-sdk-package. Hosted/default config keeps this disabled. + - ALLOW_AGENT_SDKMANAGER=${ALLOW_AGENT_SDKMANAGER:-true} volumes: # Isolated workspace mounts - each workspace at its own root - ${WORKSPACE_MOUNT_PATH:-./workspaces}:/workspaces:rw