Skip to content

ci: adversarial AI security review gate - #26

Closed
odtgit wants to merge 7 commits into
masterfrom
ci/security-gate
Closed

odtgit wants to merge 7 commits into
masterfrom
ci/security-gate

Conversation

@odtgit

@odtgit odtgit commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

Adds a required security-review check: a deterministic plus two-pass adversarial AI review of every PR, fail closed, before anything reaches master and the auto-release to PyPI, GHCR and Home Assistant users. Also adds SHA pinning across all workflows, dependency review, Dependabot and a ruleset script.

Design

  • .github/workflows/security-review.yml on pull_request_target (opened, synchronize, reopened, ready_for_review). One concurrency group per PR with cancel-in-progress.
    • Job security-review (contents: read, pull-requests: read) runs .github/security-review/review.py.
    • Job security-review-comment (pull-requests: write, no Claude token) creates or updates one PR comment.
  • Deterministic pre-checks (policy.py) run first and hard fail:
    • hidden unicode (bidi, zero-width, BOM, U+2028/2029) in added lines, paths, title or body
    • binary files other than images that pass a magic-byte check
    • size limits, which fail with a "split the PR" message
    • pull_request_target added outside the gate, curl|sh in sensitive files, or any other workflow defining a job named security-review (spoofing)
    • non-inert files in the gate's fixture directory
  • Sensitive paths switch the AI to strict mode: pre-change content is included and the block threshold drops from high to medium. They cover .github/**, Dockerfile, compose, add-on config, pyproject, requirements, custom_components/*/manifest.json, hacs.json, run.sh, scripts/**, *.service, .gitattributes and SECURITY.md.
  • Two passes with claude-opus-5-5 at high effort:
    • Reviewer: file:line, category, severity, evidence.
    • Verifier: a different system prompt. It confirms, refutes or marks uncertain, and hunts for misses.
    • Both system prompts say all PR content is untrusted and that injection attempts are high findings. PR data sits inside nonce-named tags.
  • Verdict (verdict.py) is FAIL on:
    • a pre-check failure
    • an unrefuted finding at or above the threshold (uncertain keeps the higher severity; unassessed and verifier-new findings count)
    • any CLI or API error, timeout, refusal, schema-invalid output or unexpected model
    • a missing secret, a moved head, or a draft PR (deferred until the PR is marked ready)

Trust boundary

  • The reviewer code, prompts and thresholds come from the default branch. The workflow file is from master, and the job does a sparse checkout of .github/security-review at github.sha. A PR cannot weaken the gate that reviews it.
  • PR metadata, patches, post-change contents and pre-change contents of sensitive files are fetched from the REST API as data. PR code is never checked out or executed in a job with secrets. No caches.
  • Auth uses the existing repo secret CLAUDE_CODE_OAUTH_TOKEN through the Claude Code CLI (a subscription OAuth token, not an API key).
  • No agentic tools:
    • The CLI runs as claude -p --tools "" --strict-mcp-config --setting-sources "" --disable-slash-commands --permission-mode dontAsk --max-turns 1 --json-schema ... --output-format stream-json.
    • It gets an empty HOME, and an environment with the OAuth token and no GITHUB_TOKEN.
    • llm.py fails closed unless the init event lists only the StructuredOutput sink and no MCP servers, and no other tool is called. Real runs confirmed the tool list is ['StructuredOutput'] and that an instruction to touch a file did nothing.
  • The CLI is the native linux-x64 binary from the npm registry, pinned by version (2.1.283) and sha256 in install_claude.sh, with no install scripts. The sha256 was cross-checked against npm's sha512 integrity.

Supply chain

  • Every action in ci.yml and release.yml, and in the new workflows, is pinned to a full SHA with # vX.Y.Z. Each SHA was verified with gh api against the tag (and the moving major tag where one exists). pypa/gh-action-pypi-publish release/v1 is pinned to v1.14.2, which is the same commit.
  • dependency-review workflow uses dependency-review-action v5.0.0 with fail-on-severity: high.
  • .github/dependabot.yml: github-actions (grouped) and pip, weekly. Dependabot PRs go through the same gate.
  • scripts/setup_ruleset.sh --deploy-key-id ID (not run):
    • master: requires a PR, squash only and linear history; blocks force pushes and deletion; applies to admins.
    • Required checks, each bound to the GitHub Actions app: security-review, dependency-review, lint, typecheck, gate-tests, test (3.10..3.13), lint-pr-title from ci: automatic releases on merge to master #24's pr-title-lint.yml, and tests-ha/hassfest/hacs when present. All are read from master's workflows at run time; --workflows-from DIR previews unmerged workflows.
    • New release-tags ruleset on refs/tags/v* blocks creation, update and deletion. ci: automatic releases on merge to master #24 does not add one.
    • The only bypass actor on both rulesets is DeployKey. It is not GitHub Actions (15368, verified via the API), because that would let any contents: write workflow push. The rulesets API requires actor_id: null for DeployKey, so the bypass covers every write deploy key. The script therefore refuses to apply unless the given key is the only write deploy key.
    • --dry-run prints all three payloads.
    • owner-review, a second master ruleset:
      • one approval, code owner review, last-push approval, stale approvals dismissed
      • bypass: RepositoryRole 5 (admin) in pull_request mode, so the owner's own PRs merge without self-approval; plus the deploy key, because release: commits touch owned files and would otherwise be blocked
      • master-protection has no admin bypass, so the owner's PRs still need security-review
    • Admin = 5 is not in the REST reference; it is cited by GitHub's rest-api-description issue #4406 and the Pulumi/Terraform docs. After applying, the script reads it back through GraphQL and fails if it isn't admin.
  • .github/CODEOWNERS (@odtgit) covers /.github/, /Dockerfile, /config.yaml, /repository.yaml, /pyproject.toml, /custom_components/*/manifest.json, /hacs.json, /scripts/, /run.sh and /evmqtt.service.
    • Every workflow added here declares its permissions explicitly.

