From 03032b6f0b2d9fa6582def574e35ce1d38fb05d1 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Wed, 4 Nov 2026 17:59:45 -0800 Subject: [PATCH 1/2] fix(security-issue-sync): read the hand-off rules when a gate blocks the label The pointer to release-manager-comments.md fired only when a pass proposed a transition. With a shipped release and a record still DRAFT the label cannot flip, so with no marker comment the agent never read the fill-fields rule. Trigger on the tracker's state instead: any open tracker labelled `pr merged` or `fix released`. Also strip a version suffix after a short project name in a title, such as "in Foo 2.1.0", which the earlier fix missed. A bare short name stays, since it may be a real word in the title. Generated-by: Claude Opus 5 --- .../src/security_management/cve_allocate/title.py | 2 ++ .../skills/issue-sync/signals-to-actions.md | 2 +- .../src/security_management/cve_allocate/title.py | 2 ++ tools/security-management/tests/test_issue_sync_rules.py | 7 +++++++ tools/skill-evals/README.md | 2 +- tools/skill-evals/evals/security-issue-sync/README.md | 2 +- .../fixtures/case-10-fix-released-no-handoff/expected.json | 1 + .../fixtures/case-10-fix-released-no-handoff/report.md | 5 +++++ .../case-9-release-shipped-record-draft/expected.json | 1 + .../fixtures/case-9-release-shipped-record-draft/report.md | 6 ++++++ 10 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-10-fix-released-no-handoff/expected.json create mode 100644 tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-10-fix-released-no-handoff/report.md create mode 100644 tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-9-release-shipped-record-draft/expected.json create mode 100644 tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-9-release-shipped-record-draft/report.md diff --git a/plugins/magpie-security-tools/tools/security-management/src/security_management/cve_allocate/title.py b/plugins/magpie-security-tools/tools/security-management/src/security_management/cve_allocate/title.py index 7acc513f6..6f6fd113a 100644 --- a/plugins/magpie-security-tools/tools/security-management/src/security_management/cve_allocate/title.py +++ b/plugins/magpie-security-tools/tools/security-management/src/security_management/cve_allocate/title.py @@ -59,6 +59,8 @@ def _patterns(names: Sequence[str], short: Sequence[str], prefixes: Sequence[str for name in short: leading.append(rf"^[ \t]*{re.escape(name)}{_VERSION}[ \t]*{_MARK}[ \t]*") trailing.append(rf"[ \t]*\({re.escape(name)}{_VERSION}\)\.?[ \t]*$") + # The version is required: a bare short name may be a real noun in the title. + trailing.append(rf"[ \t]+in[ \t]+{re.escape(name)}[ \t]+v?\d+(?:\.\d+)*(?:\.x)?[ \t]*\.?$") # Reporter attribution belongs in the credits field; kept end-anchored because # the pattern is loose enough to eat a substantive clause mid-title. trailing.append(r"[ \t]*\([\w.][\w. -]*[ \t]+follow-up\)\.?[ \t]*$") diff --git a/plugins/magpie-security/skills/issue-sync/signals-to-actions.md b/plugins/magpie-security/skills/issue-sync/signals-to-actions.md index 5f3ce10b1..19d38c43f 100644 --- a/plugins/magpie-security/skills/issue-sync/signals-to-actions.md +++ b/plugins/magpie-security/skills/issue-sync/signals-to-actions.md @@ -543,7 +543,7 @@ Group them by category: Re-read [`status-rollup.md` — The rollup comment shape](../../../../tools/github/status-rollup.md#the-rollup-comment-shape) before posting, and do not indent entries for "readability". -- **Remediation-developer fill-fields, release-manager hand-off and publication-ready comments** — read [`release-manager-comments.md`](release-manager-comments.md) before proposing anything when this pass proposes `pr created` → `pr merged` or `pr merged` → `fix released`, when it proposes populating the *Public advisory URL* body field, or when the tracker already carries a `remediation-developer-fill-fields`, `release-manager-handoff` or `release-manager-publication-ready` marker comment. +- **Remediation-developer fill-fields, release-manager hand-off and publication-ready comments** — read [`release-manager-comments.md`](release-manager-comments.md) before proposing anything when this pass proposes `pr created` → `pr merged`, on any open tracker labelled `pr merged` or `fix released`, even when a gate blocks the next label, when it proposes populating the *Public advisory URL* body field, or when the tracker already carries a `remediation-developer-fill-fields`, `release-manager-handoff` or `release-manager-publication-ready` marker comment. - **Advisory-records update and security-pages reminder comment** — on a closed-`announced` tracker from the Step 1g scan, read [`post-advisory-records.md`](post-advisory-records.md) before proposing anything. diff --git a/tools/security-management/src/security_management/cve_allocate/title.py b/tools/security-management/src/security_management/cve_allocate/title.py index 7acc513f6..6f6fd113a 100644 --- a/tools/security-management/src/security_management/cve_allocate/title.py +++ b/tools/security-management/src/security_management/cve_allocate/title.py @@ -59,6 +59,8 @@ def _patterns(names: Sequence[str], short: Sequence[str], prefixes: Sequence[str for name in short: leading.append(rf"^[ \t]*{re.escape(name)}{_VERSION}[ \t]*{_MARK}[ \t]*") trailing.append(rf"[ \t]*\({re.escape(name)}{_VERSION}\)\.?[ \t]*$") + # The version is required: a bare short name may be a real noun in the title. + trailing.append(rf"[ \t]+in[ \t]+{re.escape(name)}[ \t]+v?\d+(?:\.\d+)*(?:\.x)?[ \t]*\.?$") # Reporter attribution belongs in the credits field; kept end-anchored because # the pattern is loose enough to eat a substantive clause mid-title. trailing.append(r"[ \t]*\([\w.][\w. -]*[ \t]+follow-up\)\.?[ \t]*$") diff --git a/tools/security-management/tests/test_issue_sync_rules.py b/tools/security-management/tests/test_issue_sync_rules.py index 32ea4aac1..e745836c9 100644 --- a/tools/security-management/tests/test_issue_sync_rules.py +++ b/tools/security-management/tests/test_issue_sync_rules.py @@ -480,6 +480,7 @@ def test_a_failed_state_fetch_posts_nothing_on_the_hand_off_front(capsys, tmp_pa [ "Stored XSS in the task instance notes view in Apache Airflow 2.10.5", "Stored XSS in the task instance notes view (Airflow 2.10.5)", + "Stored XSS in the task instance notes view in Airflow 2.10.5", ], ) def test_a_version_noise_suffix_fails_the_title_gate(capsys, tmp_path, title): @@ -487,6 +488,12 @@ def test_a_version_noise_suffix_fails_the_title_gate(capsys, tmp_path, title): assert (out["action"], out["gate_failed"]) == ("block", "title_strip_cascade") +def test_a_short_name_with_no_version_stays_in_the_title(capsys, tmp_path): + title = "Stored XSS in the task instance notes view in Airflow" + out = decide(capsys, tmp_path, record=record(title=title), session="expired") + assert out["action"] == "hand-off" + + def test_a_malformed_cve_id_never_reaches_a_command(capsys, tmp_path): code, out = run( capsys, tmp_path, "push-decision", {"regenerated": True, "cve_id": "CVE-2026-1; rm -rf ~", **NAMES} diff --git a/tools/skill-evals/README.md b/tools/skill-evals/README.md index 2264d0b57..d8f4ad943 100644 --- a/tools/skill-evals/README.md +++ b/tools/skill-evals/README.md @@ -21,7 +21,7 @@ Suites are currently implemented for: - **security-issue-triage** — 23 cases across 7 suites (2, 2.5, 2.6, 3, 4, 6, item-failure-budget); Steps 1, 5 and 7 and the Step 6 bare-name scrub are decided and pytest-tested in `tools/security-management` - **security-issue-deduplicate** — 3 cases across 1 step (3, the merge inputs); Steps 1, 2, 3 (the merged body), 4, 5 and 6 are decided and pytest-tested in `tools/security-management` - **security-cve-allocate** — 4 cases across 1 step (4, the reporter draft); Steps 1, 2, 3, 4 (proposal and rollup), 5 and 7 are decided and pytest-tested in `tools/security-management` -- **security-issue-sync**: 60 cases across 11 steps (1d, 1f, 2a, 2b, 2c, 5b judged gates, handoff-delegate, pointer-routing, pointer-routing-gather, D0 dashboard ASF-only gate, D4 dashboard proposals); bulk selection and pre-flight, the buckets, Steps 3 and 6, the Guardrails check, the security CC, the drafting backend, the GHSA tier and the Step 5b flow are decided and pytest-tested in `tools/security-management` +- **security-issue-sync**: 62 cases across 11 steps (1d, 1f, 2a, 2b, 2c, 5b judged gates, handoff-delegate, pointer-routing, pointer-routing-gather, D0 dashboard ASF-only gate, D4 dashboard proposals); bulk selection and pre-flight, the buckets, Steps 3 and 6, the Guardrails check, the security CC, the drafting backend, the GHSA tier and the Step 5b flow are decided and pytest-tested in `tools/security-management` - **security-issue-fix** — 37 cases across 9 steps (2, 3, 5b, 5d, 5g, 7, Subversion steps 8 and 9, and pointer-routing); the flow choice, 5a, 5c, 5e, 5f, 6 and 11 are decided and pytest-tested in `tools/security-management` - **security-issue-invalidate** — 7 cases across 2 steps (3, 4); Steps 2, 5a, 5b, 5d, 5e, 5f and 7 are decided and pytest-tested in `tools/security-management` - **security-issue-import-from-md** — 7 cases across 4 steps (1, 2, 4, 6); the Step 1 parser and Step 2 keyword clean are pytest-tested in `tools/security-management` diff --git a/tools/skill-evals/evals/security-issue-sync/README.md b/tools/skill-evals/evals/security-issue-sync/README.md index 9aeffde8c..6f425e52b 100644 --- a/tools/skill-evals/evals/security-issue-sync/README.md +++ b/tools/skill-evals/evals/security-issue-sync/README.md @@ -19,7 +19,7 @@ Steps 0 (pre-flight), 1a–1c and 1e (data gathering), 1g (cve.org API check), 4 | D0 | Dashboard mode is ASF-only | 3 | An ASF project proceeds; an `independent` project and an unset (`TODO`) organization stop without asking for a dashboard session | | D4 | Dashboard-mode proposals | 3 | A dashboard-triage finding beside an untracked report; two findings for the ASF Security team; an untriaged case that may be a standing label | | Hand-off delegate | Next-step hand-off honouring a project delegate (pre-flight `step-13`) | 4 | Triage delegated (`replace`) named and invoked through `security-issue-triage`, CVE allocation delegated (`after`), a delegate for an unrelated skill (not named), and `--no-overrides` (ignored) | -| Pointer routing | `signals-to-actions.md` alone, no sibling text | 8 | The agent finds `release-manager-comments.md` and `post-advisory-records.md` from the pointers alone, including from an existing marker comment with no transition, and opens neither on a routine pass; the output spec names no file, so removing a pointer fails the case | +| Pointer routing | `signals-to-actions.md` alone, no sibling text | 10 | The agent finds `release-manager-comments.md` and `post-advisory-records.md` from the pointers alone, including from an existing marker comment with no transition and on a tracker whose next label is blocked, and opens neither on a routine pass; the output spec names no file, so removing a pointer fails the case | | Pointer routing (gather) | Step 1 alone, no sibling text | 7 | The agent finds `cve-record-comments.md`, `closed-tracker-check.md` and `release-vote-detection.md` from their conditions alone, and opens none for an open tracker with no CVE or a closed one without `announced`; the output spec names no file, so removing a pointer fails the case | ## Hard rules exercised diff --git a/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-10-fix-released-no-handoff/expected.json b/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-10-fix-released-no-handoff/expected.json new file mode 100644 index 000000000..e6ffacf61 --- /dev/null +++ b/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-10-fix-released-no-handoff/expected.json @@ -0,0 +1 @@ +{"files_to_open": ["release-manager-comments.md"]} diff --git a/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-10-fix-released-no-handoff/report.md b/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-10-fix-released-no-handoff/report.md new file mode 100644 index 000000000..52d92ec58 --- /dev/null +++ b/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-10-fix-released-no-handoff/report.md @@ -0,0 +1,5 @@ + + +An open tracker labelled `fix released`. +This pass proposes no label change, the CVE record is `REVIEW`, and no comment carries an `apache-magpie` marker. diff --git a/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-9-release-shipped-record-draft/expected.json b/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-9-release-shipped-record-draft/expected.json new file mode 100644 index 000000000..e6ffacf61 --- /dev/null +++ b/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-9-release-shipped-record-draft/expected.json @@ -0,0 +1 @@ +{"files_to_open": ["release-manager-comments.md"]} diff --git a/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-9-release-shipped-record-draft/report.md b/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-9-release-shipped-record-draft/report.md new file mode 100644 index 000000000..18ee5d82d --- /dev/null +++ b/tools/skill-evals/evals/security-issue-sync/pointer-routing/fixtures/case-9-release-shipped-record-draft/report.md @@ -0,0 +1,6 @@ + + +An open tracker labelled `pr merged`. +The release carrying the fix has shipped, but the CVE record is still `DRAFT` because *Severity* is empty, so the `fix released` label cannot be flipped. +No comment carries an `apache-magpie` marker. From 60bcde6bb67f31f521d303c9727bebc461911a61 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Wed, 4 Nov 2026 18:00:36 -0800 Subject: [PATCH 2/2] fix(adversarial-review): review a PR's head, not an empty directory The pre-PR block and security-issue-import-from-pr told agents to pass an empty temporary directory as --repo-dir for a PR with no local checkout. Reviewers there can read only the diff, so no findings is not a clean review. Give an export recipe that uses a throwaway repository, because that skill runs in the private tracker checkout, and warn in the report when --repo-dir is empty. Generated-by: Claude Opus 5 --- .../fix-workflow/pre-pr-adversarial-review.md | 19 +++++++++-- .../skills/announce-draft/site-bump.md | 19 +++++++++-- .../skills/audit-report/audit-log-pr.md | 19 +++++++++-- .../skills/prepare/automated-signing.md | 19 +++++++++-- .../skills/prepare/post.md | 19 +++++++++-- .../skills/prepare/prep.md | 19 +++++++++-- .../pre-pr-adversarial-review.md | 19 +++++++++-- .../issue-fix/pre-pr-adversarial-review.md | 19 +++++++++-- .../skills/issue-import-from-pr/SKILL.md | 4 +-- .../pre-pr-adversarial-review.md | 19 +++++++++-- .../pre-pr-adversarial-review.md | 19 +++++++++-- .../pre-pr-adversarial-review.md | 19 +++++++++-- .../model-verify/pre-pr-adversarial-review.md | 19 +++++++++-- .../pre-pr-adversarial-review.md | 19 +++++++++-- .../pre-pr-adversarial-review.md | 19 +++++++++-- .../upstream-fix/pre-pr-adversarial-review.md | 19 +++++++++-- tools/adversarial-review/README.md | 1 + .../src/adversarial_review/cli.py | 10 ++++-- tools/adversarial-review/tests/test_cli.py | 33 +++++++++++++++++++ tools/dev/blocks/pre-pr-adversarial-review.md | 19 +++++++++-- tools/spec-loop/specs/adversarial-review.md | 8 +++-- 21 files changed, 321 insertions(+), 39 deletions(-) diff --git a/plugins/magpie-issue/skills/fix-workflow/pre-pr-adversarial-review.md b/plugins/magpie-issue/skills/fix-workflow/pre-pr-adversarial-review.md index f3da97963..a4096773a 100644 --- a/plugins/magpie-issue/skills/fix-workflow/pre-pr-adversarial-review.md +++ b/plugins/magpie-issue/skills/fix-workflow/pre-pr-adversarial-review.md @@ -40,8 +40,23 @@ around that through the body file. **Where it runs.** `--repo-dir` is a checkout of the code under review — the reviewers can read every file in it. Never the project's private tracker: the tool refuses that checkout. With `--target pr:` and -no such checkout, create an empty temporary directory first, as its own -command, and pass its path. When the change is not a committed local +no such checkout, export the PR's head into a new temporary directory and +pass that: an empty directory leaves the reviewers only the diff, so they +cannot check a claim against the surrounding code, and no findings from them +is not a clean review. Run each line as its own command, in a throwaway +repository so nothing lands in the current one: + +```bash +git init -q /pr- +git -C /pr- fetch --depth=1 https://github.com/.git pull//head +git -C /pr- archive FETCH_HEAD -- . ':(exclude).apache-magpie-overrides/tools/vetted-ops' | tar -x -C +``` + +Keep the exclusion: the secure setup denies writes to that path wherever it +appears, so a full export fails partway through. The exported tree reaches +the reviewers' providers, so export only a PR that is already public. + +When the change is not a committed local branch — a helper builds it elsewhere, or the skill applies file diffs through the API — save the diff to a file in a temporary directory and review it with `--target diff:`. diff --git a/plugins/magpie-release-management/skills/announce-draft/site-bump.md b/plugins/magpie-release-management/skills/announce-draft/site-bump.md index 99ac41a8b..5cf048d61 100644 --- a/plugins/magpie-release-management/skills/announce-draft/site-bump.md +++ b/plugins/magpie-release-management/skills/announce-draft/site-bump.md @@ -70,8 +70,23 @@ around that through the body file. **Where it runs.** `--repo-dir` is a checkout of the code under review — the reviewers can read every file in it. Never the project's private tracker: the tool refuses that checkout. With `--target pr:` and -no such checkout, create an empty temporary directory first, as its own -command, and pass its path. When the change is not a committed local +no such checkout, export the PR's head into a new temporary directory and +pass that: an empty directory leaves the reviewers only the diff, so they +cannot check a claim against the surrounding code, and no findings from them +is not a clean review. Run each line as its own command, in a throwaway +repository so nothing lands in the current one: + +```bash +git init -q /pr- +git -C /pr- fetch --depth=1 https://github.com/.git pull//head +git -C /pr- archive FETCH_HEAD -- . ':(exclude).apache-magpie-overrides/tools/vetted-ops' | tar -x -C +``` + +Keep the exclusion: the secure setup denies writes to that path wherever it +appears, so a full export fails partway through. The exported tree reaches +the reviewers' providers, so export only a PR that is already public. + +When the change is not a committed local branch — a helper builds it elsewhere, or the skill applies file diffs through the API — save the diff to a file in a temporary directory and review it with `--target diff:`. diff --git a/plugins/magpie-release-management/skills/audit-report/audit-log-pr.md b/plugins/magpie-release-management/skills/audit-report/audit-log-pr.md index 7b3f84449..322c4969c 100644 --- a/plugins/magpie-release-management/skills/audit-report/audit-log-pr.md +++ b/plugins/magpie-release-management/skills/audit-report/audit-log-pr.md @@ -56,8 +56,23 @@ around that through the body file. **Where it runs.** `--repo-dir` is a checkout of the code under review — the reviewers can read every file in it. Never the project's private tracker: the tool refuses that checkout. With `--target pr:` and -no such checkout, create an empty temporary directory first, as its own -command, and pass its path. When the change is not a committed local +no such checkout, export the PR's head into a new temporary directory and +pass that: an empty directory leaves the reviewers only the diff, so they +cannot check a claim against the surrounding code, and no findings from them +is not a clean review. Run each line as its own command, in a throwaway +repository so nothing lands in the current one: + +```bash +git init -q /pr- +git -C /pr- fetch --depth=1 https://github.com/.git pull//head +git -C /pr- archive FETCH_HEAD -- . ':(exclude).apache-magpie-overrides/tools/vetted-ops' | tar -x -C +``` + +Keep the exclusion: the secure setup denies writes to that path wherever it +appears, so a full export fails partway through. The exported tree reaches +the reviewers' providers, so export only a PR that is already public. + +When the change is not a committed local branch — a helper builds it elsewhere, or the skill applies file diffs through the API — save the diff to a file in a temporary directory and review it with `--target diff:`. diff --git a/plugins/magpie-release-management/skills/prepare/automated-signing.md b/plugins/magpie-release-management/skills/prepare/automated-signing.md index 97971def6..49871c64d 100644 --- a/plugins/magpie-release-management/skills/prepare/automated-signing.md +++ b/plugins/magpie-release-management/skills/prepare/automated-signing.md @@ -95,8 +95,23 @@ around that through the body file. **Where it runs.** `--repo-dir` is a checkout of the code under review — the reviewers can read every file in it. Never the project's private tracker: the tool refuses that checkout. With `--target pr:` and -no such checkout, create an empty temporary directory first, as its own -command, and pass its path. When the change is not a committed local +no such checkout, export the PR's head into a new temporary directory and +pass that: an empty directory leaves the reviewers only the diff, so they +cannot check a claim against the surrounding code, and no findings from them +is not a clean review. Run each line as its own command, in a throwaway +repository so nothing lands in the current one: + +```bash +git init -q /pr- +git -C /pr- fetch --depth=1 https://github.com/.git pull//head +git -C /pr- archive FETCH_HEAD -- . ':(exclude).apache-magpie-overrides/tools/vetted-ops' | tar -x -C +``` + +Keep the exclusion: the secure setup denies writes to that path wherever it +appears, so a full export fails partway through. The exported tree reaches +the reviewers' providers, so export only a PR that is already public. + +When the change is not a committed local branch — a helper builds it elsewhere, or the skill applies file diffs through the API — save the diff to a file in a temporary directory and review it with `--target diff:`. diff --git a/plugins/magpie-release-management/skills/prepare/post.md b/plugins/magpie-release-management/skills/prepare/post.md index dc093d397..99866dc4f 100644 --- a/plugins/magpie-release-management/skills/prepare/post.md +++ b/plugins/magpie-release-management/skills/prepare/post.md @@ -63,8 +63,23 @@ around that through the body file. **Where it runs.** `--repo-dir` is a checkout of the code under review — the reviewers can read every file in it. Never the project's private tracker: the tool refuses that checkout. With `--target pr:` and -no such checkout, create an empty temporary directory first, as its own -command, and pass its path. When the change is not a committed local +no such checkout, export the PR's head into a new temporary directory and +pass that: an empty directory leaves the reviewers only the diff, so they +cannot check a claim against the surrounding code, and no findings from them +is not a clean review. Run each line as its own command, in a throwaway +repository so nothing lands in the current one: + +```bash +git init -q /pr- +git -C /pr- fetch --depth=1 https://github.com/.git pull//head +git -C /pr- archive FETCH_HEAD -- . ':(exclude).apache-magpie-overrides/tools/vetted-ops' | tar -x -C +``` + +Keep the exclusion: the secure setup denies writes to that path wherever it +appears, so a full export fails partway through. The exported tree reaches +the reviewers' providers, so export only a PR that is already public. + +When the change is not a committed local branch — a helper builds it elsewhere, or the skill applies file diffs through the API — save the diff to a file in a temporary directory and review it with `--target diff:`. diff --git a/plugins/magpie-release-management/skills/prepare/prep.md b/plugins/magpie-release-management/skills/prepare/prep.md index b11213cbb..aad2a557c 100644 --- a/plugins/magpie-release-management/skills/prepare/prep.md +++ b/plugins/magpie-release-management/skills/prepare/prep.md @@ -134,8 +134,23 @@ around that through the body file. **Where it runs.** `--repo-dir` is a checkout of the code under review — the reviewers can read every file in it. Never the project's private tracker: the tool refuses that checkout. With `--target pr:` and -no such checkout, create an empty temporary directory first, as its own -command, and pass its path. When the change is not a committed local +no such checkout, export the PR's head into a new temporary directory and +pass that: an empty directory leaves the reviewers only the diff, so they +cannot check a claim against the surrounding code, and no findings from them +is not a clean review. Run each line as its own command, in a throwaway +repository so nothing lands in the current one: + +```bash +git init -q /pr- +git -C /pr- fetch --depth=1 https://github.com/.git pull//head +git -C /pr- archive FETCH_HEAD -- . ':(exclude).apache-magpie-overrides/tools/vetted-ops' | tar -x -C +``` + +Keep the exclusion: the secure setup denies writes to that path wherever it +appears, so a full export fails partway through. The exported tree reaches +the reviewers' providers, so export only a PR that is already public. + +When the change is not a committed local branch — a helper builds it elsewhere, or the skill applies file diffs through the API — save the diff to a file in a temporary directory and review it with `--target diff:`. diff --git a/plugins/magpie-repo-health/skills/audit-finding-fix/pre-pr-adversarial-review.md b/plugins/magpie-repo-health/skills/audit-finding-fix/pre-pr-adversarial-review.md index c8a83f4b6..22ca046e1 100644 --- a/plugins/magpie-repo-health/skills/audit-finding-fix/pre-pr-adversarial-review.md +++ b/plugins/magpie-repo-health/skills/audit-finding-fix/pre-pr-adversarial-review.md @@ -40,8 +40,23 @@ around that through the body file. **Where it runs.** `--repo-dir` is a checkout of the code under review — the reviewers can read every file in it. Never the project's private tracker: the tool refuses that checkout. With `--target pr:` and -no such checkout, create an empty temporary directory first, as its own -command, and pass its path. When the change is not a committed local +no such checkout, export the PR's head into a new temporary directory and +pass that: an empty directory leaves the reviewers only the diff, so they +cannot check a claim against the surrounding code, and no findings from them +is not a clean review. Run each line as its own command, in a throwaway +repository so nothing lands in the current one: + +```bash +git init -q /pr- +git -C /pr- fetch --depth=1 https://github.com/.git pull//head +git -C /pr- archive FETCH_HEAD -- . ':(exclude).apache-magpie-overrides/tools/vetted-ops' | tar -x -C +``` + +Keep the exclusion: the secure setup denies writes to that path wherever it +appears, so a full export fails partway through. The exported tree reaches +the reviewers' providers, so export only a PR that is already public. + +When the change is not a committed local branch — a helper builds it elsewhere, or the skill applies file diffs through the API — save the diff to a file in a temporary directory and review it with `--target diff:`. diff --git a/plugins/magpie-security/skills/issue-fix/pre-pr-adversarial-review.md b/plugins/magpie-security/skills/issue-fix/pre-pr-adversarial-review.md index 382479e22..8c612ce64 100644 --- a/plugins/magpie-security/skills/issue-fix/pre-pr-adversarial-review.md +++ b/plugins/magpie-security/skills/issue-fix/pre-pr-adversarial-review.md @@ -40,8 +40,23 @@ around that through the body file. **Where it runs.** `--repo-dir` is a checkout of the code under review — the reviewers can read every file in it. Never the project's private tracker: the tool refuses that checkout. With `--target pr:` and -no such checkout, create an empty temporary directory first, as its own -command, and pass its path. When the change is not a committed local +no such checkout, export the PR's head into a new temporary directory and +pass that: an empty directory leaves the reviewers only the diff, so they +cannot check a claim against the surrounding code, and no findings from them +is not a clean review. Run each line as its own command, in a throwaway +repository so nothing lands in the current one: + +```bash +git init -q /pr- +git -C /pr- fetch --depth=1 https://github.com/.git pull//head +git -C /pr- archive FETCH_HEAD -- . ':(exclude).apache-magpie-overrides/tools/vetted-ops' | tar -x -C +``` + +Keep the exclusion: the secure setup denies writes to that path wherever it +appears, so a full export fails partway through. The exported tree reaches +the reviewers' providers, so export only a PR that is already public. + +When the change is not a committed local branch — a helper builds it elsewhere, or the skill applies file diffs through the API — save the diff to a file in a temporary directory and review it with `--target diff:`. diff --git a/plugins/magpie-security/skills/issue-import-from-pr/SKILL.md b/plugins/magpie-security/skills/issue-import-from-pr/SKILL.md index 83ba0fe29..c4cfb906d 100644 --- a/plugins/magpie-security/skills/issue-import-from-pr/SKILL.md +++ b/plugins/magpie-security/skills/issue-import-from-pr/SKILL.md @@ -20,7 +20,7 @@ argument-hint: "[pr-number] [repo:owner/name]" capability: capability:intake surface_hash: sha256:26be912ca52168e5 license: Apache-2.0 -measured_tokens: 6431 +measured_tokens: 6441 ---