diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-03-08 21:03:50 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-03-08 21:03:50 +0000 |
| commit | 632ea5a44731af94b6238f330a3b5440906c8ae7 (patch) | |
| tree | d8c780412598d66b89ef390b5729e379fdfd9d5b /docs | |
| parent | 406247b14985ab57902e8e42898dc8cb8960290d (diff) | |
| parent | 93a4c852bf726b00e8014d385165f847763fa214 (diff) | |
merge: pull latest from master and resolve conflicts
- Resolve conflicts in API server, CLI, and executor.
- Maintain Gemini classification and assignment logic.
- Update UI to use generic agent config and project_dir.
- Fix ProjectDir/WorkingDir inconsistencies in Gemini runner.
- All tests passing after merge.
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/adr/003-security-model.md | 135 |
1 files changed, 135 insertions, 0 deletions
diff --git a/docs/adr/003-security-model.md b/docs/adr/003-security-model.md new file mode 100644 index 0000000..529e50e --- /dev/null +++ b/docs/adr/003-security-model.md @@ -0,0 +1,135 @@ +# ADR-003: Security Model + +## Status +Accepted + +## Context + +Claudomator is a local developer tool: it runs on the developer's own machine, +accepts tasks from the operator (a human or automation they control), and executes +`claude` subprocesses with filesystem access. The primary deployment model is +single-user, single-machine — not a shared or internet-facing service. + +This ADR documents the current security posture, the explicit trust boundary +assumptions, and known risks surfaced during code review (2026-03-08). + +## Trust Boundary + +``` +[ Operator (human / script) ] + │ loopback or LAN + ▼ + [ claudomator HTTP API :8484 ] + │ + [ claude subprocess (bypassPermissions) ] + │ + [ local filesystem, tools ] +``` + +**Trusted:** The operator and all callers of the HTTP API. Any caller can create, +run, delete, and cancel tasks; execute server-side scripts; and read all logs. + +**Untrusted:** The content that Claude processes (web pages, code repos, user +instructions containing adversarial prompts). The tool makes no attempt to sandbox +Claude's output before acting on it. + +## Explicit Design Decisions + +### No Authentication or Authorization + +**Decision:** The HTTP API has no auth middleware. All routes are publicly +accessible to any network-reachable client. + +**Rationale:** Claudomator is a local tool. Adding auth would impose key management +overhead on a single-user workflow. The operator is assumed to control who can +reach `:8484`. + +**Risk:** If the server is exposed on a non-loopback interface (the current default), +any host on the LAN — or the public internet if port-forwarded — can: +- Create and run arbitrary tasks +- Trigger `POST /api/scripts/deploy` and `POST /api/scripts/start-next-task` + (which run shell scripts on the server with no additional gate) +- Read all execution logs and task data via WebSocket or REST + +**Mitigation until auth is added:** Run behind a firewall or bind to `127.0.0.1` +only. The `addr` config key controls the listen address. + +### Permissive CORS (`Access-Control-Allow-Origin: *`) + +**Decision:** All responses include `Access-Control-Allow-Origin: *`. + +**Rationale:** Allows the web UI to be opened from any origin (file://, localhost +variants) during development. Consistent with no-auth posture: if there is no +auth, CORS restriction adds no real security. + +**Risk:** Combined with no auth, any web page the operator visits can issue +cross-origin requests to `localhost:8484` if the browser is on the same machine. + +### `bypassPermissions` as Default Permission Mode + +**Decision:** `executor.ClaudeRunner` defaults `permission_mode` to +`bypassPermissions` when the task does not specify one. + +**Rationale:** The typical claudomator workflow is fully automated, running inside +a container or a dedicated workspace. Stopping for tool-use confirmations would +break unattended execution. The operator is assumed to have reviewed the task +instructions before submission. + +**Risk:** Claude subprocesses run without any tool-use confirmation prompt. +A malicious or miscrafted task instruction can cause Claude to execute arbitrary +shell commands, delete files, or make network requests without operator review. + +**Mitigation:** Tasks are created by the operator. Prompt injection in task +instructions or working directories is the primary attack surface (see below). + +### Prompt Injection via `working_dir` in Elaborate + +**Decision:** `POST /api/elaborate` accepts a `working_dir` field from the HTTP +request body and embeds it (via `%q`) into a Claude system prompt. + +**Risk:** A crafted `working_dir` value can inject arbitrary text into the +elaboration prompt. The `%q` quoting prevents trivial injection but does not +eliminate the risk for sophisticated inputs. + +**Mitigation:** `working_dir` should be validated as an absolute filesystem path +before embedding. This is a known gap; see issue tracker. + +## Known Risks (from code review 2026-03-08) + +| ID | Severity | Location | Description | +|----|----------|----------|-------------| +| C1 | High | `server.go:62-92` | No auth on any endpoint | +| C2 | High | `scripts.go:28-88` | Unauthenticated server-side script execution | +| C3 | Medium | `server.go:481` | `Access-Control-Allow-Origin: *` (intentional) | +| C4 | Medium | `elaborate.go:101-103` | Prompt injection via `working_dir` | +| M2 | Medium | `logs.go:79,125` | Path traversal: `exec.StdoutPath` from DB not validated before `os.Open` | +| M3 | Medium | `server.go` | No request body size limit — 1 GB body accepted | +| M6 | Medium | `scripts.go:85` | `stderr` returned to caller may contain internal paths/credentials | +| M7 | Medium | `websocket.go:58` | WebSocket broadcasts task events (cost, errors) to all clients without auth | +| M8 | Medium | `server.go:256-269` | `/api/workspaces` enumerates filesystem layout; raw `os.ReadDir` errors returned | +| L1 | Low | `notify.go:34-39` | Webhook URL not validated — `file://`, internal addresses accepted (SSRF if exposed) | +| L6 | Low | `reporter.go:109-113` | `HTMLReporter` uses `fmt.Fprintf` for HTML — XSS if user-visible fields added | +| X1 | Low | `app.js:1047,1717,2131` | `err.message` injected via `innerHTML` — XSS if server returns HTML in error | + +Risks marked Medium/High are acceptable for the current local-only deployment model +but must be addressed before exposing the service to untrusted networks. + +## Recommended Changes (if exposed to untrusted networks) + +1. Add API-key middleware (token in `Authorization` header or `X-API-Key` header). +2. Bind the listen address to `127.0.0.1` by default; require explicit opt-in for + LAN/public exposure. +3. Validate `StdoutPath` is under the known data directory before `os.Open`. +4. Wrap request bodies with `http.MaxBytesReader` (e.g. 10 MB limit). +5. Sanitize `working_dir` in elaborate: must be absolute, no shell metacharacters. +6. Validate webhook URLs to `http://` or `https://` scheme only. +7. Replace `fmt.Fprintf` HTML generation in `HTMLReporter` with `html/template`. +8. Replace `innerHTML` template literals containing `err.message` with `textContent`. + +## Consequences + +- Current posture: local-only, single-user, no auth. Acceptable for the current use case. +- Future: any expansion to shared/remote access requires auth first. +- The `bypassPermissions` default is a permanent trade-off: unattended automation vs. + per-operation safety prompts. Operator override via task YAML `permission_mode` is + always available. |
