perf(security): decide and render the five import skills as code, and read tracker fields through one normaliser by potiuk · Pull Request #1680 · apache/magpie

potiuk · 2026-10-17T00:24:05Z

Summary

First layer of the security-family deep split (#1672): the five import skills move their deterministic rules into a new tools/security-management package, and every reader and writer of a tracker body goes through one parser, on the body-field contract's boundaries.

New package tools/security-management (stdlib, one CLI family per skill over a shared library, the same shape as tools/release-management). For security-issue-import, -from-pr, -from-md, -via-forwarder and -from-scan it decides what is rule-based — governance defaults, the Step 1 pre-filter, dedup batches, the forwarder pre-screen, scope detection, the findings parser, the reply grammar, the receipt-draft matrix, the scan bucket rule — and renders the tracker body, rollup entry, rejections ledger and recap. It runs nothing and posts nothing; report text reaches it only through files. Classification, field extraction, PII identification, milestone choice and injection flagging stay with the model.
One parser for reading and writing body fields. tools/github-body-field holds the body-field contract, and every reader and writer uses it or a byte-identical copy pinned by a test (vetted-ops, local-tracker, the Jira bridge, generate-cve-json, security-management):
- every column-0 ### line ends a field, code block or not, so no fence or macro left in one field can carry the next field into it (a stray fence in the public summary can never pull the private thread URL into the CVE description — fix(security-tracker): harden the local and Jira backends after review #1683's guarantee, kept);
- closed blocks (CommonMark fence pairs and Jira {code} / {noformat}) only decide which heading starts a field; a field whose only copies are inside closed blocks is absent;
- names are whitespace-lenient and case-sensitive (### Name names its field; ### Affected Versions is not Affected versions); CRLF is normalised;
- anything ambiguous is a needs_decision, never a guess: two different copies outside closed blocks, a known field heading behind > / a list marker / indentation, or a Jira macro opened mid-line that would hide a field;
- writes are refused when the new value holds a line that would end a field, and every rewrite must leave all other fields unchanged.
generate-cve-json reads only through the parser: a template-field conflict exits 2 naming the field and lines; --report-json lists normalisation_changes, needs_decision and missing_required; the record stays DRAFT while the summary, affected versions, CWE, severity or CVE link is unresolved. issue-sync, cve-allocate and issue-deduplicate hand missing fields to the session's own model (no new LLM hop): proposals only, applied on confirmation, recorded as AI-proposed; Severity and the CVE link are never proposed, and a reporter-assigned CWE is never copied.
Reporter text is stored inert, in a text fence one backtick longer than any run in it; every line that would end a field, and every <!-- line, gets a leading \; the rest is byte-exact. The Jira bridge and the local-files tracker escape fenced ### lines at ingest the same way.
Attachment markers count only as whole lines at column 0, outside closed blocks, after the last template field; an attach that would change any field is refused.

skill	`measured_tokens` main → branch	lines
`security-issue-import`	10,136 → 8,147	570 → 420
`security-issue-import-from-pr`	8,719 → 7,536	668 → 511
`security-issue-import-from-md`	7,367 → 6,088	532 → 431
`security-issue-import-via-forwarder`	6,485 → 5,996	359 → 329
`security-issue-import-from-scan`	6,236 → 6,302	410 → 394
five importers	38,943 → 34,069 (−12.5%)

-from-pr and -from-scan stay above the ~5,850 target, and -from-pr is 11 lines over the 500-line cap: their generated blocks (including the longer tracker-backend block from #1683) and eval-graded tables are a floor.

Type of change

Skill change (.claude/skills/<name>/) — eval fixtures updated below
Python package (tools/*/ with pyproject.toml)
Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
Documentation (docs/, README.md, CONTRIBUTING.md)

Test plan

Regression check on real data: generate-cve-json from main (including fix(security-tracker): harden the local and Jira backends after review #1683) and from this branch over all 379 issues of an adopter's security tracker, with its own config. Every difference is CR removal (main copied \r from CRLF bodies into CVE descriptions); the only exit-code change is a body with two differing Affected versions sections, which now refuses cleanly (that tracker has since been fixed by hand, as has one with a stale attachment block wrapped around its fields).
Model evals, every affected suite: security-issue-import 36, -from-pr 8, -from-md 7, -via-forwarder 6, -from-scan 4, security-issue-sync 86, security-cve-allocate 22, security-issue-deduplicate 18 — all pass (two cases that failed once passed on re-run). Deterministic suites became pytest cases named after the former cases.
ruff, ruff format, mypy and pytest for security-management, github-body-field, generate-cve-json, vetted-ops, local-tracker and jira; a readers-agree test runs a field read and a write over fifteen hostile bodies through all four readers and requires the same value, refusal or rewritten body from each; bounded-runtime tests for 100k-character lines and 10,000 container markers.
skill-surface-hash in sync without --fix, skill-token-count, check-family-plugins, check-doc-sync, render-preflight-fixtures, vendor-neutrality, specs synced and spec-validate; all pre-commit hooks on every commit
roborev general and security reviews on every commit range, and the background security review on each commit; every finding fixed

RFC-AI-0004 compliance

HITL — every state change stays a proposal; the AI fallback only proposes and writes on a yes
Privacy LLM — the fallback uses the session's own (default-approved) model; no new LLM hop
Write-access discipline — nothing in the package posts, sends, labels or commits
Vendor neutrality — placeholders only; tracker heading literals and labels come from project.md

Linked issues

Part of #1672 (umbrella #1342). #1686 (triage and invalidate) is stacked on this.

Notes for reviewers (optional)

Rebased onto #1683, which hardened the same readers in parallel with its own field rules. The two designs are merged rather than one picked: #1683's boundary rule (every ### ends a field) and Jira-block awareness now live in the shared parser, and generate-cve-json's private splitter is gone. Tests of #1683's that encoded "a fenced heading is content" or "else the first copy" were changed to the contract; each change is listed in the commit bodies of df44e4922 and 93c071ea0, and every private-field-merge guarantee still holds.

Behaviour changes worth a look:

Reporter formatting shows as plain text in the tracker; lines that would end a field, or start with <!--, show with a leading \.
An empty field renders _No response_ (apply.md said *No response*).
render-body refuses a body it cannot verify instead of inlining it; dedup blocks on a thread ID shape it does not recognise; overlapping scope patterns block like a mixed-scope PR.
generate-cve-json exits 2 on a duplicate, a container heading or an ambiguous Jira macro in a field it reads.
The local-tracker plugin no longer pulls github-body-field from PyPI outside the workspace (it now ships a pinned copy).
tools/spec-loop/.last-sync is not bumped (this branch is not on main); run tools/spec-loop/loop.sh update after merge.
A Copilot Autofix commit's AI Co-authored-by trailer was dropped from history (AGENTS.md); its message credits the autofix instead.

Planned follow-up PR: a normative body-field contract with a shared conformance suite, a basic Groovy implementation, the local-files tracker's Git/SVN storage through the parser, and setup config generating a project-local parser for a non-default report layout.

Generative AI disclosure: prepared with Claude Code (Claude Opus 5); reviewed by the author.

🤖 Generated with Claude Code

…ed by CodeQL Addresses the CodeQL finding "Implicit string concatenation in a list" on #1680. The change was suggested by GitHub's Copilot Autofix and accepted in the PR; the co-author trailer it added is dropped, since this repository does not attribute AI tools as co-authors (see AGENTS.md). Part of #1672

A new tools/security-management package holds the deterministic rules of the security skills, one CLI family per skill over a shared library. For security-issue-import it decides the governance defaults, the Step 1 pre-filter, the threadId dedup batches, the forwarder pre-screen, the reply grammar and the receipt-draft matrix, and renders the sanitised tracker body, rollup entry, rejections-ledger record, fix-already-public reply and recap. It runs nothing; report text reaches it only through files. SKILL.md becomes a router (9,864 -> 7,799 tokens); headings and golden rules are unchanged. The step-0, step-6 and step-7 eval suites become pytest cases named after each former case. Empty body fields now read _No response_, headings follow tracker.body_fields, fenced code is no longer demoted, and the lone-fence rule no longer strips the closing fence of every code block. Part of #1672 Generated-by: Claude Opus 5

…as code A new issue-import-from-pr family in tools/security-management parses the PR reference (refusing any repository other than upstream), detects the scope from the changed files (tests stripped, mixed-scope stop, several sub-packages of one scope), checks the milestone by exact match, builds the duplicate-tracker query and flags a 30-hit result, and renders the title, the tracker body with its sentinels, the labels, the first rollup entry and the recap. It reuses the shared body, rollup, refs and titles modules and runs nothing; PR-derived text reaches it only through files. SKILL.md becomes a router (8,447 -> 7,150 tokens, 659 -> 499 lines); headings, golden rules, the Reporter credit policy and the adversarial-review block are unchanged. The mixed-scope blocker, the scope mapping, the body-field table, the board recipe and the sibling comparison moved byte-for-byte into outcomes/ and backends/. The step-2-scope-detection eval suite becomes pytest cases named after each former case. The PR body is sanitised and linked into upstream instead of optionally fenced (a fence cannot hide a ### line from the body-field parser), and a bot author is checked by login as well as by name. Part of #1672 Generated-by: Claude Opus 5

…code fences strictly The tracker body is read by two parsers: GitHub's CommonMark renderer and github-body-field, which toggles "inside a fence" on every fence-like line. The sanitiser now pairs fences the CommonMark way (at most three spaces of indent, closed only by a bare run of the same character at least as long) and requires every fence-like line to be one of those openers or closers, so the two readers agree on what is code. A body that does not pair - an unclosed fence, ``` closed by ~~~, ```` closed by ```, an indented fence - or has an unbalanced <details> or <!-- is no longer inlined verbatim: render-body refuses with exit 2 and the indicator, in issue-import and issue-import-from-pr alike, and the skill stops. Every ![ outside code is defused, not only the inline and full-reference forms, and the < of img, picture, source, video, audio, svg, image, iframe, object, embed, details and summary tags and of <!-- is escaped. Short fields get the same treatment plus an escaped first character on any fence-like line, and render-body reports them in sanitised_fields; redaction tokens still pass through unchanged. In issue-import-from-pr the remediation developer is kept in a code span in the rollup and sanitised in the body, a sub-package path segment is substituted literally into packageName, and a segment that is not a plain name (\1, \g<0>, ..) is reported as unmatched instead. Part of #1672 Generated-by: Claude Opus 5

…nder the scan report as code A new issue-import-from-scan family in tools/security-management says what each disposition bucket allows, so Golden rule 3 holds in code: only import-as-tracker creates a tracker, PR-worth and defense-in-depth are open-a-PR-or-skip, and the rest take no action. It renders the per-scan triage report (distribution line, a section per bucket, a row per finding) and, across several scans, the outcome table, the breakdown with percentages and the severity-vs-disposition table; it parses the operator's decision grammar, holding everything on an ambiguous reply; and it lists what a public report must not carry (private tracker references, CVE IDs not yet public, embargo terms, bare #NNN) without rewriting it. Bucketing, grounding and the scanner assessment stay with the model. It runs nothing. SKILL.md: 5,964 -> 6,030 tokens, 401 -> 385 lines. The skill was nearly all judgement, so the commands it now names cost about what the moved prose saved; headings, golden rules, the Step C table and the adversarial-review block are unchanged. The skill composition list, the folder-discovery recipe, the per-source destinations and the old Step D prose moved byte-for-byte into outcomes/ and backends/. The step-c-bucket suite stays a model eval; its creates_tracker rule is pinned in pytest for every bucket. Intentional changes: a by-design, duplicate or already-fixed row without its grounding is refused, and the scrub now runs on every public surface (gist and public issue_analysis.md as well as the report-back PR). Part of #1672 Generated-by: Claude Opus 5

…y reader agrees on The parser now pairs code fences the CommonMark way (same character, a closer at least as long, nothing but whitespace after it), so a four-backtick block holding three backticks no longer hides the fields after it, and ~~~ no longer closes a ``` block. A field heading is any level-3 ATX heading outside a fence: two spaces or a tab after ###, up to three spaces of indent and a closing run of # all count, and lines end at CommonMark's line endings only. The new normalise(body, known_fields, keep) returns the canonical body with a change list and the decisions it will not make: CRLF and CR become LF, headings become "### Name" (taking a known field's spelling when the case-folded name matches), a heading-shaped line inside a fence is escaped, an unclosed fence is closed, and a repeated field is collapsed when every copy holds the same value. Copies with different values are left alone and reported, unless the caller passes the line of the copy a human chose to keep. A print-only `normalise` subcommand reports the same without ever writing body text to stdout. A duplicate field now raises DuplicateFieldError, a FieldNotFoundError subclass, so callers no longer match on the message. The vetted-ops copy of the parser is re-synced. Part of #1672 Generated-by: Claude Opus 5

…refuse ambiguous duplicates cleanly Since the switch to github-body-field's parser, a summary heading written with two spaces after ### was silently missed, leaving the CVE description empty, and a field repeated with different values ended the run in a traceback. The body is now normalised in memory against the template field names first: lenient or differently cased headings name their field again, CRLF bodies read the same as LF ones, and copies of a field holding the same value collapse to one. A field repeated with different values exits 2 with the field name and the heading line numbers, and no record is written; the tool never picks a copy. The new --report-json flag writes normalisation_changes, needs_decision and missing_required (the summary, affected versions, CWE, severity, credit and CVE link, each marked with whether an agent may propose a value; never for severity or the CVE link) for the calling skill to act on. It holds no body text and is written on exit 2 too. Run over the 379 real tracker bodies against main, every record is the same except one, which now stops on its differing duplicate; the only other change is that CRLF bodies no longer carry carriage returns into the CVE description. Part of #1672 Generated-by: Claude Opus 5

…ock quote for a field The lenient heading and fence rules accepted up to three spaces of indent everywhere, but inside a list item or block quote that indent places the line in the container: GitHub renders "- item" followed by " ### Severity" as part of the list, while the parser read a top-level field, and an indented fence there could hide every field after it. The parser now tracks a simple container context (a line starting with >, a bullet or an ordered marker, its indented lines and lazy continuations, ended by an unindented line after a blank line or an unindented heading or fence). Container lines are neither fields nor fences, and normalise never rewrites them. As a second line of defence, normalise cross-checks its result with a strict reader (### at column 0, fences opened only at column 0). Where the two disagree about a field heading it records a container-heading decision instead of picking a reading, and generate-cve-json exits 2 on it as it does for a differing duplicate. The vetted-ops copy is re-synced, and a parity test runs both implementations over the same tricky bodies. Over the 379 real tracker bodies nothing changes: no container-heading decision fires, and every CVE record matches the previous commit. Part of #1672 Generated-by: Claude Opus 5

…th the session's own model generate-cve-json never calls a model, so a tracker whose summary, affected versions, CWE or credit is empty, or whose body repeats a field with different values, needed a human to notice the gap. Every skill step that regenerates the record (security-issue-sync, security-cve- allocate, security-issue-deduplicate) now passes --report-json and, when the report lists missing_required or needs_decision, follows one shared procedure in issue-sync/outcomes/cve-json-fallback.md. The model already running the skill reads the tracker body and proposes values, labelled AI-proposed and applied only on explicit confirmation through the normal field write, with the confirmation recorded in the rollup. No new LLM hop is added. Severity is never proposed (the team scores it independently; a reporter's CVSS stays informational) and the CVE link comes only from allocation. For a field repeated with different values the user sees each candidate and picks one; the normalised body is then offered as a whole-body rewrite, applied only on confirmation. Part of #1672 Generated-by: Claude Opus 5

…of modelling lists The container tracker approximated CommonMark's list and block-quote rules (content offsets, thematic breaks, which markers may interrupt a paragraph) and got them wrong in ways a crafted body could exploit. It is gone. A field heading is now only "### <name>" at column 0, or the lenient form (extra spaces or tabs after ###, closing #s) at column 0 after a blank line or a field heading, or an indented one only right after a field heading or at the start of the body. Everything else is content. Any other heading-shaped line outside fences whose name, after stripping > and list markers, is a known field is reported as a container-heading decision: never taken as the field, never ignored, and generate-cve-json exits 2 on it. The strict cross-check now reads fence content unescaped, so an indented fence that a renderer would close at the end of a list can no longer hide a field silently. The vetted-ops copy is re-synced and the parity fixtures cover the new cases. The 379-body corpus is unchanged: no container-heading decision fires, and only #32 and #352 differ from main. Part of #1672 Generated-by: Claude Opus 5

… then verify the body The importers inlined the reporter's text as markdown and relied on a sanitiser to make it safe: demoting headings, escaping headings inside fences, defusing images and HTML, dropping empty fence pairs and refusing bodies whose fences did not pair strictly. Each new markdown trick needed another rule. The description (the email body, the PR description, each findings-file section) now goes in as one fenced block with info string `text`, its fence one backtick longer than the longest backtick run in the text (at least three), and the content byte-exact. No line of the text can close the block, and nothing inside it renders: no image loads, no HTML or comment opens, no heading becomes a section, no reference links. Short fields stay one-line escaped values. Before a body is returned it now goes through github-body-field's normaliser: it must hold exactly the template fields, once each, with nothing to decide and no rewrite but headings inside fences, or the render is refused. A body holding a line-break character other than \n is still refused. The sanitiser code this made dead is removed, and the reference linker pairs fences the CommonMark way, so it never links inside the block. Behaviour change: reporter formatting now shows as plain text in the tracker, and a malformed fence in the report no longer blocks the import. The import skills' sibling docs describe the new shape. Part of #1672 Generated-by: Claude Opus 5

… layer, in linear time The nested-heading check used a regex whose repeated group could match whitespace several ways, so a long line of quote and list markers backtracked catastrophically. It is now a loop that strips one marker at a time, at most 64 of them, behind a cheap "###" pre-check; a 100k-char line of markers normalises in well under a second. The fail-closed rules ran only inside normalise, so a caller that read or wrote a field directly could skip them. extract_field and replace_field now run the strict cross-check and the nested-heading detection for the requested field themselves and raise AmbiguousFieldError, a FieldNotFoundError subclass, when readers could disagree. The CLI's get and set exit 2 on it without writing; the vetted-ops body-field-get and body-field-set procedures refuse it, and generate-cve-json reports it as a clean exit 2 instead of reading an empty field. The vetted-ops copy is re-synced. The 379-body corpus is unchanged: only #32 and #352 differ from main. Part of #1672 Generated-by: Claude Opus 5

…ock and unify body-field readers The inert block is inert only to fence-aware readers, and the tree had several: the local-files tracker toggled fence state on any ``` or ~~~ line, the Jira bridge's Markdown path closed a fence on any fence line, and generate-cve-json placed its attachment with a regex that ignored fences. The local tracker now pairs fences through github-body-field's fence_roles (a new workspace dependency, stdlib-only), the Jira bridge pairs them by the same CommonMark rule (same character, a closer at least as long, nothing after it), and generate-cve-json finds the CVE tool link section with the shared parser. As defence in depth for any reader that ignores fences, a line inside the inert block that would be a heading naming a template field if the fence were ignored (after leading whitespace, > and list markers: ###, a space or tab, the name) now gets a leading backslash. Every other line stays byte-exact, and the render output lists the escaped lines (escaped_lines). Intentional behaviour change: such lines in a report or PoC show with a backslash in the tracker. A new test renders reports full of ```, ~~~ and ```` runs and fake (github-body-field, the vetted-ops copy, generate-cve-json, the local tracker, the Jira bridge, and a fence-blind reader) finds exactly the template fields, once each. The parser exposes fold_name and loose_heading_name for this; the vetted-ops copy is re-synced. The 379-body corpus is unchanged: only #32 and #352 differ from main. Part of #1672 Generated-by: Claude Opus 5

…fore a heading The nested-heading check stopped after 64 markers and then reported no heading, so a line of 65 > characters before "### Severity" slipped past the container-heading decision. It is now one forward-only scan over the line (whitespace, then a >, a bullet, or an ordered marker of one to nine digits followed by a space or tab, repeated until none matches), so it stays linear in the line length with no cap. Tests cover 65 and 10,000 markers of each kind, and the vetted-ops copy and parity fixtures are re-synced. The 379-body corpus is unchanged. Part of #1672 Generated-by: Claude Opus 5

normalise matched a heading to a known field case-insensitively, which main never did. On a real tracker the reporter's own "### Affected Versions" heading in the description became a third copy of the "Affected versions" field and a duplicate decision. Matching now collapses whitespace but keeps case, in the heading canonicalisation and in the container-heading detection alike, so a differently cased heading is content again. "### Affected versions" still canonicalises. The case-insensitive comparison stays only for the inert-block escapes, which guard readers that compare case-insensitively. Over the 379-body corpus every record matches main once carriage returns are stripped, except #352, which still exits 2 but now names only its two real duplicate headings (lines 239 and 248). The vetted-ops copy is re-synced. Part of #1672 Generated-by: Claude Opus 5

…ext and verify fields survive an attach --attach found its begin/end markers with a fence-unaware text search, and the inert report block keeps report text byte-exact. A report carrying "" therefore made the next attach before CVE allocation delete everything from that line to the end of the body, every trusted field after it included, and the paired form did the same between two planted markers. Markers now count only as the tool writes them: the whole line, at column 0, outside any fence (github-body-field's CommonMark pairing), and after the last template field heading, where the attachment always goes. The block is replaced by validated line offsets, not a text search. Before any PATCH the old and new bodies are parsed with the shared parser, and the attach exits 2 without writing unless every field outside the attachment keeps its name, order and value. At import, any line of report text that starts with "<!--" is now escaped inside the inert block (listed in escaped_lines), so machine markers never travel in report text; short fields already escape it. Over the 379 real tracker bodies the locator finds 93 of the 94 markers the old search found, and re-attaching changes no field in any of them. The one it skips (#492) is a begin marker above the template fields whose end marker sits below them, exactly the span the old code would have deleted on the next attach. The CVE-record corpus check is unchanged: only #352 differs from main once carriage returns are stripped. Part of #1672 Generated-by: Claude Opus 5

…yloads exact Four review findings, all held: - replace_field checked only the requested field, so replacing the description of "### The issue description ... ' ### Severity' ..." silently deleted Severity. It now refuses (AmbiguousFieldError) when the replaced span, old or new, holds a heading-shaped line outside fences, whatever its name, and when the rewritten body would change any other field; vetted-ops body-field-set inherits the refusal. - normalise escaped heading-shaped lines inside fences, so a fenced "# comment" in a public summary reached the CVE description as "\# comment" and a duplicate-resolution rewrite altered PoC code. Fenced lines are now never rewritten; a fence only decides what counts as a heading. The known-field escapes stay where they belong, in the import's inert block. - The strict cross-check accepted only "### name", so a "### Severity" after an indented list fence produced no decision. It now reads column-0 headings under the same spacing rules as the lenient reader, and a heading-shaped line naming a known field where the two fence views disagree is a decision too. - Heading parsing used regexes with adjacent whitespace quantifiers, quadratic on long space runs. It is now index scans; 100k-space headings parse in well under a second, also through the inert block. The CLI now reads body files as bytes and fetches remote bodies as JSON, so CRLF reaches normalise intact: "changed" and --output agree. The vetted-ops copy is re-synced with new parity fixtures. Over the 379-body corpus nothing changes: only #352 differs from main once carriage returns are stripped, and the attachment locator still finds 93 of 94 markers (the skipped one is #492's above-the-fields marker). Part of #1672 Generated-by: Claude Opus 5

…lds are unresolved Declining a proposed summary left the field empty, but generate-cve-json put the issue title in its place, so a complete-looking record could still advance to REVIEW, or to PUBLIC once an advisory URL was set. The generator now keeps CNA_private.state at DRAFT while any required field is empty (the summary the title stood in for included), in the record and in the attached table alike, and --report-json carries the state it gave (cna_private_state). An empty reporter credit is the one exception: anonymous and internally found reports have none, and two published records in the real tracker corpus carry only a remediation credit; holding them back would walk PUBLIC records back to DRAFT. The fallback doc now runs whenever any of normalisation_changes, missing_required or needs_decision is non-empty (the normalisation-only tidy-up was unreachable before), tells the agent to confirm the state in the report before advancing anything, and the callers in issue-sync, cve-allocate and issue-deduplicate dispatch on the same rule. The 379-body corpus check is unchanged: only #352 differs from main once carriage returns are stripped. Part of #1672 Generated-by: Claude Opus 5

The specs still described the importers' markdown sanitiser and a generate-cve-json that read fields with a first-match search. They now record what this branch ships: - cve-tooling: normalise then extract with no guessing (lenient but case-sensitive headings, differing duplicates and disputed headings exit 2), --report-json, DRAFT until the required fields are filled, attachment markers taken only where the tool writes them with a field-preservation check before every PATCH, and the agent fallback in the calling skills. - security-issue-lifecycle: the report as an inert fenced block with its two escapes, verification of the rendered body, fail-closed field reads and writes across every reader in the tree, and the confirmed-proposal fallback for CVE fields. - adapters and vetted-command-surface: github-body-field's normalise and its refusals, mirrored by the vendored copy behind body-field-get and body-field-set. .last-sync is left as it is: it records the main commit the specs were last reconciled against, this branch is not on main yet, and the main commits between that marker and the branch base were not reviewed here. Part of #1672 Generated-by: Claude Opus 5

replace_field checked the rewrite against the lenient parser only, so replacing A in "### A ... ### B" with an indented fence closed by a column-0 run passed: CommonMark closes the fence there, while a column-0 reader opens one and loses B. The rewritten body now goes through the full strict-vs-lenient and nested-heading check, with every field of the old body as a known name, and any disagreement refuses the rewrite before vetted-ops body-field-set can PATCH. Spacer detection and replacement assumed LF. On a CRLF body, get returned the blank spacer as part of the value, and set dropped it and wrote mixed line endings. Blank spacers are now recognised under any line ending, and the replacement value and spacers are written in the body's own convention (CRLF, lone CR or LF), so setting an unchanged value is byte-identical and the CLI writes nothing. The vetted-ops copy is re-synced. Part of #1672 Generated-by: Claude Opus 5

…e the DRAFT rule exactly A first attach went right after the CVE tool link section. When that field came before another template field, the attachment landed in the middle of the template, and the marker locator, which looks only after the last template field, then took it for field content and refused the next, safe update. The attachment now goes after the last template field's value wherever CVE tool link sits; a reordered-body regression covers the first attach and the refresh after it. The fallback doc and the generator's SKILL.md said any missing field keeps the record in DRAFT, which is not what the code does. They now state the rule as applied: an empty summary (title substitution included), affected versions, CWE, severity or CVE link keeps DRAFT, and a template-field needs_decision stops the run; an empty reporter credit or a duplicate of a field the tool does not read does not hold the record back, so anonymous and remediation-only records keep REVIEW or PUBLIC. The corpus checks are unchanged: only #352 differs from main once carriage returns are stripped, the attachment locator finds 93 of the 94 markers, and the three records the stricter rule would have walked back (#355, #373, #482) stay PUBLIC. Part of #1672 Generated-by: Claude Opus 5

…g boundaries Two model-eval regressions from the code split, both wording that lost a boundary main still had. The import recap now says one entry per draft_id, a consolidated receipt once under the first candidate it covers, which is what render-recap already does. The findings parser's warnings hold format problems only; a prompt-injection attempt is flagged to the user separately, never as a parse warning. All eight affected suites pass: import 36, import-from-pr 8, import-from-md 7, import-via-forwarder 6, import-from-scan 4, sync 86, cve-allocate 22 and deduplicate 18. Two cases that failed once (import step-2b case-4, sync step-2b case-6) passed when re-run alone. Part of #1672 Generated-by: Claude Opus 5

…ed by CodeQL Addresses the CodeQL finding "Implicit string concatenation in a list" on #1680. The change was suggested by GitHub's Copilot Autofix and accepted in the PR; the co-author trailer it added is dropped, since this repository does not attribute AI tools as co-authors (see AGENTS.md). Part of #1672

…dy-field contract's boundaries Rebasing this stack onto #1683 brought two designs together. #1683 made generate-cve-json follow the body-field contract with its own field splitter: every `###` line ends a field, code block or not, and closed code blocks only choose which `### <heading>` starts the field. No fence or Jira macro left in one field's text can then merge the next field into it, so a stray fence in the public summary can never pull the private mailing-list thread into the published CVE description. This stack had instead routed every reader through github-body-field, where a `###` line inside a fence was content, and the rebased tip failed #1683's tests for exactly those cases. The contract now lives in github-body-field, for reading and writing alike, and nothing else splits fields on its own. Every column-0 line of `###` followed by a space, a tab or nothing ends a field. Closed blocks (CommonMark fence pairs and Jira `{code}` / `{noformat}` blocks, scanned by one implementation; a block left open marks nothing) pick the first `### <name>` outside them, else the first, and two differing copies outside them are a duplicate decision. An indented or quoted heading ends no field and stays a container-heading decision when it names a known field. normalise closes a fence left open inside the field it opens in, and replace_field refuses a value holding a line that would end the field, or a rewrite that moves another heading into or out of a closed block. generate-cve-json drops its private splitter and reads values and the attachment anchor through the parser. The import tools' inert block escapes every line that would end a field, whatever its name. The local-files tracker pairs fences through a byte-identical copy of the parser instead of a workspace dependency the vetted-ops plugin cannot resolve. Part of #1672 Generated-by: Claude Opus 5

…parser The Jira bridge and the local-files tracker still split tracker bodies into fields with their own section scanners, so the same body could give a different field value there than in github-body-field and generate-cve-json. Both now read and write body fields through a byte-identical copy of github-body-field's parser (the vetted-ops plugin ships them without the workspace, as it already did for local-tracker), pinned by a test in each package. Jira bridge: wiki h1. to h3. headings outside closed code blocks read as `### <Heading>` before the parser sees the body, and the parser's block scanner decides what is code, so a {code} or {noformat} macro opens only at the start of a line and a block left open hides nothing. Every column-0 `###` line ends a field, code block or not. get_field and set_field resolve the name with case and whitespace ignored, then call the parser's extract_field and replace_field; a replacement is mapped back onto the wiki body and refused when the value would add, hide or move a field heading. `body` exports exactly the fields field-get reads, so generate-cve-json sees the same values. Duplicate, disputed and unsafe writes exit 4. Local-files tracker: `###` body fields go through the parser for reads, replacements and appends; an append must leave every other field as it was and read back as written. The managed comment and rollup sections, comment headings and a non-`###` field level keep the heading-level section split. Field values, comments and rollup entries have every line that would end a field escaped, fence or not, as the import's inert block does; a whole report body keeps its field headings. Tests that encoded the opposite rule now assert the contract: - local-tracker test_headings_inside_code_fences_are_ignored: get_field(B) is None became get_field(B) == "```" (with no other copy the fenced heading is the field), and "### B" in get_field(A) became get_field(A) == "```" (the fenced `### B` ends field A); added: a copy of B outside the fence wins, and C reads "c". - jira test_headings_inside_code_blocks_are_ignored: get_field(fenced, "fake") is None became == "```", plus a copy outside the fence wins. - jira test_shorter_fence_inside_longer_fence_does_not_close_it, test_fence_closes_only_on_the_same_character (both bodies) and test_fence_close_must_be_bare asserted get_field(body, "Field") is None for a `### Field` inside a closed fence. They now assert that with a real `### Field` appended outside the fence, the real one is read: that holds only if the fence paired correctly (a mispaired fence leaves two copies outside closed blocks, a duplicate), which is what these tests were written to check. Their other assertions are unchanged. The readers-agree test reads field lists through each implementation's own parser path and now also runs a field read and a write over fifteen bodies (fenced, Jira-macro and unclosed blocks, duplicates, CRLF, two-space and closing-hash headings, container headings, private-field merge attempts) through github-body-field, the vetted-ops copy, the local-files tracker and the Jira bridge, requiring the same value, refusal or rewritten body from each. Part of #1672 Generated-by: Claude Opus 5

A security review of the previous commit found two ways reporter text could still become a field. When every copy of a `### <name>` heading sat inside closed code blocks, the parser fell back to the first copy. On the Jira and local-files trackers a field can legitimately be absent, so a reporter's fenced `### Severity` became the Severity field. A heading inside a closed block now never starts a field: a field whose only copies are inside closed blocks is absent (extract and replace report it not found, list_fields and field_sections leave it out, normalise records nothing for it). Every `###` line still ends a field, code block or not, so no block can merge two fields either. Jira's renderer opens `{code}` and `{noformat}` anywhere in a line, while the parser opens a block only on a macro at the start of one, so a heading Jira shows as code could parse as a field. Whichever reading is right depends on the renderer, so the parser fails closed: a line outside closed blocks that leaves such a macro open, closed on a later line with field boundaries between, is a `macro-ambiguous` decision (the first hidden field; the opener's line, then each hidden heading's). Extract and replace refuse the field the macro opens in and every field it would hide, a rewrite that would create such a span is refused, and generate-cve-json exits 2 on it as it does on a container heading. A macro opened and closed on the same line stays text. At ingest, quoted text now never becomes a field copy at all: the local-files tracker's whole-body path (create, edit) and the Jira bridge's create and edit escape every `###` line inside a closed block with a leading backslash, as the import's inert block does. The local-files tracker closes a fence a body leaves open in the field it opens in, before the next field heading, instead of at the end of the body, where the closer would have moved the later field headings into a block. The vetted-ops, local-tracker and Jira copies are re-copied from the parser. The trackers' case-insensitive name lookup now applies the parser's ambiguity refusal before its duplicate check, as the parser does, and the local-files tracker writes an empty value as the parser does instead of as one blank line. Tests that relied on the "else the first" fallback: - github-body-field test_a_heading_inside_a_closed_fence_ends_the_field_but_never_becomes_it: the field quoted only inside a fence now raises FieldNotFoundError instead of returning the quoted value. - generate-cve-json test_code_blocks_cannot_merge_a_private_field_into_a_public_one (#1683): with a fence or {code} block closed in a later field, the private thread heading sits in the closed block, so the thread field is now absent ("") instead of read from inside the block; with the macro inside inline code (`{code}` in prose, closed later), both the summary and the thread are now refused as macro-ambiguous. The guarantee the test exists for holds in every case: the public summary never holds the private thread URL. - The previous commit's changes to #1683's Jira and local-tracker tests are undone where they asserted the fallback: get_field(fenced, "fake"), get_field(body, "Field") in the three Jira fence-pairing tests and get_field(body, "B") in the local-tracker test are None again, as #1683 wrote them; the added checks that a real copy outside the fence is read stay. The Jira test of a mid-line macro now expects the refusal. The corpus check (379 tracker bodies) produces output identical to the previous commit: no difference from main beyond CR removal, and #32's summary equal to main. Part of #1672 Generated-by: Claude Opus 5

CodeQL flagged the `except FieldNotFoundError: pass` in both trackers' field-name lookup as an empty except clause. It is intentional: the loop only probes each spelling of the name for the parser's ambiguity refusal, an absent spelling is fine, and the duplicate check follows. The clause now says so. Part of #1672 Generated-by: Claude Opus 5

potiuk marked this pull request as ready for review October 17, 2026 00:24

github-advanced-security Bot found potential problems Oct 17, 2026

View reviewed changes

Comment thread tools/security-management/src/security_management/issue_import_from_md/render.py Fixed

potiuk mentioned this pull request Oct 17, 2026

docs(spec-loop): sync specs with main up to 43e4c6eeb #1682

Merged

3 tasks done

potiuk force-pushed the perf/security-1-import branch from aa162c4 to 397aa83 Compare October 17, 2026 09:53

potiuk mentioned this pull request Oct 17, 2026

perf(security): decide and render triage and invalidate as code #1686

Merged

7 tasks done

potiuk added this pull request to stack #1689 October 17, 2026 15:56

potiuk force-pushed the perf/security-1-import branch from 397aa83 to 93c071e Compare October 17, 2026 21:07

github-advanced-security Bot found potential problems Oct 17, 2026

View reviewed changes

Comment thread tools/jira/src/jira_bridge/markup.py Fixed

Comment thread tools/local-tracker/src/magpie_local_tracker/sections.py Fixed

potiuk added 3 commits October 17, 2026 23:31

potiuk and others added 23 commits October 17, 2026 23:31

potiuk force-pushed the perf/security-1-import branch from 93c071e to 7015ef3 Compare October 17, 2026 21:35

potiuk merged commit 76cc6e8 into main Oct 18, 2026
60 checks passed

potiuk deleted the perf/security-1-import branch October 18, 2026 12:59

github-actions Bot added the substrate:analytics Tool substrate: read-only metrics / dashboards / renderers label Oct 18, 2026

Navigation Menu

Repository navigation

perf(security): decide and render the five import skills as code, and read tracker fields through one normaliser - #1680

perf(security): decide and render the five import skills as code, and read tracker fields through one normaliser#1680
potiuk merged 32 commits into
mainfrom
perf/security-1-import

potiuk commented Oct 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

potiuk commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of change

Test plan

RFC-AI-0004 compliance

Linked issues

Notes for reviewers (optional)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

potiuk commented Oct 17, 2026 •

edited

Loading