Fixtures and results

Real historical changes from this repo, snapshotted as inert *.fx files with fixture_from_git.py. No synthetic malware was written or tested. Run with the real model via evaluate.py:

Fixture Source Expected Result (final prompts) Time Cost (list)
hist_devinput_rw_mknod 5cc7239, /dev/input made read-write, cgroup c 13:* rmw FAIL FAIL, strict, medium privilege 55s $0.39
hist_event_keys_recorded 0a9e354, key attributes written to HA recorder (fixed in bb44613) FAIL FAIL, high keystroke_logging 24s $0.15
hist_autodiscover_all_enabled 0856a78, auto-discovered keyboards read and enabled by default FAIL FAIL, strict, high keystroke_logging 54s $0.96
benign_docs_readme 199d6f1, README example PASS PASS, no findings 20s $0.27
benign_ble_fix 9b355cf, BLE selection fix PASS PASS, strict, one low note 40s $0.84

Costs are the CLI's list-price estimate; the gate runs on the subscription token. The fixtures ran in parallel, so the times are per fixture for both passes. The results held across the last three runs.

  • In the first run, hist_event_keys_recorded was only rated low (a PASS, so a miss). I tuned the prompts, not the fixtures, to treat keystrokes persisted to the HA recorder or other storage as high. All five now match.
  • Unit tests (91) cover the pre-checks (single hidden characters, a fake ELF, fake and real PNG, spoofed workflow names, pull_request_target, size limits), the verdict logic with mocked LLM output, fail-closed CLI stream parsing through a fake CLI binary, and comment sanitising and upsert (a spoofed marker from another user is ignored).
  • Running the gate on its own diff found real problems, which are now fixed:
    • literal hidden characters in the sanitiser regex and in a test
    • curl|sh named in prose tripping the regex
    • the from_git binary detection being fooled by the text "Binary files" in its own source

This PR itself

The gate does not run on this PR: pull_request_target takes the workflow from master, where it does not exist yet. Ran locally instead with the pinned CLI and the OAuth token:

  • Full diff: FAIL on the size pre-check (diff over 200k chars, mostly fixture snapshots). That is the intended "split the PR" behaviour.
  • Diff without tests/fixtures/: self-review results, in order:
    • The first real run returned FAIL with 5 medium findings on the gate itself: gaps in hidden characters and commit messages, a regex-only spoof check, the fixture exemption in the prompts, a GitHub Actions bypass in the ruleset, and the unreviewed fixtures. There were also lows: duplicate assessments, the empty modelUsage check, missing sensitive paths, and workflow_run. All of them are fixed in later commits, except that the fixtures are excluded locally only because of the size limit.
    • After those fixes, the broadened spoof check fired on the gate-tests job's .github/security-review paths (a false positive). That is fixed too, and the pre-checks now pass on this diff.
    • The last real-model run of the final state could not complete: the subscription returned 429 twice. The gate failed closed, which is correct, but there is no final AI verdict on this PR. Expect strict-mode medium findings on a gate change like this one.

