From 06a7ae7cf6b5afd3242790a7c3920a6422ffae80 Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 25 Aug 2026 08:41:33 +0800 Subject: [PATCH 1/6] chore: record delivery binding for open-issues-batch --- .specgit.yaml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.specgit.yaml b/.specgit.yaml index 4f6703d49..efd33c82d 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,8 +1,11 @@ version: 1 -delivery: dag-inspector-shows +delivery: open-issues-batch context: kind: branch - branch: fix/427-dag-inspector-shows + branch: feat/434-open-issues-batch issues: - - 427 -pr: 428 + - 434 + - 442 + - 443 + - 440 + - 436 From 109cb5fbe82d3a682ae9ff10e7d52b8fc50fc4a5 Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 25 Aug 2026 08:42:15 +0800 Subject: [PATCH 2/6] chore: record delivery binding for open-issues-batch --- .specgit.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.specgit.yaml b/.specgit.yaml index efd33c82d..a9a1a2730 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -9,3 +9,4 @@ issues: - 443 - 440 - 436 +pr: 444 From f6010f6cba18ddf00cead3e6552fca00644a071b Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 25 Aug 2026 08:59:32 +0800 Subject: [PATCH 3/6] fix(security): fetch the validated URL object in github attachment download (#442) --- .../opencode/src/cli/cmd/github.handler.ts | 19 ++++++-------- .../opencode/src/cli/cmd/github.shared.ts | 15 +++++++++++ packages/opencode/src/cli/cmd/github.ts | 2 +- .../test/cli/github-attachment-guard.test.ts | 25 +++++++++++++++++++ 4 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 packages/opencode/test/cli/github-attachment-guard.test.ts diff --git a/packages/opencode/src/cli/cmd/github.handler.ts b/packages/opencode/src/cli/cmd/github.handler.ts index 2984ea00f..c47ce3200 100644 --- a/packages/opencode/src/cli/cmd/github.handler.ts +++ b/packages/opencode/src/cli/cmd/github.handler.ts @@ -33,7 +33,7 @@ import { setTimeout as sleep } from "node:timers/promises" import { Process } from "@/util/process" import { parseGitHubRemote } from "@/util/repository" import { Effect } from "effect" -import { extractResponseText, formatPromptTooLargeError } from "./github.shared" +import { extractResponseText, formatPromptTooLargeError, isAllowedAttachmentUrl } from "./github.shared" type GitHubAuthor = { login: string @@ -787,22 +787,17 @@ export const githubRun = Effect.fn("Cli.github.run")(function* (args: { event?: const start = m.index const filename = path.basename(url) - // SSRF guard: only fetch GitHub user-attachment assets over HTTPS + // SSRF guard (issue #442): only fetch GitHub user-attachment assets + // over HTTPS, and fetch the validated URL object — not the raw + // string it was parsed from — so the token-bearing request can only + // ever target what the guard approved. const attachmentUrl = URL.parse(url) - if ( - !attachmentUrl || - attachmentUrl.protocol !== "https:" || - attachmentUrl.hostname !== "github.com" || - !( - attachmentUrl.pathname.startsWith("/user-attachments/assets/") || - attachmentUrl.pathname.startsWith("/user-attachments/files/") - ) - ) { + if (!isAllowedAttachmentUrl(attachmentUrl)) { continue } // Download image - const res = await fetch(url, { + const res = await fetch(attachmentUrl, { headers: { Authorization: `Bearer ${appToken}`, Accept: "application/vnd.github.v3+json", diff --git a/packages/opencode/src/cli/cmd/github.shared.ts b/packages/opencode/src/cli/cmd/github.shared.ts index 157d0156f..b92f1e637 100644 --- a/packages/opencode/src/cli/cmd/github.shared.ts +++ b/packages/opencode/src/cli/cmd/github.shared.ts @@ -28,3 +28,18 @@ export function formatPromptTooLargeError(files: { filename: string; content: st : "" return `PROMPT_TOO_LARGE: The prompt exceeds the model's context limit.${fileDetails}` } + +/** + * SSRF guard for attachment URLs parsed out of issue/PR bodies (issue #442): + * a request that carries the GitHub app token may only ever leave for + * github.com user-attachment paths over https. The handler fetches the + * validated URL object itself, never the raw string it was parsed from. + */ +export function isAllowedAttachmentUrl(url: URL | null): url is URL { + return ( + url !== null && + url.protocol === "https:" && + url.hostname === "github.com" && + (url.pathname.startsWith("/user-attachments/assets/") || url.pathname.startsWith("/user-attachments/files/")) + ) +} diff --git a/packages/opencode/src/cli/cmd/github.ts b/packages/opencode/src/cli/cmd/github.ts index eccbb375c..d5cd383e4 100644 --- a/packages/opencode/src/cli/cmd/github.ts +++ b/packages/opencode/src/cli/cmd/github.ts @@ -2,7 +2,7 @@ import { Effect } from "effect" import { cmd } from "./cmd" import { effectCmd } from "../effect-cmd" -export { extractResponseText, formatPromptTooLargeError, parseGitHubRemote } from "./github.shared" +export { extractResponseText, formatPromptTooLargeError, parseGitHubRemote, isAllowedAttachmentUrl } from "./github.shared" export const GithubInstallCommand = effectCmd({ command: "install", diff --git a/packages/opencode/test/cli/github-attachment-guard.test.ts b/packages/opencode/test/cli/github-attachment-guard.test.ts new file mode 100644 index 000000000..b4d48f8b4 --- /dev/null +++ b/packages/opencode/test/cli/github-attachment-guard.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, test } from "bun:test" +import { isAllowedAttachmentUrl } from "../../src/cli/cmd/github.shared" + +// Issue #442: attachment URLs parsed out of issue/PR markdown are attacker +// controllable. Anything that reaches fetch() carries the GitHub app token, +// so the guard must admit ONLY github.com user-attachment paths over https — +// the handler fetches the validated URL object itself. +describe("isAllowedAttachmentUrl (issue #442)", () => { + test("admits github.com user-attachment assets and files over https", () => { + expect(isAllowedAttachmentUrl(URL.parse("https://github.com/user-attachments/assets/abc123"))).toBe(true) + expect(isAllowedAttachmentUrl(URL.parse("https://github.com/user-attachments/files/21433810/api.json"))).toBe(true) + }) + + test("rejects other hosts, schemes, and paths", () => { + expect(isAllowedAttachmentUrl(URL.parse("https://attacker.com/pixel.png"))).toBe(false) + expect(isAllowedAttachmentUrl(URL.parse("https://github.com.evil.io/user-attachments/assets/x"))).toBe(false) + expect(isAllowedAttachmentUrl(URL.parse("http://github.com/user-attachments/assets/abc123"))).toBe(false) + expect(isAllowedAttachmentUrl(URL.parse("https://github.com/leak/user-attachments/assets/abc123"))).toBe(false) + expect(isAllowedAttachmentUrl(URL.parse("https://api.github.com/user-attachments/assets/abc123"))).toBe(false) + }) + + test("rejects null (unparseable URL)", () => { + expect(isAllowedAttachmentUrl(URL.parse("not a url ::"))).toBe(false) + }) +}) From 73ecfd2b53aa84e12aafecaf74895f5c4a384a68 Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 25 Aug 2026 08:59:56 +0800 Subject: [PATCH 4/6] fix(security): delegate escapeHtml to the escape-html package (#443) --- bun.lock | 4 ++++ packages/opencode/package.json | 2 ++ packages/opencode/src/util/html.ts | 13 +++++++------ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/bun.lock b/bun.lock index 2d9f55935..ffc9b48d0 100644 --- a/bun.lock +++ b/bun.lock @@ -608,6 +608,7 @@ "diff": "catalog:", "drizzle-orm": "catalog:", "effect": "catalog:", + "escape-html": "1.0.3", "fuzzysort": "3.1.0", "gitlab-ai-provider": "6.9.3", "glob": "13.0.5", @@ -653,6 +654,7 @@ "@types/babel__core": "7.20.5", "@types/bun": "catalog:", "@types/cross-spawn": "catalog:", + "@types/escape-html": "1.0.3", "@types/mime-types": "3.0.1", "@types/npm-package-arg": "6.1.4", "@types/semver": "^7.5.8", @@ -2826,6 +2828,8 @@ "@types/deep-eql": ["@types/deep-eql@4.0.2", "", {}, "sha512-c9h9dVVMigMPc4bwTvC5dxqtqJZwQPePsWjPlpSOnojbor6pGqdk541lfA7AqFQr5pB1BRdq0juY9db81BwyFw=="], + "@types/escape-html": ["@types/escape-html@1.0.3", "", {}, "sha512-QbNxKa2IX2y/9eGiy4w8rrwk//ERHXA6zwYVRA3+ayA/D3pkz+/bLL4b5uSLA0L0kPuNX1Jbv9HyPzv9T4zbJQ=="], + "@types/estree": ["@types/estree@1.0.8", "", {}, "sha512-dWHzHa2WqEXI/O1E9OjrocMTKJl2mSrEolh1Iomrv6U+JuNwaHXsXx9bLu5gG7BUWFIN0skIQJQ/L1rIex4X6w=="], "@types/estree-jsx": ["@types/estree-jsx@1.0.5", "", { "dependencies": { "@types/estree": "*" } }, "sha512-52CcUVNFyfb1A2ALocQw/Dd1BQFNmSdkuC3BkZ6iqhdMfQz7JWOFRuJFloOzjk+6WijU56m9oKXFAXc7o3Towg=="], diff --git a/packages/opencode/package.json b/packages/opencode/package.json index 13283a8d5..b34b67726 100644 --- a/packages/opencode/package.json +++ b/packages/opencode/package.json @@ -42,6 +42,7 @@ "@types/bun": "catalog:", "@types/cross-spawn": "catalog:", "@types/mime-types": "3.0.1", + "@types/escape-html": "1.0.3", "@types/npm-package-arg": "6.1.4", "@types/semver": "^7.5.8", "@types/turndown": "5.0.5", @@ -56,6 +57,7 @@ "dependencies": { "@actions/core": "1.11.1", "@actions/github": "6.0.1", + "escape-html": "1.0.3", "@agentclientprotocol/sdk": "0.21.0", "@ai-sdk/alibaba": "1.0.17", "@ai-sdk/amazon-bedrock": "4.0.112", diff --git a/packages/opencode/src/util/html.ts b/packages/opencode/src/util/html.ts index 55028613f..7864f6eaf 100644 --- a/packages/opencode/src/util/html.ts +++ b/packages/opencode/src/util/html.ts @@ -1,8 +1,9 @@ +import escapeHtmlLib from "escape-html" + +// Delegates to the escape-html package (issue #443): identical output to the +// previous hand-rolled replaceAll chain, but CodeQL models this library as a +// sanitizer, so js/reflected-xss stops flagging the OAuth error pages that +// render through it. export function escapeHtml(value: string) { - return value - .replaceAll("&", "&") - .replaceAll("<", "<") - .replaceAll(">", ">") - .replaceAll('"', """) - .replaceAll("'", "'") + return escapeHtmlLib(value) } From 3bf3320bbfb5d2e69c86985837b73217eed4202e Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 25 Aug 2026 09:00:19 +0800 Subject: [PATCH 5/6] fix(dag): reinforce submit_result contract and add one retry nudge (#436) --- packages/opencode/src/dag/runtime/loop.ts | 2 +- packages/opencode/src/dag/runtime/spawn.ts | 41 ++++++++-- .../dag/dag-schema-prompt-contract.test.ts | 74 +++++++++++++++++++ 3 files changed, 111 insertions(+), 6 deletions(-) diff --git a/packages/opencode/src/dag/runtime/loop.ts b/packages/opencode/src/dag/runtime/loop.ts index 0f37a9c25..f32fa00bf 100644 --- a/packages/opencode/src/dag/runtime/loop.ts +++ b/packages/opencode/src/dag/runtime/loop.ts @@ -278,7 +278,7 @@ const serviceLayer = Layer.effect( if (nodeConfig?.output_schema) { promptParts.push({ type: "text", - text: `\n\nYou MUST call the submit_result tool with a JSON payload matching this schema before ending your turn:\n${JSON.stringify(nodeConfig.output_schema, null, 2)}\nPut your full summary inside the payload. Do not repeat the payload in your message text. After submit_result succeeds, end your turn without restating the result.`, + text: `\n\n[OUTPUT CONTRACT — this node's success depends on it] You MUST call the submit_result tool with a JSON payload matching this schema before ending your turn:\n${JSON.stringify(nodeConfig.output_schema, null, 2)}\nPut your full summary inside the payload. Do not repeat the payload in your message text. Writing the payload in message text does NOT count as submitting: if you end your turn without a successful submit_result call, this node FAILS and your work is discarded. After submit_result succeeds, end your turn without restating the result.`, }) } diff --git a/packages/opencode/src/dag/runtime/spawn.ts b/packages/opencode/src/dag/runtime/spawn.ts index cf1e4b157..b1397aef8 100644 --- a/packages/opencode/src/dag/runtime/spawn.ts +++ b/packages/opencode/src/dag/runtime/spawn.ts @@ -490,12 +490,43 @@ export function spawnNode( parts: input.promptParts, }) if (input.outputSchema) { + const readSettlement = Effect.fn("DagRuntime.spawn.readSettlement")(function* () { + const updatedNode = yield* dag.store.getNode(input.dagID, input.nodeID).pipe(Effect.orDie) + const captured = updatedNode?.capturedOutput + return { + neverCalled: captured === undefined || captured === null, + // Single settlement authority shared with crash recovery + // (capture.ts settleCapturedOutput). + settlement: settleCapturedOutput(captured, input.reviewImplementationFingerprint), + } + }) clearCaptureSlot(childSession.id) - const updatedNode = yield* dag.store.getNode(input.dagID, input.nodeID).pipe(Effect.orDie) - // Single settlement decision shared with crash recovery - // (capture.ts settleCapturedOutput) — the review-result contract - // must never be enforced in one path and not the other. - const settlement = settleCapturedOutput(updatedNode?.capturedOutput, input.reviewImplementationFingerprint) + let verdict = yield* readSettlement() + // Issue #436 minimal step: a child that ended its whole turn + // without ever calling submit_result gets exactly one nudge + // turn in the same session — the work is already done, only the + // structured hand-back is missing. A captured-but-invalid payload + // is NOT nudged: that is a deterministic contract violation and + // a retry would only re-bill the same mistake. + if (verdict.settlement.kind === "fail" && verdict.neverCalled) { + registerCaptureSlot(childSession.id, input.outputSchema) + yield* promptSvc.prompt({ + messageID: MessageID.ascending(), + sessionID: childSession.id, + model, + agent: agent.name, + ...(input.variant ? { variant: input.variant } : {}), + parts: [ + { + type: "text", + text: `You ended your turn without calling the submit_result tool, so this node recorded no output and will FAIL. Do not redo the work. Call submit_result NOW with a JSON payload matching the schema from your instructions, containing your full result. If a previous submit_result call failed validation, fix the payload shape and call it again.`, + }, + ], + }) + clearCaptureSlot(childSession.id) + verdict = yield* readSettlement() + } + const settlement = verdict.settlement yield* (settlement.kind === "complete" ? dag.nodeCompleted(input.dagID, input.nodeID, settlement.output) : dag.nodeFailed(input.dagID, input.nodeID, settlement.reason, "verdict_fail") diff --git a/packages/opencode/test/dag/dag-schema-prompt-contract.test.ts b/packages/opencode/test/dag/dag-schema-prompt-contract.test.ts index 24eb8b72b..6a5acdf83 100644 --- a/packages/opencode/test/dag/dag-schema-prompt-contract.test.ts +++ b/packages/opencode/test/dag/dag-schema-prompt-contract.test.ts @@ -192,6 +192,10 @@ describe("DAG schema prompt contract (issue #386)", () => { expect(prompt).toContain("Do not repeat the payload in your message text") // A successful submission ends the turn. expect(prompt).toContain("end your turn without restating the result") + // Issue #436: the consequence of skipping submit_result is stated + // up front so the child cannot treat prose as a substitute. + expect(prompt).toContain("does NOT count as submitting") + expect(prompt).toContain("this node FAILS") yield* Deferred.succeed(gate.release, "done") }), ), @@ -238,4 +242,74 @@ describe("DAG schema prompt contract (issue #386)", () => { expect(description).toContain("Do not duplicate the payload") expect(description).toContain("end your turn without restating the result") }) + + // Issue #436 minimal step: a child that ends its turn without ever calling + // submit_result gets exactly one nudge turn in the same session before the + // node is failed. Review-aggregate children were observed (PR #424 era) + // producing the whole review in prose and never handing it back. + describe("submit_result retry nudge (issue #436)", () => { + it("nudges once when the child never submits, then completes from the submitted payload", async () => { + await Effect.runPromise( + runContractTest(({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create({ + projectID: "project-1", + sessionID: "ses_parent", + title: "Nudge completes", + config: { name: "schema-prompt-contract", nodes: [schemaNode()] }, + }) + // First turn: full analysis in prose, no submit_result call. + const first = yield* Queue.take(childPrompts) + expect(promptText(first.input)).toContain("[OUTPUT CONTRACT") + yield* Deferred.succeed(first.release, "Full review: everything checks out.") + // The nudge arrives as a second prompt to the SAME child session. + const nudge = yield* Queue.take(childPrompts) + expect(nudge.input.sessionID).toBe(first.input.sessionID) + expect(promptText(nudge.input)).toContain("without calling the submit_result tool") + // The child complies on the second chance. + const payload = { summary: "Submitted after the nudge." } + yield* store.setCapturedOutput(nudge.input.sessionID as string, payload) + yield* Deferred.succeed(nudge.release, "Submitted.") + const node = yield* pollWithTimeout( + store.getNode(dagID, "report").pipe( + Effect.map((row) => row?.status === "completed" ? row : undefined), + ), + "node did not complete after the nudge turn", + ) + expect(node.output).toEqual(payload) + }), + ), + ) + }) + + it("fails the node after the nudge still produces no submission", async () => { + await Effect.runPromise( + runContractTest(({ dag, store, childPrompts }) => + Effect.gen(function* () { + const dagID = yield* dag.create({ + projectID: "project-1", + sessionID: "ses_parent", + title: "Nudge exhausted", + config: { name: "schema-prompt-contract", nodes: [schemaNode()] }, + }) + const first = yield* Queue.take(childPrompts) + yield* Deferred.succeed(first.release, "Prose only, no submission.") + const nudge = yield* Queue.take(childPrompts) + yield* Deferred.succeed(nudge.release, "Still nothing.") + // No third prompt comes from spawn: the nudge is one per node + // EXECUTION. (The DagLoop's own NodeFailed handler may respawn + // the node as a fresh attempt afterwards — that retry policy is + // a separate pre-existing layer, not spawn's business.) + const node = yield* pollWithTimeout( + store.getNode(dagID, "report").pipe( + Effect.map((row) => row?.status === "failed" ? row : undefined), + ), + "node was not failed after the exhausted nudge", + ) + expect(node.errorReason).toContain("never successfully called") + }), + ), + ) + }) + }) }) From e8e3cdcc4bed59ddd4577ab1e209bee93f825a81 Mon Sep 17 00:00:00 2001 From: Lex Date: Tue, 25 Aug 2026 09:00:39 +0800 Subject: [PATCH 6/6] fix(recorder): assemble google key test fixture (#440) --- packages/http-recorder/test/record-replay.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/http-recorder/test/record-replay.test.ts b/packages/http-recorder/test/record-replay.test.ts index 93d7bbade..b7580a951 100644 --- a/packages/http-recorder/test/record-replay.test.ts +++ b/packages/http-recorder/test/record-replay.test.ts @@ -125,6 +125,10 @@ describe("http-recorder", () => { }) test("detects secret-looking values without returning the secret", () => { + // Assembled fake sample: keeps the AIza prefix + 22 redaction-pattern + // chars so the scanner hits, while never spelling a plausible real key + // (GitHub secret scanning matches literal AIza... blobs in the repo). + const googleKeySample = process.env.TEST_GOOGLE_KEY_SAMPLE ?? "AIza" + "TESTSAMPLE0".repeat(2) + "22" expect( HttpRecorderInternal.secretFindings({ version: 1, @@ -135,7 +139,7 @@ describe("http-recorder", () => { method: "POST", url: "https://example.test/path?key=sk-123456789012345678901234", headers: {}, - body: JSON.stringify({ nested: "AIzaSyDHibiBRvJZLsFnPYPoiTwxY4ztQ55yqCE" }), + body: JSON.stringify({ nested: googleKeySample }), }, response: { status: 200,