From e05a4b403039b1790748afd35f7dedc7b21fbd2b Mon Sep 17 00:00:00 2001 From: Ian Webster Date: Thu, 27 Aug 2026 14:54:25 -0700 Subject: [PATCH 1/2] fix(publish): explain findings before scan details --- .../codex-security/references/final-report.md | 2 +- .../references/finding-detail-fields.md | 4 ++ .../skills/track-findings/SKILL.md | 4 ++ sdk/typescript/src/publication.ts | 70 ++++++++++++------- sdk/typescript/tests-ts/publication.test.ts | 49 +++++++++++++ 5 files changed, 103 insertions(+), 26 deletions(-) diff --git a/plugins/codex-security/references/final-report.md b/plugins/codex-security/references/final-report.md index 5886eff61..20ccf451c 100644 --- a/plugins/codex-security/references/final-report.md +++ b/plugins/codex-security/references/final-report.md @@ -118,7 +118,7 @@ Affected lines must include the root broken control or dangerous sink line when Then render these subsections under each finding: - `#### Summary` - - Explain why the issue matters, what the vulnerable path is, and why the current controls are insufficient. + - Lead with a short user-facing reproduction summary: required access, the product action or crafted input, expected behavior, and the observed or source-predicted result. State whether the sequence was run. Explain how the low-level failure reaches the product behavior; do not make the reader infer this from library names or source locations. For a library-only target, describe caller-visible behavior without inventing a deployment. - Wrap code identifiers, RPC names, functions, types, fields, parameters, configuration keys, and literal values in single backticks. - `#### Root Cause` - State the violated security invariant and explain exactly how the implementation breaks it. diff --git a/plugins/codex-security/references/finding-detail-fields.md b/plugins/codex-security/references/finding-detail-fields.md index da6979888..645057878 100644 --- a/plugins/codex-security/references/finding-detail-fields.md +++ b/plugins/codex-security/references/finding-detail-fields.md @@ -4,6 +4,10 @@ For every reportable finding in `findings.json`, preserve the validated reasonin ## Writing Rules +- Write the title and `summary` for the person responsible for the affected feature. Lead with who can do what, through which product action or API, and what another user would observe. Explain required access or configuration in ordinary language before naming library internals. For a library-only target, describe the caller-visible behavior; do not invent a downstream product or deployment. +- Use `attackPath.summary` for a short reproduction narrative: required access and setup, the action or crafted input, expected behavior, and the observed or source-predicted result. Say whether it was run. Do not invent UI steps, payloads, commands, or results to fill gaps. +- Connect the low-level failure to the product consequence in `rootCause.summary`: explain how the feature reaches the library, which check fails, and how that failure affects the request, user, or shared service. Keep broader impact conditional when isolation, deployment, or runtime behavior is unverified. +- Describe concrete product behavior instead of abstract security labels. State each fact once in reader-facing prose; structured fields are supporting data, not a checklist of labels to repeat in the description. - Wrap RPC names, functions, types, fields, parameters, configuration keys, literal identifiers, and short expressions in single backticks. For example: `environment/add`, `environmentId`, `execServerUrl`, and `EnvironmentManager::upsert_environment()`. - Keep code out of prose. Put source snippets in `codeEvidence[].code`, then reference them from the section that explains why the snippet matters. The workspace consolidates those referenced snippets under **Root cause** so the violated invariant and its source proof stay together. - Root cause must be a source-backed walkthrough, not a verdict paragraph. Start with the code where user-controlled data is declared, decoded, or read; follow each meaningful call, transformation, or state transition; then show the missing control, dangerous operation, and later consumer when it affects impact. diff --git a/plugins/codex-security/skills/track-findings/SKILL.md b/plugins/codex-security/skills/track-findings/SKILL.md index 93608afa0..4752b16cd 100644 --- a/plugins/codex-security/skills/track-findings/SKILL.md +++ b/plugins/codex-security/skills/track-findings/SKILL.md @@ -132,6 +132,10 @@ Treat failed requests, incomplete exact-identifier searches, unread plausible ma ### 4. Preview The Exact Writes +Before drafting issue text, read the Writing Rules in `../../references/finding-detail-fields.md`. Apply them to the validated canonical facts without changing the sealed scan. Lead with the user-facing problem and a short reproduction summary, then explain why the code causes that behavior, the proposed fix, and what was checked or remains unverified. Distinguish source-predicted behavior from an executed reproduction. If the scan does not establish the product path, say what is missing instead of inventing it. + +Write a coherent explanation, not a field dump. Fold dataflow and reachability facts into the reproduction and root-cause narrative; do not turn individual fields into numbered prose labels or repeat the same limitation under several headings. Keep necessary evidence and access requirements. Put source locations, scan identifiers, fingerprints, and generator attribution at the end, after the explanation. Preserve the audience restrictions below. + Present a compact review before any mutation. For every finding show: - finding id and fingerprint diff --git a/sdk/typescript/src/publication.ts b/sdk/typescript/src/publication.ts index dfa3bef36..03f8a6719 100644 --- a/sdk/typescript/src/publication.ts +++ b/sdk/typescript/src/publication.ts @@ -112,7 +112,50 @@ function renderFindingDescription( ): string { const { coverage } = contract; const { scan } = contract.manifest; - const lines = [ + const lines = ["## Summary", "", finding.summary]; + + if (finding.attackPath?.summary !== undefined) { + lines.push("", "## Reproduction summary", "", finding.attackPath.summary); + } + + const rootCause = finding.rootCause; + if (typeof rootCause === "string") { + lines.push("", "## Root cause", "", rootCause); + } else if (rootCause !== undefined) { + lines.push("", "## Root cause", "", rootCause.summary); + if (rootCause.code !== undefined) { + lines.push("", fencedCode(rootCause.code, rootCause.language)); + } + } + + lines.push("", "## Remediation", "", finding.remediation); + + const validation = finding.validation; + const limits = [ + ...new Set([ + ...(validation?.limitations ?? []), + ...(validation?.counterEvidence ?? []), + ...(finding.attackPath?.limitations ?? []), + ]), + ]; + if (validation?.summary || validation?.method || limits.length > 0) { + lines.push("", "## Validation", ""); + if (validation?.summary) lines.push(validation.summary, ""); + if (validation?.method) lines.push(`**Method:** ${validation.method}`, ""); + lines.push(...limits.map((limit) => `- ${limit}`)); + } + + if (finding.codeEvidence !== undefined && finding.codeEvidence.length > 0) { + lines.push("", "## Source-code evidence"); + for (const evidence of finding.codeEvidence) { + lines.push("", ...renderCodeEvidence(scan.target, evidence)); + } + } + + lines.push( + "", + "---", + "", "## Codex Security finding", "", `**Scan ID:** ${scan.id}`, @@ -148,30 +191,7 @@ function renderFindingDescription( ...finding.locations.map((location) => renderLocation(scan.target, location), ), - "", - "## Summary", - "", - finding.summary, - ]; - - const rootCause = finding.rootCause; - if (typeof rootCause === "string") { - lines.push("", "## Root cause", "", rootCause); - } else if (rootCause !== undefined) { - lines.push("", "## Root cause", "", rootCause.summary); - if (rootCause.code !== undefined) { - lines.push("", fencedCode(rootCause.code, rootCause.language)); - } - } - - if (finding.codeEvidence !== undefined && finding.codeEvidence.length > 0) { - lines.push("", "## Source-code evidence"); - for (const evidence of finding.codeEvidence) { - lines.push("", ...renderCodeEvidence(scan.target, evidence)); - } - } - - lines.push("", "## Remediation", "", finding.remediation); + ); return `${lines.join("\n")}\n`; } diff --git a/sdk/typescript/tests-ts/publication.test.ts b/sdk/typescript/tests-ts/publication.test.ts index 5581d118e..ac969d4e5 100644 --- a/sdk/typescript/tests-ts/publication.test.ts +++ b/sdk/typescript/tests-ts/publication.test.ts @@ -150,9 +150,58 @@ describe("scan publication preparation", () => { expect(issue.description).toContain("**Uploaded:** 2026-06-01T10:30:00Z"); expect(issue.description).toContain("without containment validation"); expect(issue.description).toContain("Normalize destinations"); + expect( + issue.description.indexOf("without containment validation"), + ).toBeLessThan(issue.description.indexOf("**Scan ID:**")); + expect(issue.description.indexOf("Normalize destinations")).toBeLessThan( + issue.description.indexOf("**Scan ID:**"), + ); expect(issue.description).not.toContain("/blob/deadbeef/"); }); + test("keeps the reproduction and validation limits ahead of scan metadata", async () => { + const scanDirectory = await copyExample(); + const findingsPath = join(scanDirectory, "findings.json"); + const findings = await readJson(findingsPath); + const finding = findings.findings[0]!; + const reproduction = + "Import an archive containing ../escape.txt. The importer should reject it; source review shows it writing outside the selected directory. This was not run."; + const unrun = "No runtime reproduction was run."; + const attackPath = { + summary: reproduction, + limitations: [unrun, "Files must be writable by the importing process."], + }; + const validation = { + method: "Source review", + summary: "The importer passes the entry name to the filesystem write.", + limitations: [unrun], + counterEvidence: ["The user must first choose to import the archive."], + }; + finding.attackPath = attackPath; + finding.validation = validation; + await writeJson(findingsPath, findings); + await reseal(scanDirectory); + + const { description } = ( + await prepareScanPublication(scanDirectory, DESTINATION) + ).issues[0]!; + + const metadata = description.indexOf("**Scan ID:**"); + for (const text of [ + finding.summary, + reproduction, + validation.summary, + validation.method, + ...validation.limitations, + ...validation.counterEvidence, + ...attackPath.limitations, + ]) { + expect(description).toContain(text); + expect(description.indexOf(text)).toBeLessThan(metadata); + } + expect(description.split(unrun)).toHaveLength(2); + }); + test("uses the canonical scan directory beneath an aliased parent", async () => { const root = await mkdtemp( join(tmpdir(), "codex-security-publication-alias-"), From 40ec66a73bee1d5e5605e379aabfb6a81b2e3064 Mon Sep 17 00:00:00 2001 From: Ian Webster Date: Thu, 27 Aug 2026 15:00:42 -0700 Subject: [PATCH 2/2] docs: simplify finding descriptions --- .../codex-security/references/final-report.md | 2 +- .../references/finding-detail-fields.md | 19 +++++-------------- .../skills/track-findings/SKILL.md | 4 +--- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/plugins/codex-security/references/final-report.md b/plugins/codex-security/references/final-report.md index 20ccf451c..8fa17bd1e 100644 --- a/plugins/codex-security/references/final-report.md +++ b/plugins/codex-security/references/final-report.md @@ -118,7 +118,7 @@ Affected lines must include the root broken control or dangerous sink line when Then render these subsections under each finding: - `#### Summary` - - Lead with a short user-facing reproduction summary: required access, the product action or crafted input, expected behavior, and the observed or source-predicted result. State whether the sequence was run. Explain how the low-level failure reaches the product behavior; do not make the reader infer this from library names or source locations. For a library-only target, describe caller-visible behavior without inventing a deployment. + - Start with how to reproduce the issue and what happens in the product, then explain why the code causes that behavior. - Wrap code identifiers, RPC names, functions, types, fields, parameters, configuration keys, and literal values in single backticks. - `#### Root Cause` - State the violated security invariant and explain exactly how the implementation breaks it. diff --git a/plugins/codex-security/references/finding-detail-fields.md b/plugins/codex-security/references/finding-detail-fields.md index 645057878..df5d5fa82 100644 --- a/plugins/codex-security/references/finding-detail-fields.md +++ b/plugins/codex-security/references/finding-detail-fields.md @@ -4,10 +4,9 @@ For every reportable finding in `findings.json`, preserve the validated reasonin ## Writing Rules -- Write the title and `summary` for the person responsible for the affected feature. Lead with who can do what, through which product action or API, and what another user would observe. Explain required access or configuration in ordinary language before naming library internals. For a library-only target, describe the caller-visible behavior; do not invent a downstream product or deployment. -- Use `attackPath.summary` for a short reproduction narrative: required access and setup, the action or crafted input, expected behavior, and the observed or source-predicted result. Say whether it was run. Do not invent UI steps, payloads, commands, or results to fill gaps. -- Connect the low-level failure to the product consequence in `rootCause.summary`: explain how the feature reaches the library, which check fails, and how that failure affects the request, user, or shared service. Keep broader impact conditional when isolation, deployment, or runtime behavior is unverified. -- Describe concrete product behavior instead of abstract security labels. State each fact once in reader-facing prose; structured fields are supporting data, not a checklist of labels to repeat in the description. +- Lead the title and `summary` with the user action and product impact. +- Use `attackPath.summary` to briefly explain how to reproduce the issue. +- Explain how the code causes that product behavior in `rootCause.summary`. Use plain language and avoid repetition. - Wrap RPC names, functions, types, fields, parameters, configuration keys, literal identifiers, and short expressions in single backticks. For example: `environment/add`, `environmentId`, `execServerUrl`, and `EnvironmentManager::upsert_environment()`. - Keep code out of prose. Put source snippets in `codeEvidence[].code`, then reference them from the section that explains why the snippet matters. The workspace consolidates those referenced snippets under **Root cause** so the violated invariant and its source proof stay together. - Root cause must be a source-backed walkthrough, not a verdict paragraph. Start with the code where user-controlled data is declared, decoded, or read; follow each meaningful call, transformation, or state transition; then show the missing control, dangerous operation, and later consumer when it affects impact. @@ -110,11 +109,7 @@ The following shape shows how to encode the `environment/add` reserved-environme "validation": { "method": "static source trace", "summary": "The source trace confirms that an `environment/add` caller controls both inputs, the RPC forwards them unchanged, and runtime insertion accepts `local`.", - "evidenceRefs": [ - "rpc-input", - "rpc-forward", - "runtime-upsert" - ], + "evidenceRefs": ["rpc-input", "rpc-forward", "runtime-upsert"], "assertions": [ "The runtime path lacks the reserved-ID check present during startup.", "Inserting `local` replaces the existing `HashMap` entry." @@ -143,11 +138,7 @@ The following shape shows how to encode the `environment/add` reserved-environme "entrypoint": "experimental `environment/add` RPC", "outcome": "future operations selected for `local` are routed to the remote executor" }, - "evidenceRefs": [ - "rpc-forward", - "runtime-upsert", - "default-lookup" - ], + "evidenceRefs": ["rpc-forward", "runtime-upsert", "default-lookup"], "impact": { "level": "medium", "why": "Later commands and filesystem requests selected for `local` can be routed to the attacker-controlled remote executor." diff --git a/plugins/codex-security/skills/track-findings/SKILL.md b/plugins/codex-security/skills/track-findings/SKILL.md index 4752b16cd..e546e2c16 100644 --- a/plugins/codex-security/skills/track-findings/SKILL.md +++ b/plugins/codex-security/skills/track-findings/SKILL.md @@ -132,9 +132,7 @@ Treat failed requests, incomplete exact-identifier searches, unread plausible ma ### 4. Preview The Exact Writes -Before drafting issue text, read the Writing Rules in `../../references/finding-detail-fields.md`. Apply them to the validated canonical facts without changing the sealed scan. Lead with the user-facing problem and a short reproduction summary, then explain why the code causes that behavior, the proposed fix, and what was checked or remains unverified. Distinguish source-predicted behavior from an executed reproduction. If the scan does not establish the product path, say what is missing instead of inventing it. - -Write a coherent explanation, not a field dump. Fold dataflow and reachability facts into the reproduction and root-cause narrative; do not turn individual fields into numbered prose labels or repeat the same limitation under several headings. Keep necessary evidence and access requirements. Put source locations, scan identifiers, fingerprints, and generator attribution at the end, after the explanation. Preserve the audience restrictions below. +Follow the Writing Rules in `../../references/finding-detail-fields.md`. Explain the problem, how to reproduce it, the cause, the proposed fix, and validation. Put source locations and scan identifiers last. Present a compact review before any mutation. For every finding show: