chore: promote dev to main (open-issues-batch + security fixes)

.specgit.yaml

@@ -1,8 +1,12 @@
 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
+pr: 444

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,

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",

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",

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/"))
+  )
+}

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",

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.`,
               })
             }
 

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")

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("<", "&lt;")
-    .replaceAll(">", "&gt;")
-    .replaceAll('"', "&quot;")
-    .replaceAll("'", "&#39;")
+  return escapeHtmlLib(value)
 }

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)
+  })
+})

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")
+          }),
+        ),
+      )
+    })
+  })
 })