fix(security): audit shared agent skills

[agn-7]

Summary

• include .agents/ in the install-audit expected path set for shared skill writes
• audit both Claude skills and $HOME/.agents/skills for dangerous patterns
• keep the temporary URL scratch file under the script mktemp directory instead of predictable /tmp/sec_skill_urls

Testing

• bash -n scripts/security-install.sh
• make -f Makefile.cbm cbm
• bash scripts/security-install.sh build/c/codebase-memory-mcp

Context

Extracted from #977 after Codex skill support landed in main. This addresses the two remaining scripts/security-install.sh hardening items identified in the maintainer review thread.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Merged as f5bbfa498. Thank you — this is exactly the extraction we hoped for, and a genuinely clean one. Splitting the two scripts/security-install.sh hardening items out of #977 into a single focused commit made it reviewable in one pass, and I appreciate you carrying that overhead rather than leaving the bundle for us to untangle.

I verified both problems on current main rather than taking them on trust, and both are real — one of them worse than you described.

The predictable /tmp/sec_skill_urls path is more than untidy. Because the redirection is set up before grep runs, the target gets truncated on every skill file scanned. So a pre-planted symlink there is an arbitrary-file overwrite running as the auditing user. Worse for a security script: a pre-planted unwritable file makes the redirect fail inside an if condition, which set -e does not catch — silently disabling the URL check while the audit still reports success. Moving the scratch file under the script's own mktemp -d removes both by construction and inherits the existing trap ... EXIT cleanup for free.

It also fixes something you did not claim: the Windows smoke leg deliberately roots TMPDIR under $USERPROFILE, and the scratch file was escaping that choice into MSYS2 /tmp.

The .agents half is equally real. cli.c writes ~/.agents/skills for Zed, OpenHands and Warp, README.md documents it, and the smoke suite already asserts it — yet the audit's expected-path set never learned about it. We were auditing a path set that no longer matched what we install.

Two details I want to credit specifically, because they are the kind of thing that goes unnoticed: you hoisted SKILL_ISSUES=0 out of the loop so it is not reset per directory, and you kept the inner reader as done < <(find ...). Had that been a pipe, FAIL would have been lost in a subshell and the audit would have quietly passed. I checked that specifically, and it propagates correctly.

One thing worth knowing for context: this script only runs in the release dry-run and release smoke, not in PR CI — so none of the green checks above actually exercised your change. It was verified by running the script end-to-end against a built binary, plus shellcheck.

Thanks again for the careful, well-scoped follow-up. #977 stays open for the maintainer's disposition on the original feature discussion, as you asked.