Repository navigation
Run AGT pin check and upstream watch in CI - #259
aaronormerod wants to merge 2 commits into
Conversation
Closes GenAI-Security-Project#74 Signed-off-by: aaronormerod <137795541+aaronormerod@users.noreply.github.com>
|
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
left a comment
There was a problem hiding this comment.
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.
-
.github/workflows/reference-implementation-supply-chain.yml, after line 60.verify-pin.sh:18andrun-upstream-watch.sh:24readagt_repofromagt.lock, and a PR can change that file. I pointedagt_repoandagt_refat a local copy of AGT with a changedegress.rego, made the same change inpolicy/lib/, andverify-pin.shpassed. 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_repoinagt.lockon 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_repocorrect andagt_refset to a commit that never reachedmain, 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 everyagt_refchange. -
Same file, line 48. Rename the job from
agttoagt-supply-chain.reference-implementation.yml:42also has a job namedagt, andgh pr checksprints only that name. On this PR it showsagt 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 259should showagt-supply-chainand noagt. -
Same file, lines 3 and 4. The comment says the script diffs the vendored bundle, but
reference-implementations/agt/test/pin.test.ts:54reads only.regofiles.policy/lib/data.jsonholds the egress allowlist, and it's our own file, with no copy upstream. I added one host to that list, andverify-pin.shandbun testboth 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. -
reference-implementations/agt/packages/conformance/README.md:53says 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 onmain. 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
left a comment
There was a problem hiding this comment.
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)
|
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. |
What changed
Add a workflow that runs
verify-pin.shandrun-upstream-watch.sh. It runs weekly, on manual dispatch, and on changes toagt.lock,policy/, those scripts, or the workflow itself.Which issue does this implement
Closes #74
Base branch
integration, because this touches CIType of change
Notes
verify-pin.shbyte-diffs the vendored.regofiles against the pinned commit. A mismatch fails the job. It skips the other files inpolicy/lib, includingdata.json. A first step checks thatagt.locknamesmicrosoft/agent-governance-toolkit, since a PR can change that file. It does not checkagt_ref. The watch reports drift in the job summary and exits 0. It fails the job only when it could not run, which is whatupstream-watch.tsalready does.verify:zero-diffis 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
uv run pytest -vpasses on my machineuv run mkdocs build --strictpasses on my machineAlso 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.lockcheck passes on the real value and exits 1 on a changedagt_repo.actionlintandzizmor --offlinereport no findings.Checklist
git commit -s(required by the DCO)Security