feat: deterministic enforcement hooks for migration quality#33
Open
AlexDeMichieli wants to merge 7 commits into
Open
feat: deterministic enforcement hooks for migration quality#33AlexDeMichieli wants to merge 7 commits into
AlexDeMichieli wants to merge 7 commits into
Conversation
AlexDeMichieli
force-pushed
the
feat/plugin-hooks
branch
4 times, most recently
from
c99e748 to
e18c746
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a plugin/hooks.json hook pack for the Copilot CLI plugin to deterministically enforce migration-quality constraints (block/deny unsafe tool calls, inject workflow quality feedback, and produce an audit scorecard), and documents the new enforcement model in plugin/README.md.
Changes:
- Adds deterministic enforcement hooks (
preToolUse,postToolUse,agentStop,sessionEnd) inplugin/hooks.json. - Implements workflow “quality” detection (unpinned actions, placeholders,
write-all, missing permissions, actionlint) and a blocking quality gate with a 3-attempt safety valve. - Documents hook behavior, rationale, and how to enable/disable hooks in
plugin/README.md.
Show a summary per file
| File | Description |
|---|---|
| plugin/hooks.json | Adds 5 hooks to block hardcoded secrets and unsafe deletions, lint/check workflows, enforce a blocking quality gate, and append a migration scorecard. |
| plugin/README.md | Documents the hooks, their lifecycle events, and how to verify/disable them. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 7
| "description": "Block hardcoded secrets in file writes", | ||
| "matcher": "create|edit", | ||
| "timeoutSec": 10, | ||
| "bash": "INPUT=$(cat); ARGS=$(echo \"$INPUT\" | jq -c '.toolArgs' 2>/dev/null); HAS_SECRET=$(echo \"$ARGS\" | grep -ciE '(password|secret|token|api[_-]?key)\\s*[:=]' 2>/dev/null || true); HAS_EXPR=$(echo \"$ARGS\" | grep -cF '${' 2>/dev/null || true); if [ \"${HAS_SECRET:-0}\" -gt 0 ] && [ \"${HAS_EXPR:-0}\" -eq 0 ]; then echo '{\"permissionDecision\":\"deny\",\"permissionDecisionReason\":\"Blocked: hardcoded secret detected. Use GitHub Secrets (${{ secrets.NAME }}) instead.\"}'; else echo '{}'; fi" |
Collaborator
Author
There was a problem hiding this comment.
Fixed. Secret detection now: (1) extracts only the content/new_string field from toolArgs, (2) checks each line individually — a line must match a secret keyword + [:=] + 8+ chars of value AND not contain \${ on that same line, (3) secrets: inherit no longer triggers because inherit is <8 chars and there's no [:=] value pattern.
| "description": "Guard against file deletion outside ci-archive", | ||
| "matcher": "bash", | ||
| "timeoutSec": 10, | ||
| "bash": "INPUT=$(cat); CMD=$(echo \"$INPUT\" | jq -r '.toolArgs.command // .toolArgs // empty' 2>/dev/null); if echo \"$CMD\" | grep -qE 'rm\\s+(-[rfi]+\\s+)*' 2>/dev/null; then if ! echo \"$CMD\" | grep -q '.github/ci-archive' 2>/dev/null; then echo '{\"permissionDecision\":\"deny\",\"permissionDecisionReason\":\"Blocked: file deletion only allowed inside .github/ci-archive/. Move source CI files there before removing.\"}'; exit 0; fi; fi; echo '{}'" |
Collaborator
Author
There was a problem hiding this comment.
Fixed. The rm guard now: (1) checks for ... path traversal BEFORE checking ci-archive paths — any target containing .. is denied immediately, (2) uses a case statement with glob patterns (*/.github/ci-archive/* and *.github/ci-archive/*) for literal matching instead of regex, so xgithub/ci-archive no longer slips through.
| "description": "Quality check and actionlint on workflow files after write", | ||
| "matcher": "create|edit", | ||
| "timeoutSec": 60, | ||
| "bash": "INPUT=$(cat); ARGS=$(echo \"$INPUT\" | jq -c '.toolArgs' 2>/dev/null); FILE=$(echo \"$ARGS\" | jq -r '.file_path // .path // empty' 2>/dev/null); echo \"$FILE\" | grep -q '.github/workflows/' || exit 0; [ -f \"$FILE\" ] || exit 0; if ! command -v actionlint >/dev/null 2>&1; then if [ \"$(uname)\" = 'Linux' ]; then V='1.7.11'; curl -fsSL \"https://github.com/rhysd/actionlint/releases/download/v${V}/actionlint_${V}_linux_amd64.tar.gz\" | tar xz -C /tmp actionlint 2>/dev/null && install -m 755 /tmp/actionlint /usr/local/bin/actionlint 2>/dev/null; elif command -v brew >/dev/null 2>&1; then brew install actionlint 2>/dev/null; fi; fi; W=''; grep -qE 'uses:\\s+[^@]+@v[0-9]' \"$FILE\" 2>/dev/null && W=\"${W}- Unpinned actions: use full SHA commit refs\\n\"; grep -qiE '(TODO|FIXME|CHANGEME|PLACEHOLDER|XXX)' \"$FILE\" 2>/dev/null && W=\"${W}- Placeholder text found: replace before merging\\n\"; grep -qE 'permissions:\\s*write-all' \"$FILE\" 2>/dev/null && W=\"${W}- Over-broad permissions: replace write-all with least-privilege\\n\"; grep -qE '^permissions:' \"$FILE\" 2>/dev/null || W=\"${W}- Missing top-level permissions block\\n\"; L=''; command -v actionlint >/dev/null 2>&1 && L=$(actionlint \"$FILE\" 2>&1 | head -5); if [ -n \"$W\" ] || [ -n \"$L\" ]; then MSG=\"MIGRATION QUALITY CHECK ($FILE):\\n${W}\"; [ -n \"$L\" ] && MSG=\"${MSG}actionlint errors:\\n${L}\\n\"; MSG=\"${MSG}Fix these issues now.\"; ESCAPED=$(printf '%s' \"$MSG\" | sed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/g'); printf '{\"additionalContext\":\"%s\"}' \"$ESCAPED\"; fi" |
Collaborator
Author
There was a problem hiding this comment.
Fixed. All 3 hooks now download to a temp file and verify SHA256 (900919a8...) before extracting. Checksum mismatch aborts install and surfaces an explicit warning. Also fixed the PR description — removed the 'checksum-verified upstream' claim and replaced with accurate description.
| "type": "command", | ||
| "description": "Migration quality gate — block completion if workflows have issues", | ||
| "timeoutSec": 60, | ||
| "bash": "INPUT=$(cat); CWD=$(echo \"$INPUT\" | jq -r '.cwd // \".\"' 2>/dev/null); COUNTER_FILE='/tmp/.migration-quality-gate'; COUNT=$(cat \"$COUNTER_FILE\" 2>/dev/null || echo 0); COUNT=$((COUNT + 1)); echo \"$COUNT\" > \"$COUNTER_FILE\"; if [ \"$COUNT\" -gt 3 ]; then rm -f \"$COUNTER_FILE\"; echo '{}'; exit 0; fi; if ! command -v actionlint >/dev/null 2>&1; then if [ \"$(uname)\" = 'Linux' ]; then V='1.7.11'; curl -fsSL \"https://github.com/rhysd/actionlint/releases/download/v${V}/actionlint_${V}_linux_amd64.tar.gz\" | tar xz -C /tmp actionlint 2>/dev/null && install -m 755 /tmp/actionlint /usr/local/bin/actionlint 2>/dev/null; elif command -v brew >/dev/null 2>&1; then brew install actionlint 2>/dev/null; fi; fi; ISSUES=''; for f in \"$CWD\"/.github/workflows/*.yml \"$CWD\"/.github/workflows/*.yaml; do [ -f \"$f\" ] || continue; FN=$(basename \"$f\"); W=''; grep -qE 'uses:\\s+[^@]+@v[0-9]' \"$f\" 2>/dev/null && W=\"${W}unpinned-actions \"; grep -qiE '(TODO|FIXME|CHANGEME|PLACEHOLDER|XXX)' \"$f\" 2>/dev/null && W=\"${W}placeholders \"; grep -qE 'permissions:\\s*write-all' \"$f\" 2>/dev/null && W=\"${W}write-all \"; command -v actionlint >/dev/null 2>&1 && ! actionlint \"$f\" >/dev/null 2>&1 && W=\"${W}actionlint-errors \"; [ -n \"$W\" ] && ISSUES=\"${ISSUES}${FN}: ${W}; \"; done; if [ -n \"$ISSUES\" ]; then ESCAPED=$(printf '%s' \"$ISSUES\" | sed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/g'); printf '{\"decision\":\"block\",\"reason\":\"Migration quality gate FAILED (attempt %d/3). Fix these workflow issues:\\n%s\"}' \"$COUNT\" \"$ESCAPED\"; else rm -f \"$COUNTER_FILE\"; echo '{}'; fi" |
Collaborator
Author
There was a problem hiding this comment.
Fixed. agentStop now includes grep -qE '^permissions:' || W='no-permissions ' in its per-file scan, consistent with postToolUse.
| "type": "command", | ||
| "description": "Generate final migration scorecard", | ||
| "timeoutSec": 45, | ||
| "bash": "INPUT=$(cat); CWD=$(echo \"$INPUT\" | jq -r '.cwd // \".\"' 2>/dev/null); REASON=$(echo \"$INPUT\" | jq -r '.reason // \"unknown\"' 2>/dev/null); SESSION=$(echo \"$INPUT\" | jq -r '.sessionId // \"unknown\"' 2>/dev/null); if ! command -v actionlint >/dev/null 2>&1; then if [ \"$(uname)\" = 'Linux' ]; then V='1.7.11'; curl -fsSL \"https://github.com/rhysd/actionlint/releases/download/v${V}/actionlint_${V}_linux_amd64.tar.gz\" | tar xz -C /tmp actionlint 2>/dev/null && install -m 755 /tmp/actionlint /usr/local/bin/actionlint 2>/dev/null; elif command -v brew >/dev/null 2>&1; then brew install actionlint 2>/dev/null; fi; fi; TOTAL=0; CLEAN=0; BAD=0; for f in \"$CWD\"/.github/workflows/*.yml \"$CWD\"/.github/workflows/*.yaml; do [ -f \"$f\" ] || continue; TOTAL=$((TOTAL + 1)); HAS=0; grep -qE 'uses:\\s+[^@]+@v[0-9]' \"$f\" 2>/dev/null && HAS=1; grep -qiE '(TODO|FIXME|CHANGEME|PLACEHOLDER|XXX)' \"$f\" 2>/dev/null && HAS=1; grep -qE 'permissions:\\s*write-all' \"$f\" 2>/dev/null && HAS=1; command -v actionlint >/dev/null 2>&1 && ! actionlint \"$f\" >/dev/null 2>&1 && HAS=1; [ \"$HAS\" -eq 0 ] && CLEAN=$((CLEAN + 1)) || BAD=$((BAD + 1)); done; rm -f /tmp/.migration-quality-gate; SC=\"$CWD/.github/MIGRATION-SCORECARD.md\"; [ -f \"$SC\" ] || printf '# Migration Scorecard\\n' > \"$SC\" 2>/dev/null; printf '\\n## %s\\n- Session: %s\\n- Reason: %s\\n- Workflows: %d total, %d clean, %d with issues\\n' \"$(date -u +%Y-%m-%dT%H:%M:%SZ)\" \"$SESSION\" \"$REASON\" \"$TOTAL\" \"$CLEAN\" \"$BAD\" >> \"$SC\" 2>/dev/null; echo '{}'" |
Collaborator
Author
There was a problem hiding this comment.
Fixed. sessionEnd scorecard now includes grep -qE '^permissions:' || HAS=1 so missing-permissions counts as an issue. Consistent with all other hooks.
| | File deletion guard | `preToolUse` | `bash` | Hard-denies `rm` operations outside `.github/ci-archive/`. Prevents accidental deletion of application source code. | | ||
| | Quality check + actionlint | `postToolUse` | `create\|edit` | After any workflow file write, injects `additionalContext` with: unpinned actions (tag vs SHA), placeholder text (TODO/FIXME), over-broad permissions (`write-all`), missing permissions block, and actionlint errors. The agent sees these on the same turn. | | ||
| | **Quality gate** | `agentStop` | — | Scans ALL workflow files when the agent finishes a turn. If any have issues, returns `decision: "block"` forcing the agent to take another turn to fix them. Safety valve releases after 3 attempts to prevent infinite loops. | | ||
| | **Migration scorecard** | `sessionEnd` | — | Generates `.github/MIGRATION-SCORECARD.md` with session ID, timestamp, completion reason, and workflow counts (total / clean / with-issues). Audit artifact for migration quality tracking. | |
Collaborator
Author
There was a problem hiding this comment.
Fixed. Description changed to 'Appends an entry to' and hook description changed to 'Append migration scorecard entry'.
| "description": "Quality check and actionlint on workflow files after write", | ||
| "matcher": "create|edit", | ||
| "timeoutSec": 60, | ||
| "bash": "INPUT=$(cat); ARGS=$(echo \"$INPUT\" | jq -c '.toolArgs' 2>/dev/null); FILE=$(echo \"$ARGS\" | jq -r '.file_path // .path // empty' 2>/dev/null); echo \"$FILE\" | grep -q '.github/workflows/' || exit 0; [ -f \"$FILE\" ] || exit 0; if ! command -v actionlint >/dev/null 2>&1; then if [ \"$(uname)\" = 'Linux' ]; then V='1.7.11'; curl -fsSL \"https://github.com/rhysd/actionlint/releases/download/v${V}/actionlint_${V}_linux_amd64.tar.gz\" | tar xz -C /tmp actionlint 2>/dev/null && install -m 755 /tmp/actionlint /usr/local/bin/actionlint 2>/dev/null; elif command -v brew >/dev/null 2>&1; then brew install actionlint 2>/dev/null; fi; fi; W=''; grep -qE 'uses:\\s+[^@]+@v[0-9]' \"$FILE\" 2>/dev/null && W=\"${W}- Unpinned actions: use full SHA commit refs\\n\"; grep -qiE '(TODO|FIXME|CHANGEME|PLACEHOLDER|XXX)' \"$FILE\" 2>/dev/null && W=\"${W}- Placeholder text found: replace before merging\\n\"; grep -qE 'permissions:\\s*write-all' \"$FILE\" 2>/dev/null && W=\"${W}- Over-broad permissions: replace write-all with least-privilege\\n\"; grep -qE '^permissions:' \"$FILE\" 2>/dev/null || W=\"${W}- Missing top-level permissions block\\n\"; L=''; command -v actionlint >/dev/null 2>&1 && L=$(actionlint \"$FILE\" 2>&1 | head -5); if [ -n \"$W\" ] || [ -n \"$L\" ]; then MSG=\"MIGRATION QUALITY CHECK ($FILE):\\n${W}\"; [ -n \"$L\" ] && MSG=\"${MSG}actionlint errors:\\n${L}\\n\"; MSG=\"${MSG}Fix these issues now.\"; ESCAPED=$(printf '%s' \"$MSG\" | sed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/g'); printf '{\"additionalContext\":\"%s\"}' \"$ESCAPED\"; fi" |
Collaborator
Author
There was a problem hiding this comment.
Fixed. All 3 hooks now: (1) on checksum mismatch, abort install and inject explicit warning via additionalContext, (2) after install attempt, re-check command -v actionlint — if still missing, inject 'actionlint not available (install failed)' warning, (3) agentStop adds 'actionlint-unavailable' to the issues list so it blocks completion.
AlexDeMichieli
force-pushed
the
feat/plugin-hooks
branch
from
e18c746 to
e142373
Compare
Collaborator
|
@AlexDeMichieli, please set up a meeting with @ssulei7 and myself for a quick demo. This is good stuff and we're excited to rubber duck a couple things with you. |
AlexDeMichieli
force-pushed
the
feat/plugin-hooks
branch
from
e142373 to
8c7ac84
Compare
AlexDeMichieli
force-pushed
the
feat/plugin-hooks
branch
from
8c7ac84 to
dc16122
Compare
… actionlint Rebuild hooks.json with correct Copilot hooks API: - preToolUse (matcher: create|edit): secret detection with permissionDecision deny - preToolUse (matcher: bash): rm guard blocks deletion outside ci-archive - postToolUse (matcher: create|edit): quality check + actionlint per workflow file - agentStop: quality gate blocks agent completion until workflows pass (3-attempt safety valve) - sessionEnd: generates MIGRATION-SCORECARD.md with session stats Key changes from previous version: - Use permissionDecision/permissionDecisionReason (not decision/reason) - Add matcher filtering (no more shell-level tool name checks) - agentStop replaces postToolUse-only approach — actually blocks completion - sessionEnd provides audit artifact for migration quality tracking - actionlint runs in 3 hooks: postToolUse, agentStop, sessionEnd - Test harness with 21 passing tests included
AlexDeMichieli
force-pushed
the
feat/plugin-hooks
branch
from
dc16122 to
3ffcb3d
Compare
Convert declarative Jenkinsfile with Build, Test, and Deploy stages to a GitHub Actions CI workflow with pinned action SHAs and least-privilege permissions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
🚀 Jenkins to GitHub Actions Migration Report📊 Migration Overview
🔄 Conversion Diagramgraph LR
A[Jenkins Pipeline] --> B[GitHub Actions Workflow]
subgraph "Jenkins Structure"
D1[Stage: Build]
D2[Stage: Test]
D3[Stage: Deploy]
end
subgraph "GitHub Actions Structure"
G1[Job: build]
G2[Job: test]
G3[Job: deploy]
end
D1 --> G1
D2 --> G2
D3 --> G3
🔧 Key TransformationsStage and Step Conversions
Trigger Mapping
✅ Validation ResultsLinting ResultsManual Verification Checklist
🔐 Security Improvements
🔗 Variable and Secret RequirementsRequired GitHub SecretsNone required for this pipeline. Required GitHub VariablesNone required for this pipeline. 🎯 Next Steps
📁 Original Jenkins FilesThe original Jenkins pipeline file has been archived:
📚 Migration Notes
Migration completed by GitHub Copilot Jenkins Migration Agent |
…nter
Plugin manifest:
- plugin/plugin.json: declare hooks field, bump to 1.4.0
- .github/plugin/marketplace.json: sync to 1.4.0
Hook fixes:
- preToolUse rm guard: extend to rm, mv, unlink, find -delete, git rm, git mv;
strip shell redirect operators (2>&1, >/tmp/x, etc.) before tokenizing targets
so legitimate commands with redirects are not denied (regression).
Allowlist: targets inside .github/ci-archive/ OR CI source files at repo root
(Jenkinsfile, .travis.yml, .gitlab-ci.yml, .drone.yml, bitbucket-pipelines.yml,
azure-pipelines.yml, bamboo-specs/*, .circleci/*).
- agentStop quality-gate: per-session counter (${sessionId}) instead of a
global /tmp file (parallel sessions no longer collide).
- agentStop & sessionEnd: fall back to $GITHUB_WORKSPACE then $PWD when input
cwd is missing/'/root' (CCA sandbox).
- sessionEnd: prune stale per-session counters > 60 min as garbage collection.
Consumer adoption:
- consumer-template/.github/copilot/settings.json with enabledPlugins entry
- consumer-template/README.md explaining surfaces (CLI, CCA, VS Code Agent
Plugins preview).
Closes Sully + Anthony feedback (2026-06-11): toolArgs parsing, scorecard
per-file detail, custom-agent hook firing, plus newly found:
- destructive-op bypass via git mv / mv / find -delete
- false-positive denies on commands with shell redirects
- shared /tmp counter across parallel sessions
- agent narrating fake delete after hook denial
Tested:
- 13 unit tests against the rm guard (all pass)
- End-to-end CLI run with --agent actions-migrator:jenkins-migrator
on alexdemichieli-migrations/jenkins-migration-test:
* README protected (hook denied bash rm)
* Jenkinsfile migrated to clean .github/workflows/ci.yml (10 SHA pins, perms)
* Original archived via git mv into .github/ci-archive/
* Scorecard shows: 1 total, 1 clean, 0 with issues
…ract tests
Problem
Hooks only understood the Copilot CLI / Cloud agent payload schema. In VS
Code Agent Plugins (preview) the same hooks ran but read empty fields, so
enforcement silently no-opped (audit logged tool:"null"; the README 'block'
users saw was VS Code's own terminal safety, not our hook).
Root cause (captured from live payloads on both surfaces)
CLI: { toolName, toolArgs:<json string>, sessionId, toolResult }
VS Code: { tool_name, tool_input:<object>, session_id, tool_response }
- field names differ (camelCase vs snake_case)
- args differ: CLI sends a JSON *string*; VS Code sends an *object*
- tool names differ: bash/create/edit vs run_in_terminal/create_file/...
- deny output differs: top-level permissionDecision vs hookSpecificOutput
- lifecycle events differ: CLI agentStop+sessionEnd vs VS Code Stop
- VS Code ignores matchers (every preToolUse hook runs on every tool)
Fix (plugin/hooks.json) — one file, self-adapting on all surfaces
- Normalize input: read .toolName // .tool_name; parse args whether
.toolArgs is a string (fromjson) or .tool_input is an object.
- Normalize fields: command, filePath//path//file_path, content//new_string.
- Emit BOTH output shapes (top-level + hookSpecificOutput) so each surface
finds the field it expects.
- Register lifecycle under both names: sessionEnd (CLI) and Stop (VS Code)
run the same scorecard script; Stop honors stop_hook_active to avoid loops.
- Each preToolUse hook self-guards (no-op when its field is absent) since
VS Code ignores matchers.
Tests (NEW — guards against silent breakage)
- plugin/hooks.test.sh: 22 contract tests exercising every hook against BOTH
CLI and VS Code payloads (deny/allow/context/scorecard/loop-guard).
- .github/workflows/hooks-test.yml: runs the suite on every PR touching the
hooks; validates hooks.json parses; fails the PR on regression.
- Negative-tested: sabotaging the VS Code arg parsing makes the suite fail
exactly the 4 VS Code destructive-guard cases and exit non-zero.
Validation performed
- 22/22 contract tests pass (CLI + VS Code schemas).
- VS Code live capture: PreToolUse/PostToolUse/UserPromptSubmit/Stop all fire
with correct payloads (create_file + run_in_terminal).
- CLI end-to-end (--agent actions-migrator:jenkins-migrator): README delete
blocked; Jenkinsfile migrated to clean ci.yml (12 SHA pins + permissions);
archived via git mv; scorecard = 1 clean, 0 issues.
Commit 5aa2045 accidentally committed a test Jenkins migration into the plugin repository. These are not part of the plugin and should not ship: - .github/ci-archive/Jenkinsfile - .github/ci-archive/MIGRATION-README.md - .github/workflows/ci.yml (migrated-from-Jenkins test output) The migration was a test of the hooks, not a deliverable. Removing keeps the PR scoped to the hooks, tests, and consumer template.
Adds a 'Testing the hooks' subsection covering: - how to run the suite (bash plugin/hooks.test.sh) - what the 22 contract tests check across CLI + VS Code schemas - requirements (bash + jq only; no actionlint/network/curl/brew needed) - self-contained guarantees (no working-tree writes, cleans temp files) - CI wiring (.github/workflows/hooks-test.yml runs on hook changes) Also updates the hooks table to reflect cross-surface support (CLI sessionEnd + VS Code Stop) so contributors know the same hooks.json serves all surfaces.
Per Tim Rogers' guidance (#cloud-agent-team): permissionDecisionReason is per-tool-call feedback; additionalContext is the field that carries session-level rules the model uses for subsequent tool choices. Today, when the destructive guard denies bash 'rm README', the agent on CCA silently retries via MCP delete_file (matcher gap) or via apply_patch because the deny is interpreted as 'this tool failed, try another'. Adding additionalContext with explicit 'do not attempt this operation through any alternative tool, including MCP / apply_patch / etc.' wording lets the autonomous agent see a session-level policy, not just a per-tool veto. Verified end-to-end in AlexDeMichieli/cca-hook-repro-consumer #10: agent tried bash, then MCP delete, hit deny on each, gave up with a 0-file PR explaining the policy blocked every path. Follow-up still needed: widen matchers to include MCP write tool names and handle their input shape so the hook fires on first-shot MCP attempts rather than relying on additionalContext from a prior bash deny persisting in conversation history. 22/22 contract tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this brings
This PR adds deterministic enforcement hooks to the
actions-migratorplugin. The hooks run as shell commands outside the model — the agent does not generate them, parse them, or have to remember to run them — and they're registered once inplugin/hooks.jsonfor all three Copilot surfaces (CLI, Cloud agent, and VS Code). On CLI and VS Code, a deny stops the agent because the user is the loop’s pacemaker. On Cloud agent, hooks fire and deny individual tool calls but the autonomous loop may retry through other write tools — see Enforcement strength varies by surface for the full picture and theadditionalContextpattern that strengthens denials there.Hooks overview
preToolUseadditionalContextpolicy rule for autonomous surfaces. Forcessecrets.NAMEreferences.preToolUserm,mv,git rm,git mv,unlink, andfind -deleteoutside.github/ci-archive/; emits anadditionalContextpolicy rule for autonomous surfaces. Allows CI-source archival. Blocks path traversal.postToolUseagentStop(CLI)sessionEnd(CLI) /Stop(VS Code).github/MIGRATION-SCORECARD.md— an audit artifact with session ID, timestamp, completion reason, and per-file quality table. Appends rather than overwrites, so progression across passes is visible.How enforcement works
The hooks run as shell commands invoked by the runtime, not advice the model has to remember. On CLI / VS Code that gives a hard-gate semantic (the agent stops on deny). On Cloud agent the agent may try alternate write tools after a deny — see Enforcement strength varies by surface. The deny outputs include
additionalContextcarrying a session-level policy so the autonomous loop sees the rule as it picks subsequent tools.Cross-surface support (CLI + Cloud agent + VS Code)
Hooks must work on every surface a migration can run on. The three surfaces send different payload schemas, which we captured directly from live hook invocations:
toolNametool_nametoolArgs(a JSON string)tool_input(an object)sessionIdsession_idtoolResulttool_responsebash,create,editrun_in_terminal,create_file,replace_string_in_filepermissionDecisionhookSpecificOutput.permissionDecisionagentStop+sessionEndStopA single
hooks.jsonadapts to both:.toolName // .tool_name; parses args whether.toolArgsis a JSON string (fromjson) or.tool_inputis an object; normalizescommand,filePath/path/file_path, andcontent/new_string.hookSpecificOutputshapes so each surface reads the field it expects.sessionEnd(CLI) andStop(VS Code) run the same scorecard script; theStopvariant honorsstop_hook_activeto avoid infinite loops.preToolUsehook no-ops when its field is absent.actionlint auto-install
The three hooks that use actionlint install it automatically if absent:
brew install actionlint.This removes the dependency on the agent remembering to install the linter — the hook handles it deterministically.
Migration scorecard
sessionEnd/Stopappends to.github/MIGRATION-SCORECARD.mdafter each session. Multiple passes show quality progression with per-file detail:Each workflow is checked for unpinned actions (
@v4instead of SHA), placeholder text (TODO/FIXME/etc.), over-broad permissions (write-all), missing permissions block, and actionlint errors. A workflow must pass all checks to count as "clean."Enforcement strength varies by surface
Hooks fire on all three surfaces, but how strongly they enforce depends on the execution model:
tool.execution_complete: <tool> success=falseis logged, the agent silently retries through other write tools (e.g., falls back frombashtoapply_patch).Verified in
AlexDeMichieli/cca-hook-repro-consumer: a hook denyingcreate|edit|write|str_replace|bashgot bypassed when the agent routed the same write throughapply_patch. Hook fired (success=falseon the bash call); enforcement was not hard.Implication: on Cloud agent, skills are the primary enforcement layer — the agent reading and following the rule in
migration-coreis more reliable than hooks alone, since hooks raise the retry cost but don't stop the agent. Hooks remain valuable as deterministic backstops on CLI/VS Code and as friction (plus per-call logging) on Cloud agent.Tests
A committed test suite guards against silent breakage — critical because the surfaces use different schemas and a change that breaks one would otherwise go unnoticed.
plugin/hooks.test.sh— 22 contract tests exercising every hook against both CLI and VS Code payloads (deny / allow / quality-context / scorecard / loop-guard)..github/workflows/hooks-test.yml— runs the suite on every PR touching the hooks and validateshooks.jsonparses. A schema or behavior change that breaks any surface now fails the PR.Validation performed
bash plugin/hooks.test.shPreToolUse,PostToolUse,UserPromptSubmit,Stopall fire with correctcreate_file/run_in_terminalpayloadscopilot --allow-all --agent actions-migrator:jenkins-migratorrm README.mdblocked; Jenkinsfile migrated to cleanci.yml(12 SHA-pinned actions + least-privilege permissions); original archived viagit mv; scorecard =1 clean, 0 with issuesIssues found and fixed during testing:
git mv/mv/find -deletebypass of the originalrm-only guard — the guard now covers all destructive verbs while still allowing CI-source archival into.github/ci-archive/.2>&1,> file) caused false-positive denies — the guard now strips redirect operators before checking targets./tmpquality-gate counter — now keyed persessionId.$GITHUB_WORKSPACEthen$PWDso the scorecard lands in the cloned repo, not the plugin install dir.Adoption
Consumers enable the plugin on all three surfaces with one committed file — see
consumer-template/.github/copilot/settings.json:{ "enabledPlugins": { "actions-migrator@actions-migrations-via-copilot": true } }This single declaration drives the CLI auto-install, Cloud agent plugin load, and VS Code workspace recommendation. No personalized paths or per-user setup required.
Skills retained — complementary to hooks
The
actionlintskill and the migration-core / platform skill guidance are intentionally kept. The earlier plan was to thin them out once hooks shipped, on the assumption hooks would deterministically replace skill-driven enforcement. End-to-end testing on Cloud agent (above) shows that's not safe today — the autonomous loop routes around hooks via alternate write tools.So the layering is:
No follow-up "skill removal" PR. Future PRs may reword guidance as positive instructions ("always use SHA pinning") rather than "the hook will block this," but the content stays.
Token cost reduction
Each LLM turn re-prompts the model with the full session context (which grows monotonically across the session), so turns are the unit of cost — not just dollars but also wall-clock latency. Hooks move deterministic procedural work out of those turns and into shell scripts that run in milliseconds.
Per workflow file migrated, hooks remove turns the agent would otherwise spend on:
postToolUseruns actionlint and injects results asadditionalContexton the same turn the agent just wrote the filebrew installsessionEnd/Stophook appends it deterministicallyagentStopgate forces another fix turn if issues remainAggregated across a real migration, that's roughly 4–5 turns shifted from model time to script time per workflow file. On a multi-file migration (10+ workflows) the savings compound — each saved turn is also ~5–15s of model latency, so token cost and wall-clock duration both drop.
Scope of the claim
preToolUsedeny-style hooks (secret detection, destructive guard) are friction rather than hard gates — the autonomous agent may retry through alternate tools (verified in a minimal repro). This partially offsets savings from those specific hooks on that surface.postToolUseinjection (actionlint warnings same-turn) and lifecycle hooks (sessionEndscorecard,agentStopgate on CLI) save turns on all surfaces regardless of the agent's tool choices, because they run on every relevant event.How to test it yourself
Contract tests (no Copilot session needed)
CLI (end-to-end)
VS Code Agent Plugins (preview)
To inspect raw hook execution in VS Code, open Output → "GitHub Copilot Chat Hooks" — it shows each hook firing with the stdin payload.
Cloud agent (github.com)
Where to find hook logs
Hook execution surfaces differently on each surface — useful for debugging:
copilotprocess. Denials print directly.• Action logs (workflow run "Running Copilot cloud agent") show high-level signal:
[plugins] Resolved plugin "…"confirms the plugin loaded, andtool.execution_complete: <tool> success=falseindicates a hook denied that call.•
logs/.copilot/logs-process-X.loginside the action artifacts (surfaced in staging environments) contains the full hook execution detail, including the deny reason string. Confirmed by the CCA team.Files changed
plugin/hooks.json— cross-surface enforcement hooks (secret detection, destructive guard, quality check, quality gate, scorecard) with auto-install and dual-schema input/output.plugin/hooks.test.sh— 22 contract tests across CLI and VS Code schemas..github/workflows/hooks-test.yml— CI that runs the suite on every hooks change.consumer-template/— drop-insettings.json+ README for enabling the plugin across surfaces.plugin/README.md— hooks documentation.