Owner steps

  1. The secret CLAUDE_CODE_OAUTH_TOKEN is already set. Only if Dependabot PRs fail with "secret is missing": add the same value as a Dependabot secret.
  2. Settings, Actions, General, "Fork pull request workflows from outside collaborators": set to Require approval for all outside collaborators. This does not affect pull_request_target: GitHub docs say those workflows "will always run, regardless of approval settings". That is intended here, since the gate code is base-owned. The setting matters for ci.yml/dependency-review.yml, and it stops a fork's own pull_request workflow from spoofing the security-review name before you have read the gate comment.
  3. Create the release deploy key per ci: automatic releases on merge to master #24 (write access, secret RELEASE_DEPLOY_KEY in environment release, restricted to master), and make sure it is the only write deploy key.
  4. Deploy key 164724043 ("evmqtt release bot", write) exists and is the only deploy key; the dry-run check passes.
  5. After merging this and ci: automatic releases on merge to master #24 (and ideally the HACS PR, so tests-ha/hassfest/hacs are picked up): run scripts/setup_ruleset.sh --deploy-key-id <id> --dry-run, review the JSON, then run it without --dry-run.
  6. After applying, open a test PR as the owner and confirm the merge box offers the owner-review bypass but still waits for security-review. The docs confirm rules aggregate and bypass lists are per ruleset, but not in so many words that bypassing one ruleset leaves the others in force.
  7. ci: automatic releases on merge to master #24's RELEASING.md still documents the Integration 15368 bypass and a tag job that may create tags through the API. The tag ruleset blocks anything other than the deploy key, so ci: automatic releases on merge to master #24 must push the tag with the deploy key.

Residual risks

  • Prompt injection can still sway the verdict. The mitigations are the adversarial verifier, the injection rule and the deterministic checks.
  • The model can miss subtle backdoors. Detection on deliberately malicious code is not measured.
  • pull_request_target runs without approval, so anyone can spend subscription usage. This is bounded by the limits, draft deferral and cancellation.
  • Retargeting a PR's base does not re-run the gate; close and reopen it. The branch is not required to be up to date, so merge skew is not reviewed.
  • Anyone holding a write deploy key can push to master and create v* tags.
  • required_approving_review_count: 1 applies to every non-owner PR, not only to owned paths.
  • Push rulesets with file path restrictions would be stricter, but GitHub offers them only for private and internal repositories, so they are not an option here.
  • Subscription rate limits (429) fail the gate. That is fail-closed but blocks merges until the limit resets; re-run the job.
  • The spoof pre-check matches text. A job name built from an expression can evade it.
  • Images are not reviewed by the model. CI pip install of tools is unpinned.

Details in SECURITY.md.

https://claude.ai/code/session_01TJskTZQqRHiHQBc8kE5Lr2

Every third-party action is pinned to a full commit SHA with a version
comment, each checked against its upstream tag. dependency-review fails
PRs that add high-severity vulnerable dependencies. Dependabot updates
actions and pip weekly. ci.yml gains a gate-tests job for the security
review gate added next.
pull_request_target workflow that runs the reviewer from the default
branch only and reads the PR as API data. Deterministic pre-checks
(hidden unicode, binaries, size, workflow spoofing and pull_request_target,
pipe to shell) hard fail. Sensitive paths switch to strict mode. Two passes
through the pinned Claude Code CLI with all tools disabled: reviewer, then
an adversarial verifier. Fail closed on any error. One updated PR comment.

Fixtures are real commits from this repo's history plus benign changes.
scripts/setup_ruleset.sh builds the master ruleset; it is not applied.
…to-shell, numstat binary detection

The gate's own diff tripped its pre-checks: literal hidden characters in
the sanitiser regex and a test, curl|sh named in prose, and the text
'Binary files' in sources.py. Adds --exclude for local runs.
…t check

GitHub Actions is no longer a bypass actor: it would let any workflow with
contents: write push to master. The only bypass on master and on the new
refs/tags/v* ruleset (creation, update, deletion blocked) is DeployKey.
The API takes no id for it, so the script requires --deploy-key-id to be
the repository's only write deploy key. Required checks now include the
PR title lint job when pr-title-lint.yml is on master.
Wider invisible-character set (tag block, word joiners, fillers) and
commit messages checked; workflow_run treated like pull_request_target;
any mention of security-review in another workflow fails; more sensitive
paths; the model check requires exactly the pinned model; a confirmation
beats a refutation among duplicate assessments. The fixture exemption is
dropped from the prompts since the fixtures are real historical code.
…paths

.github/, Dockerfile, add-on config, pyproject.toml, HACS manifests,
scripts/, run.sh and the systemd unit are owned by @odtgit. A second
master ruleset, owner-review, requires one approval, code owner review
and last-push approval. It bypasses for the admin role on pull requests
only and for the release deploy key; master-protection keeps the deploy
key as its only bypass, so the owner's PRs still need security-review.
The script reads back that role 5 resolves to admin after applying.
@odtgit

odtgit commented Sep 28, 2026

Copy link
Copy Markdown
Owner Author

Dropped: rescoped as over-engineered for this repo.

@odtgit odtgit closed this Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant