Skip to content

Run AGT pin check and upstream watch in CI - #259

Open
aaronormerod wants to merge 2 commits into
GenAI-Security-Project:integrationfrom
aaronormerod:ci/supply-chain-checks
Open

aaronormerod wants to merge 2 commits into
GenAI-Security-Project:integrationfrom
aaronormerod:ci/supply-chain-checks

Conversation

@aaronormerod

@aaronormerod aaronormerod commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Add a workflow that runs verify-pin.sh and run-upstream-watch.sh. It runs weekly, on manual dispatch, and on changes to agt.lock, policy/, those scripts, or the workflow itself.

Which issue does this implement

Closes #74

Base branch

  • integration, because this touches CI

Type of change

  • Tooling or CI

Notes

verify-pin.sh byte-diffs the vendored .rego files against the pinned commit. A mismatch fails the job. It skips the other files in policy/lib, including data.json. A first step checks that agt.lock names microsoft/agent-governance-toolkit, since a PR can change that file. It does not check agt_ref. The watch reports drift in the job summary and exits 0. It fails the job only when it could not run, which is what upstream-watch.ts already does. verify:zero-diff is left out because it exits 1 by design in this repo.

Actions and Bun are pinned to the same versions as reference-implementation.yml.

I tested this

  • I synced my branch with the base branch before opening this
  • uv run pytest -v passes on my machine
  • uv run mkdocs build --strict passes on my machine

Also ran both scripts on the merged tree on Bun 1.3.11. Pin test passes 5 of 5. The watch exits 0 and reports 73 moved fields against main. The agt.lock check passes on the real value and exits 1 on a changed agt_repo. actionlint and zizmor --offline report no findings.

Checklist

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

  • This change has no security impact

Closes GenAI-Security-Project#74

Signed-off-by: aaronormerod <137795541+aaronormerod@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

This pull request's description says it closes #74, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging.

@rocklambros rocklambros left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaron, your workflow did run on GitHub when you opened this PR, so the note in your description is out of date. Run 38214137657 passed every step on a real runner on 2026-10-12. I ran the same two scripts on the merged tree on integration 19b1197 and got the same result: 5 of 5 on the pin test and 73 moved fields from the watch. Pytest, mkdocs build --strict, typecheck and bun test on Bun 1.3.11 and 1.3.14 give the same counts as integration.

I need four changes in one push. I ran all four on a scratch copy, and actionlint and zizmor --offline report no findings.

  1. .github/workflows/reference-implementation-supply-chain.yml, after line 60. verify-pin.sh:18 and run-upstream-watch.sh:24 read agt_repo from agt.lock, and a PR can change that file. I pointed agt_repo and agt_ref at a local copy of AGT with a changed egress.rego, made the same change in policy/lib/, and verify-pin.sh passed. The check was only comparing the PR with itself. Add this step right after the checkout:

          # agt.lock comes from the pull request, and both scripts clone whatever agt_repo
          # names. The expected value lives here because /.github/ needs a project lead's
          # review. This checks the repository only. agt_ref still comes from the pull
          # request, so a reviewer has to read every change to it.
          - name: Check that agt.lock names the upstream repository
            run: |
              set -euo pipefail
              expected="https://github.com/microsoft/agent-governance-toolkit"
              actual="$(jq -r '.agt_repo' agt.lock)"
              if [ "$actual" != "$expected" ]; then
                echo "agt.lock names $actual, expected $expected" >&2
                exit 1
              fi

    To test it, change agt_repo in agt.lock on a throwaway branch. The job should fail at this step, before Bun installs. With the real value it passes.

    This step checks the repository only. With agt_repo correct and agt_ref set to a commit that never reached main, the step and the pin check both passed on my local copy. You don't need to fix that here. It's why the comment tells reviewers to read every agt_ref change.

  2. Same file, line 48. Rename the job from agt to agt-supply-chain. reference-implementation.yml:42 also has a job named agt, and gh pr checks prints only that name. On this PR it shows agt pass. That's your job. The Bun tests did not run here, but a reader can't tell. No ruleset or test uses the name, so the rename is safe. After your push, gh pr checks 259 should show agt-supply-chain and no agt.

  3. Same file, lines 3 and 4. The comment says the script diffs the vendored bundle, but reference-implementations/agt/test/pin.test.ts:54 reads only .rego files. policy/lib/data.json holds the egress allowlist, and it's our own file, with no copy upstream. I added one host to that list, and verify-pin.sh and bun test both passed. Issue #74 warns about a reviewer who reads these scripts and thinks everything is covered, so the comment should say what the check skips. Replace those two lines with:

    # verify-pin.sh clones agt.lock's pinned commit and byte-diffs the vendored .rego
    # files against it. A mismatch fails the job. It skips the other files in policy/lib,
    # including data.json, which holds the egress allowlist.
    
  4. reference-implementations/agt/packages/conformance/README.md:53 says the watch "never fails a build". This PR makes that false. The step exited 128 when the repository could not be fetched, and 1 when a watched file was missing on main. Replace the first two sentences of that line with:

    The watch reports drift, and drift never fails a build. A run that could not finish does fail the job.
    

