sec-check

name	sec-check
description	On-demand semantic security review of one PR before merge — reads the diff and judges intent, rather than just pattern-matching. Blocks and asks for human review on any suspicion instead of trying to resolve it. Use when asked to security-check, vet, or judge whether a PR is safe to merge. Triggers on "sec-check", "security check this PR", "vet this PR", "is this PR safe to merge".

/sec-check — Pre-merge Security Review

Usage

• /sec-check 123 — PR number
• /sec-check https://github.com/TA-Lib/ta-lib/pull/123 — PR link

<PR> accepts either form; extract the number if a link is given.

Why this exists, and what it is not

.github/workflows/pr-security-gate.yml already runs two free, deterministic checks on every PR automatically: a binary-file check and a Betterleaks secret scan. Those catch "does this diff contain a flagged pattern." They cannot catch "does this code do something malicious that reads perfectly innocent." That second question needs judgment, and judgment costs API tokens — so unlike the free gates, this skill runs on demand, per PR, at your discretion, not automatically on every PR. That split is deliberate: see the CI workflow's own header comment for why an always-on paid reviewer was rejected (unbounded per-PR cost, no natural rate limit on a public repo).

This skill reports and recommends — it does not merge, approve, or request changes on GitHub itself, and it does not approve any pending deployment/environment review for other workflows. If a future refinement wires those actions in, they must stay conditioned on this skill's own verdict, never on "a run happened" — an agent that auto-approves whatever it runs has quietly rebuilt the always-on gate this was built to avoid.

Core rule

Never clear a PR that raises any suspicion. On doubt, stop and hand it to a human — do not try to resolve, explain away, or auto-fix the concern yourself. Silence/uncertainty defaults to BLOCK, not PASS. This is the opposite autonomy stance from a perf-iteration loop: there, keep going and log open questions; here, stopping IS the correct output.

Steps

1. Resolve the target. Parse <PR> to a bare number.

2. Gather context — don't skip this, a diff without context is easy to misjudge:

   gh pr view <PR> --json number,title,body,author,baseRefName,headRefName,isCrossRepository,files,additions,deletions,changedFiles
   gh pr diff <PR>
   gh pr checks <PR>
   

   • isCrossRepository: true → fork PR, unknown/untrusted author → default to more scrutiny, not less.
   • If binary-check or secret-scan in gh pr checks is failing or hasn't completed, stop here — that is already a hard signal, and re-deriving what those gates already check is wasted work, not extra rigor.
   • If running inside tmux ($TMUX set), title the window with the linked issue — a session that opens with a skill invocation never gets Claude Code's own auto-generated title, because that feature only ever looks at the literal first message and gives up for the rest of the session once that message is a bare command. Pull the issue number from a closing keyword in body (Fixes/Closes/Resolves #NNN); fall back to the PR number if none is linked:

     tmux rename-window -t "$TMUX_PANE" "Issue <N>"
     tmux select-pane -t "$TMUX_PANE" -T "Issue <N>"
     

3. Read the diff against these lenses (tailored to this repo — generic "look for eval()" checklists miss what's actually sensitive here):

   • .github/workflows/** — new/changed secret usage, any drift toward pull_request_target, permission escalation, new or re-pinned third-party actions/Docker images (especially anything moving away from a pinned digest/tag).
   • ta_codegen/generator/** — this is the single generator for every backend (C/Rust/Java/C#). A subtle change here poisons all four at once; scrutinize it harder than an equivalent change to one backend's generated output.
   • Build-system changes (CMakeLists.txt, , , , , ) — new dependencies, changed source lists, altered compiler flags. carries specific load-bearing flags (, ) — a change to either is a correctness/perf question for a human, not a rubber stamp either way.

Where the verdict goes

Report the verdict in the conversation only — that's the "agent console" the person driving this session is watching, and that's where a pre-merge check belongs. Never write that a sec-check happened, or its result, into a commit message, PR description/review, or a GitHub issue comment. It isn't part of the durable record of what changed or why — it's a gate someone watched pass before merging, and restating that in text other people read later is noise, not signal. If the same turn also produces a commit or a posted comment (for unrelated reasons), that commit/comment must stand on its own, with no mention that a sec-check ran or what it found.

Refining this skill

First version — expect to tune it against real PRs rather than treat this list as fixed:

• No persistent memory of past verdicts yet (e.g., "this contributor's prior 3 PRs were clean") — every run starts cold.
• The lens list above is a starting set, not exhaustive — add to it when a real PR exposes a category it missed.
• Whether a SUSPICIOUS verdict should also post a gh pr review --request-changes (a real, visible GitHub action, not just a local report) is an open decision — deliberately left out of v1 so this skill stays purely advisory until that tradeoff is made on purpose.