Skip to content

feat: AI supply-chain code review for upstream-sync PRs - #6

Open
SamFleming-TylerTech wants to merge 10 commits into
mainfrom
feat/ai-supply-chain-review
Open

feat: AI supply-chain code review for upstream-sync PRs#6
SamFleming-TylerTech wants to merge 10 commits into
mainfrom
feat/ai-supply-chain-review

Conversation

@SamFleming-TylerTech

Copy link
Copy Markdown
Contributor

What

Adds an advisory, informational AI code review to the security-scan reusable workflow. On an upstream-sync PR it reviews the forked GitHub Action's changes for supply-chain risks and posts inline + summary comments for a human maintainer. It never gates the merge (like the CodeQL check).

Why

Sync PRs pull unverified third-party code. The deterministic checks (analyze-composite-changes.sh, dependency-review, codeql) cover pinning/known-vuln/static analysis; this adds semantic review of what changed code does — the territory those can't reason about.

How it works

  • New composite action .github/actions/ai-review, invoked by a new ai-review job that discovers the sync PR via find-pr.
  • Per the repo's "logic out of YAML" convention, all logic lives in scripts/ai-review/ and the prompt text in scripts/ai-review/prompts/*.md; the action is thin wiring.
  • Review types: supplychain (default), bash, javascript, docker, python, go, powershell, general. Criteria are precision-biased and severity-ranked (CRITICAL/HIGH/MEDIUM/LOW), and explicitly avoid re-flagging what the deterministic checks already gate.
  • Three auth modes: Bedrock OIDC (ai_aws_role_arn), Bedrock API key, or Anthropic API key. Skips cleanly when no auth is configured.
  • Per-type model override (ai_supplychain_model_id) lets the security-critical supplychain review run on a stronger model than the cheap default.
  • Reported as an informational security-scan/ai-review check via report-scan-statuses.sh — excluded from the aggregate gate.

Verification

  • All third-party actions SHA-pinned; YAML + bash -n + python ast clean.
  • Prompt emitter smoke-tested for every type; prompt content proven byte-identical across the markdown-extraction refactor.
  • Composite-action review steps map 1:1 to the emitted review types.

Follow-ups (not in this PR)

  • Deterministic tooling that shouldn't be the AI's job: unicode/bidi grep, dist/ rebuild-and-diff, tag→known-good-SHA manifest, lockfile/typosquat linter.
  • Caller-template secret forwarding (external-action-setup) so consuming forks pass BEDROCK_API_KEY/ANTHROPIC_API_KEY.
  • shellcheck in CI for the new scripts.

Add an advisory, informational AI review of upstream-sync PRs to the
security-scan reusable workflow. It reviews forked GitHub Actions for
supply-chain risks (untrusted-input injection, secret egress,
download-and-run, privilege escalation, obfuscation) and posts inline
plus summary comments for a maintainer.

- New composite action .github/actions/ai-review; logic lives in
  scripts/ai-review with prompts in prompts/*.md (repo convention)
- Types: supplychain (default), bash, javascript, docker, python, go,
  powershell, general; precision-biased, severity-ranked criteria
- Three auth modes (Bedrock OIDC, Bedrock key, Anthropic API); per-type
  model override lets supplychain run on a stronger tier
- Reported as an informational check in report-status (never gates)
Comment on lines +254 to +264
- name: Checkout shared-workflows action
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
with:
repository: tyler-technologies-oss/external-action-shared-workflows
ref: ${{ github.job_workflow_sha }}
sparse-checkout: |
.github/actions/ai-review
scripts/ai-review
path: .shared-workflows

- name: Determine auth presence
Add a deterministic, informational check for dangerous invisible and
bidirectional Unicode (bidi controls, zero-width chars, BOM) in the
added lines of a sync diff -- the Trojan-Source class the AI reviewer
cannot reliably see. Surfaces a security-scan/unicode check plus a
step-summary listing; excluded from the merge-gating aggregate.

- scripts/security-scan/scan-unicode.sh matches a fixed danger set by
  codepoint; takes BASE_SHA/HEAD_SHA or derives them from DEFAULT_BRANCH
- new unicode-scan job in security-scan.yml, reported via
  report-scan-statuses.sh alongside CodeQL and the AI review
Call out committed build-output (dist/) changes in the Security Diff
Summary PR comment. The dist/ bundle is what a JS action actually runs
but is generated and not human-readable, so the source diff hides what
changed -- surface it for the reviewer with a rebuild-and-verify note.
Informational only; does not affect the merge gate.

- generate-diff-analysis.sh emits dist_changes (matches a dist/ segment)
- post-diff-summary.js renders a Built Bundle (dist/) Changes section
The ai-review job statically requested id-token: write for the Bedrock
OIDC (AWS_ROLE_ARN) mode. GitHub rejects a reusable workflow whose nested
job requests a permission the caller did not grant, so every consumer
caller was forced to grant id-token: write or the whole security scan
failed to start -- even when using a bearer key or Anthropic key.

Remove the OIDC path entirely: drop AWS_ROLE_ARN / ai_aws_role_arn, the
configure-aws-credentials step, the bedrock-role branch, and the id-token
permission. Auth is now BEDROCK_API_KEY (bearer) or ANTHROPIC_API_KEY --
neither needs id-token, so no consumer caller needs that permission.
Checkout steps referenced github.job_workflow_sha, which is not a real
context — it always evaluated empty, so actions/checkout fell back to the
repo default branch (main). Consumers pinning the workflow at a tag/branch
still got main's scripts (version skew), and testing a feature branch ran
main's code. Use the documented job.workflow_repository + job.workflow_sha
so the shared scripts/actions are checked out at the exact version of the
running workflow file.
The summary was posted with $ and backticks banned, so the model emitted
bare ${{ ... }} and $(...) fragments. GitHub renders un-backticked $...$
as LaTeX math, mangling the comment into stacked characters. Post via a
single-quoted heredoc (keeps $/backticks shell-safe) and require every
code fragment, identifier, and path to be wrapped in backticks or fenced
code blocks so GitHub never math-renders them.
The prompt led with the "verify L N, else fold into the summary" escape
hatch, so the model routed findings to a single summary line and posted
0 inline comments. Make inline comments the primary, mandatory output:
one per finding, on its annotated L N line, comment on each recurrence,
and only fall back to the summary when a finding genuinely has no L N.
- Bump anthropics/claude-code-action v1.0.89 -> v1.0.176 (the v1.0.90
  Bedrock SigV4 regression, #1193, is fixed upstream; we use the bearer
  token path regardless). Also clears the deprecated Node 20 setup-bun
  warning from the old action.
- Bump actions/github-script v7 -> v9 (Node 24) to clear the Node 20
  deprecation warning on the diff-summary jobs.
- CodeQL: detect JS/TS source and skip init/analyze when there is none,
  instead of failing finalize with "no source code seen" on composite/
  Docker actions.
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.

2 participants