Please leave Closes #74 as it is for now. The issue names three scripts and this PR runs two, so today it's part of issue #74.

Item 8 on issue #32 asks Bar and Ariel about verify:zero-diff. If it stays out of scope, Closes #74 is right once these changes land. If it stays in, change the line to Part of #74.

@bar-capsule @afogel, here is one new fact for item 8, with no new question. verify-zero-diff.sh exits 1 here only because slice/v4 doesn't exist. With a base that exists it can't catch anything, because its patterns start at packages/ and git prints reference-implementations/agt/packages/.

On PR #229, which changes Guardian source, it exited 0 and printed "zero changed lines". When I made the paths relative to the agt tree, it exited 1 and listed 15 files. So a yes means more than a new base.

Once repaired, it fails any PR that touches its frozen list, which includes Guardian source and agt.lock. Today 17 open PRs touch that list. reference-implementations/agt/README.md:546 calls the script a past measurement between two branches of the standalone repository.

A no still leaves gaps that I'm not asking Aaron to fix here. Drift never turns the run red, a red pin check blocks no merge because it isn't a required check, and a failed Monday run emails only the person who last changed the cron line (monitor-roadmap.yml:3).

Aaron, please also comment on issue #74, as I asked on PR #253, so I can assign it to you.

@drk-ram drk-ram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked 70f1c0b against #74. Five pin tests and nineteen watch/diff tests pass with the prerequisites restored.

The live comparison reports 73 moved fields and exits zero, matching the documented reporting-only behavior.

I changed a vendored policy byte sequence; pin verification failed. Restoring it restored five passes.

Removing pipefail hides a simulated watch exit 7; my local workflow probe catches that regression and checks summary output.

The workflow pins actions, disables persisted credentials, and limits the job token to read access.

I exercised the workflow's shell pipeline locally; I did not launch a hosted workflow run.

Add a step that fails when agt.lock names a repository other than
microsoft/agent-governance-toolkit. Rename the job to agt-supply-chain.
State what verify-pin.sh skips. Correct the conformance README: the
watch fails the job when it could not run.

Signed-off-by: aaronormerod <137795541+aaronormerod@users.noreply.github.com>
(cherry picked from commit f15768bbcdfbd893869bdf9af6496e708e2e0026)
@aaronormerod

Copy link
Copy Markdown
Collaborator Author

Done, one push. agt_repo check added (passes on real agt.lock, exits 1 on a changed repo), job renamed to agt-supply-chain, header comment and conformance README corrected. actionlint and zizmor --offline clean.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

[Proposal] Wire the supply-chain pin and upstream-drift checks into CI

3 participants