`secrets-manager/review.md`

36 lines

## Snapshot
- Local secrets fan-out tool: captures/validates API tokens and writes them into project, skill, desktop, MCP, and remote `.env` targets.
- Stack: Node.js CLI, Bash watchers, JSON config/registry, local plaintext `.env`; no package manager/datastore.
- How it's run: launchd watchers implied; manual CLI; PM2 only referenced for downstream `site-factory`.
- Status: live; logs/registry updated `2026-04-30`.

## Top Risks (P0 — fix this week)
- `audit-2026-04-30.txt:1`: audit found `307` secret-looking leaks across projects/skills; current manager records the blast radius but does not remediate it. Rotate exposed keys, delete hardcoded copies, then rerun audit until clean.
- `auto-ingest.sh:31` + `auto-ingest.sh:51`: import failures are swallowed with `|| true`, then the changed hash is committed; bad/partial ingests can be silently skipped forever. Only write state after a successful import; log and exit nonzero on failure.
- `auto-ingest.sh:42-44`: every qualifying changed desktop file can background-launch `run-all-stages.sh`; no lock/idempotency guard. Add a lockfile or launchd `KeepAlive`/single-flight guard and record last fired input hash.
- `cli.js:68` + `cli.js:104`: `StrictHostKeyChecking=accept-new` while pushing secrets to remote root hosts risks trusting the wrong first-seen host. Pin host keys in `known_hosts` and fail closed.
- `routes.json:184-202`, `routes.json:301-319`, `routes.json:321-340`: prod API keys are routed directly to `root@45.61.58.125`. Use a restricted deploy user and least-privilege remote path ownership.

## Notable Issues (P1 — fix this month)
- `cli.js:247-259`: batch import tries to catch per-key errors, but `cmdAdd()` calls `process.exit()` on validation failure at `cli.js:225`; first bad token kills the whole batch. Return structured errors from `cmdAdd` instead of exiting inside it.
- `clipboard-watcher.launchd.err:1` + `supervisor.log:23`: launchd lacks permission for the Desktop workflow, so the watcher path is currently broken/bypassed. Move the watched file outside Desktop or install the required Full Disk Access/plist config.
- `clipboard-watcher.log:6`: one launchd run could not find `node`; script later hardcodes `/usr/local/bin/node` at `clipboard-watcher.sh:36`, while `auto-ingest.sh:31` still uses bare `node`. Use one absolute Node path everywhere.
- `cli.js:33`, `cli.js:59`, `cli.js:210`: writes are non-atomic; interruption can corrupt `.env`, registry, or `.claude.json`. Write temp file, `fsync`, then rename.
- `cli.js:64-72`: remote read ignores SSH failure and returns empty text, so a transient read failure can overwrite a remote `.env` with only the current key. Throw on nonzero SSH read before writing.
- `clipboard-watcher.sh:60-70`: low-confidence token classifiers can send arbitrary clipboard strings to third-party verification endpoints. Require explicit key context or user confirmation for prefixless tokens.
- `cli.js:290-329`: audit skips all `.env`/`.env.local` files globally, even outside this manager. That misses committed env files in other projects; skip only approved managed destinations.

## Polish (P2 — when convenient)
- No README or install notes observed; document launchd setup, required Full Disk Access, Node path, and recovery steps.
- No tests observed; add fixtures for env parsing, batch import failure behavior, and audit skip rules.
- `routes.json` mixes local projects, skills, MCP, and remote prod targets in one large object; split destinations by environment/owner to reduce accidental fan-out.

## Strengths
- `cli.js:96-110` writes remote secret bodies over stdin, not command-line args.
- `cli.js:31` and registry storage avoid full secret values, keeping only last4/hash digests.
- `cli.js:43-60` preserves existing `.env` structure while tightening file mode to `0600`.

## Open questions for the owner
- Are any repos in `audit-2026-04-30.txt` pushed to GitHub or synced externally?
- Is `45.61.58.125` production, staging, or a disposable box?
- Should this tool be allowed to trigger `site-factory` automatically, or only stage secrets?