fix(maintainer): gate body notifications after redaction (#81993)

* feat(secret-scanning): advise delete-and-recreate for issue/PR body leaks * fix(maintainer): gate body notifications
2026-07-21 10:16:44 +00:00 · 2026-05-16 19:50:26 +08:00
parent 9b560b8a41
commit 5a14b1c5c5
3 changed files with 245 additions and 32 deletions
@@ -34,10 +34,10 @@ Supports single or multiple alerts. For multiple alerts, process in ascending or
 For each alert:

 1. **Identify** — `fetch-alert` + `fetch-content` to get metadata and body
-2. **Decide** — Agent reads the body file, identifies all secrets, produces redacted version
-3. **Redact** — `redact-body` for issue/PR body; skip for comments (delete directly)
+2. **Decide** — Agent reads the body file, identifies whether plaintext secrets remain, and produces a redacted version only when needed
+3. **Redact** — `redact-body-if-needed` for issue/PR body; skip for comments (delete directly)
 4. **Purge** — `delete-comment` + `recreate-comment` for comments; cannot purge body history
-5. **Notify** — `notify` posts the right template per location type
+5. **Notify** — `notify` posts the right template per location type, unless the current issue/PR body is already redacted
 6. **Resolve** — `resolve` closes the alert
 7. **Summary** — `summary` prints formatted results

@@ -81,11 +81,20 @@ The `fetch-content` output includes:
 The agent reads the body file from `fetch-content` output and:

 1. Identifies ALL secrets in the content (there may be more than the alert flagged)
-2. Replaces each secret with `[REDACTED <secret_type>]` — **no partial values, no prefix/suffix**
-3. Saves the redacted content to a new temp file
+2. Determines whether any plaintext credential remains in the current body
+3. Replaces each remaining secret with `[REDACTED <secret_type>]` — **no partial values, no prefix/suffix**
+4. Saves the redacted content to a new temp file

 This is the only step that requires semantic understanding. Everything else is mechanical.

+For `issue_body` and `pull_request_body`: if the current body has already been redacted by the author and no plaintext credential remains, **do not post a public notification comment**. Resolve the alert with a maintainer-only resolution comment such as:
+
+```bash
+node secret-scanning.mjs resolve <ALERT_NUMBER> revoked "Current issue/PR body is already redacted; no public notification posted."
+```
+
+This avoids creating a fresh public pointer to historical sensitive content.
+
 ## Step 3: Redact

 ### For comments (issue_comment / PR comments)
@@ -95,9 +104,11 @@ This is the only step that requires semantic understanding. Everything else is m
 ### For issue_body / pull_request_body

 ```bash
-node secret-scanning.mjs redact-body <issue|pr> <NUMBER> <redacted-body-file>
+node secret-scanning.mjs redact-body-if-needed <issue|pr> <NUMBER> <current-body-file> <redacted-body-file> <result-file>
 ```

+Use the `body_file` from `fetch-content` as `<current-body-file>`. The command writes `notify_required` to `<result-file>` and only PATCHes the body when the redacted file differs from the current body.
+
 ## Step 4: Purge Edit History

 ### Comments — Delete and Recreate
@@ -134,10 +145,12 @@ The recreated comment should follow this format:
 <redacted original content>
 ```

-### issue_body / pull_request_body — Cannot Purge
+### issue_body / pull_request_body — Cannot Purge Edit History

 Editing creates an edit history revision with the pre-edit plaintext. This cannot be cleared via API.

+Do not advise authors publicly to delete/recreate issues or close/reopen PRs. That can draw attention to historical content. Keep purge guidance maintainer-only.
+
 **Output to maintainer terminal only (never in public comments):**

 ```
@@ -155,12 +168,13 @@ Cannot clean. Notify author to delete branch or force-push (for unmerged PRs).
 ## Step 5: Notify

 ```bash
-node secret-scanning.mjs notify <TARGET> <AUTHOR> <LOCATION_TYPE> <SECRET_TYPES> [REPLY_TO_NODE_ID]
+node secret-scanning.mjs notify <TARGET> <AUTHOR> <LOCATION_TYPE> <SECRET_TYPES> [REPLY_TO_NODE_ID|BODY_REDACTION_RESULT_FILE]
 ```

 - For non-discussion types, `<TARGET>` is the issue/PR number.
 - For `discussion_comment`, `<TARGET>` is the `discussion_node_id` returned by `fetch-content`.
 - For reply-style `discussion_comment` locations, pass the optional `reply_to_node_id` from `fetch-content` so the notification stays in the same thread.
+- For `issue_body` and `pull_request_body`, pass the `<result-file>` from `redact-body-if-needed`. The script skips notification when `notify_required` is `false` and refuses body notifications without this file.

 Secret types are comma-separated: `"Discord Bot Token,Feishu App Secret"`

@@ -170,6 +184,8 @@ The script picks the right template:
 - **body types**: "your issue/PR description … redacted in place"
 - **commit**: "code you committed"

+For `issue_body` and `pull_request_body`, only notify when the current body still contained plaintext and maintainers redacted it. If the user already redacted the current body, skip this step and resolve silently.
+
 ## Step 6: Resolve

 ```bash
@@ -178,7 +194,7 @@ node secret-scanning.mjs resolve <ALERT_NUMBER>
 node secret-scanning.mjs resolve <ALERT_NUMBER> revoked "Custom comment"
 ```

-Resolution is `revoked` by default. As maintainers we cannot control whether users rotate — our responsibility is to redact + notify. The `revoked` means "this secret should be considered leaked", not "I confirmed it was revoked".
+Resolution is `revoked` by default. As maintainers we cannot control whether users rotate — our responsibility is to remove current plaintext exposure and notify only when public notification is useful. The `revoked` means "this secret should be considered leaked", not "I confirmed it was revoked".

 ## Step 7: Summary

@@ -7,6 +7,7 @@ import crypto from "node:crypto";
 import fs from "node:fs";
 import os from "node:os";
 import path from "node:path";
+import { pathToFileURL } from "node:url";

 const REPO = "openclaw/openclaw";
 const REPO_URL = `https://github.com/${REPO}`;
@@ -50,6 +51,34 @@ function ghGraphQL(query, options = {}) {
  return gh(["api", "graphql", "-f", `query=${query}`], options);
 }

+function isBodyLocationType(locationType) {
+  return locationType === "issue_body" || locationType === "pull_request_body";
+}
+
+export function decideBodyRedaction(currentBody, redactedBody) {
+  const bodyChanged = String(currentBody) !== String(redactedBody);
+  return {
+    body_changed: bodyChanged,
+    notify_required: bodyChanged,
+  };
+}
+
+export function loadBodyRedactionResult(locationType, resultFile) {
+  if (!isBodyLocationType(locationType)) {
+    return { notify_required: true };
+  }
+  if (!resultFile) {
+    fail("Body notifications require a redaction result file from redact-body-if-needed");
+  }
+  if (!fs.existsSync(resultFile)) fail(`File not found: ${resultFile}`);
+
+  const result = JSON.parse(fs.readFileSync(resultFile, "utf8"));
+  if (typeof result.notify_required !== "boolean") {
+    fail(`Invalid redaction result file: missing boolean notify_required in ${resultFile}`);
+  }
+  return result;
+}
+
 function failOnGraphQLFailure(result, message) {
  if (result?.gh_failed) {
    const details = (
@@ -470,6 +499,43 @@ function cmdRedactBody(kind, number, bodyFile) {
  console.log(JSON.stringify({ ok: true, kind, number: Number(number) }));
 }

+/**
+ * redact-body-if-needed <issue|pr> <number> <current-body-file> <redacted-body-file> <result-file>
+ * PATCH only when the agent-produced redacted body differs from the current body.
+ */
+function cmdRedactBodyIfNeeded(kind, number, currentBodyFile, redactedBodyFile, resultFile) {
+  if (!kind || !number || !currentBodyFile || !redactedBodyFile || !resultFile) {
+    fail(
+      "Usage: redact-body-if-needed <issue|pr> <number> <current-body-file> <redacted-body-file> <result-file>",
+    );
+  }
+  if (!fs.existsSync(currentBodyFile)) fail(`File not found: ${currentBodyFile}`);
+  if (!fs.existsSync(redactedBodyFile)) fail(`File not found: ${redactedBodyFile}`);
+
+  const currentBody = fs.readFileSync(currentBodyFile, "utf8");
+  const redactedBody = fs.readFileSync(redactedBodyFile, "utf8");
+  const decision = decideBodyRedaction(currentBody, redactedBody);
+  const result = {
+    ok: true,
+    kind,
+    number: Number(number),
+    ...decision,
+  };
+
+  if (decision.body_changed) {
+    const endpoint =
+      kind === "pr" ? `repos/${REPO}/pulls/${number}` : `repos/${REPO}/issues/${number}`;
+    gh(["api", endpoint, "-X", "PATCH", "-F", `body=@${redactedBodyFile}`]);
+    result.redacted = true;
+  } else {
+    result.redacted = false;
+    result.reason = "current_body_already_redacted";
+  }
+
+  fs.writeFileSync(resultFile, `${JSON.stringify(result, null, 2)}\n`, { mode: 0o600 });
+  console.log(JSON.stringify(result));
+}
+
 /**
 * delete-comment <comment-id>
 * Delete a comment (and all its edit history).
@@ -555,6 +621,17 @@ function cmdNotify(target, author, locationType, secretTypes, replyToNodeId) {

  const types = secretTypes.split(",").map((s) => s.trim());
  const typeList = types.map((t, i) => `${i + 1}. **${t}**`).join("\n");
+  const redactionResult = loadBodyRedactionResult(locationType, replyToNodeId);
+  if (isBodyLocationType(locationType) && !redactionResult.notify_required) {
+    console.log(
+      JSON.stringify({
+        ok: true,
+        skipped: true,
+        reason: "current_body_already_redacted",
+      }),
+    );
+    return;
+  }

  let locationDesc;
  let actionDesc;
@@ -758,12 +835,13 @@ function cmdSummary(jsonFile) {

 // ─── Dispatch ───────────────────────────────────────────────────────────────

-const [command, ...args] = process.argv.slice(2);
+const args = [];

-const commands = {
+export const commands = {
  "fetch-alert": () => cmdFetchAlert(args[0]),
  "fetch-content": () => cmdFetchContent(args[0]),
  "redact-body": () => cmdRedactBody(args[0], args[1], args[2]),
+  "redact-body-if-needed": () => cmdRedactBodyIfNeeded(args[0], args[1], args[2], args[3], args[4]),
  "delete-comment": () => cmdDeleteComment(args[0]),
  "delete-discussion-comment": () => cmdDeleteDiscussionComment(args[0]),
  "recreate-comment": () => cmdRecreateComment(args[0], args[1]),
@@ -774,26 +852,37 @@ const commands = {
  summary: () => cmdSummary(args[0]),
 };

-if (!command || !commands[command]) {
-  console.error(
-    [
-      "Usage: node secret-scanning.mjs <command> [args]",
-      "",
-      "Commands:",
-      "  fetch-alert <number>             Fetch alert metadata + locations",
-      "  fetch-content '<location-json>'   Fetch content for a location",
-      "  redact-body <issue|pr> <n> <file> PATCH body with redacted file",
-      "  delete-comment <comment-id>       Delete a comment",
-      "  delete-discussion-comment <node-id> Delete a discussion comment (GraphQL)",
-      "  recreate-comment <issue-n> <file> Create replacement comment",
-      "  recreate-discussion-comment <disc-node-id> <file> [reply-to-node-id] Create discussion comment (GraphQL)",
-      "  notify <target> <author> <type> <types> [reply-to-node-id] Post notification",
-      "  resolve <n> [resolution] [comment] Close alert",
-      "  list-open                          List open alerts",
-      "  summary <json-file>               Print formatted summary",
-    ].join("\n"),
-  );
-  process.exit(1);
+function main(argv = process.argv.slice(2)) {
+  const [command, ...commandArgs] = argv;
+  args.length = 0;
+  args.push(...commandArgs);
+
+  if (!command || !commands[command]) {
+    console.error(
+      [
+        "Usage: node secret-scanning.mjs <command> [args]",
+        "",
+        "Commands:",
+        "  fetch-alert <number>             Fetch alert metadata + locations",
+        "  fetch-content '<location-json>'   Fetch content for a location",
+        "  redact-body <issue|pr> <n> <file> PATCH body with redacted file",
+        "  redact-body-if-needed <issue|pr> <n> <current-file> <redacted-file> <result-file> PATCH body only if redaction changed it",
+        "  delete-comment <comment-id>       Delete a comment",
+        "  delete-discussion-comment <node-id> Delete a discussion comment (GraphQL)",
+        "  recreate-comment <issue-n> <file> Create replacement comment",
+        "  recreate-discussion-comment <disc-node-id> <file> [reply-to-node-id] Create discussion comment (GraphQL)",
+        "  notify <target> <author> <type> <types> [reply-to-node-id|body-result-file] Post notification",
+        "  resolve <n> [resolution] [comment] Close alert",
+        "  list-open                          List open alerts",
+        "  summary <json-file>               Print formatted summary",
+      ].join("\n"),
+    );
+    process.exit(1);
+  }
+
+  commands[command]();
 }

-commands[command]();
+if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
+  main();
+}
@@ -0,0 +1,108 @@
+import { execFileSync } from "node:child_process";
+import fs from "node:fs";
+import path from "node:path";
+import { describe, expect, it } from "vitest";
+import { createScriptTestHarness } from "./test-helpers.js";
+
+const scriptPath = ".agents/skills/openclaw-secret-scanning-maintainer/scripts/secret-scanning.mjs";
+const { createTempDir } = createScriptTestHarness();
+
+describe("secret scanning maintainer script", () => {
+  it("marks body alerts as not requiring notification when redaction is unchanged", () => {
+    const tempDir = createTempDir("openclaw-secret-scan-");
+    const currentBody = path.join(tempDir, "current.md");
+    const redactedBody = path.join(tempDir, "redacted.md");
+    const resultFile = path.join(tempDir, "redaction-result.json");
+    fs.writeFileSync(currentBody, "token: [REDACTED Discord Bot Token]\n");
+    fs.writeFileSync(redactedBody, "token: [REDACTED Discord Bot Token]\n");
+
+    const output = execFileSync(
+      process.execPath,
+      [scriptPath, "redact-body-if-needed", "issue", "123", currentBody, redactedBody, resultFile],
+      { encoding: "utf8" },
+    );
+
+    expect(JSON.parse(output)).toMatchObject({
+      body_changed: false,
+      notify_required: false,
+      reason: "current_body_already_redacted",
+      redacted: false,
+    });
+    expect(JSON.parse(fs.readFileSync(resultFile, "utf8"))).toMatchObject({
+      notify_required: false,
+    });
+  });
+
+  it("patches body alerts and requires notification when redaction changes the current body", () => {
+    const tempDir = createTempDir("openclaw-secret-scan-");
+    const binDir = path.join(tempDir, "bin");
+    const ghLog = path.join(tempDir, "gh.log");
+    fs.mkdirSync(binDir);
+    fs.writeFileSync(
+      path.join(binDir, "gh"),
+      `#!/usr/bin/env bash\nprintf '%s\\n' "$*" >> "${ghLog}"\nprintf '{}\\n'\n`,
+      { mode: 0o755 },
+    );
+
+    const currentBody = path.join(tempDir, "current.md");
+    const redactedBody = path.join(tempDir, "redacted.md");
+    const resultFile = path.join(tempDir, "redaction-result.json");
+    fs.writeFileSync(currentBody, "token: plaintext-secret\n");
+    fs.writeFileSync(redactedBody, "token: [REDACTED Discord Bot Token]\n");
+
+    const output = execFileSync(
+      process.execPath,
+      [scriptPath, "redact-body-if-needed", "issue", "123", currentBody, redactedBody, resultFile],
+      {
+        encoding: "utf8",
+        env: {
+          ...process.env,
+          PATH: `${binDir}${path.delimiter}${process.env.PATH ?? ""}`,
+        },
+      },
+    );
+
+    expect(JSON.parse(output)).toMatchObject({
+      body_changed: true,
+      notify_required: true,
+      redacted: true,
+    });
+    expect(fs.readFileSync(ghLog, "utf8")).toContain(
+      `api repos/openclaw/openclaw/issues/123 -X PATCH -F body=@${redactedBody}`,
+    );
+  });
+
+  it("skips body notification when the redaction result says the current body was already redacted", () => {
+    const tempDir = createTempDir("openclaw-secret-scan-");
+    const resultFile = path.join(tempDir, "redaction-result.json");
+    fs.writeFileSync(
+      resultFile,
+      JSON.stringify({
+        body_changed: false,
+        notify_required: false,
+      }),
+    );
+
+    const output = execFileSync(
+      process.execPath,
+      [scriptPath, "notify", "123", "contributor", "issue_body", "Discord Bot Token", resultFile],
+      { encoding: "utf8" },
+    );
+
+    expect(JSON.parse(output)).toStrictEqual({
+      ok: true,
+      reason: "current_body_already_redacted",
+      skipped: true,
+    });
+  });
+
+  it("requires body notifications to include a redaction result file", () => {
+    expect(() =>
+      execFileSync(
+        process.execPath,
+        [scriptPath, "notify", "123", "contributor", "issue_body", "Discord Bot Token"],
+        { encoding: "utf8", stdio: "pipe" },
+      ),
+    ).toThrow(/Body notifications require a redaction result file/);
+  });
+});