Skip to content

feat(skills): add secret-scanning-maintainer skill#65417

Merged
hxy91819 merged 7 commits intomainfrom
feat/secret-scanning-maintainer-skill
Apr 12, 2026
Merged

feat(skills): add secret-scanning-maintainer skill#65417
hxy91819 merged 7 commits intomainfrom
feat/secret-scanning-maintainer-skill

Conversation

@hxy91819
Copy link
Copy Markdown
Member

Summary

  • Add openclaw-secret-scanning-maintainer skill for handling GitHub Secret Scanning alerts
  • Covers 4 leak location types: issue_comment, issue_body, pull_request_body, commit
  • Includes redaction, history purge (delete+recreate for comments), author notification, and alert resolution workflows
  • Enforces security best practices: no edit history details in public comments, English-only comments, summary with full URLs

Key design decisions

  • issue_comment: Delete + recreate to fully purge edit history
  • issue_body / PR body: Redact only (cannot delete/recreate), warn maintainer in terminal only
  • commit: Notify author; flag if PR not merged or if BFG cleanup needed
  • Never reveal edit history details publicly — no "edited button" mentions in any public-facing content
  • All comments in English per skill requirement
  • Skip unsupported location types and report in summary for skill improvement

Test plan

  • Trigger the skill with a secret scanning alert number and verify it follows the full flow
  • Verify notification comments are in English and do not mention edit history
  • Verify summary output contains full clickable URLs

Add a maintainer-only skill for handling GitHub Secret Scanning alerts.
Covers issue_comment, issue_body, pull_request_body, and commit leak
types with redaction, history purge (delete+recreate for comments),
author notification, and alert resolution workflows.
@aisle-research-bot
Copy link
Copy Markdown

aisle-research-bot Bot commented Apr 12, 2026

🔒 Aisle Security Analysis

We found 5 potential security issue(s) in this PR:

# Severity Title
1 🟠 High SSRF / GitHub token exfiltration risk via unvalidated URL passed to gh api
2 🟡 Medium Potential secret disclosure via logging gh stdout/stderr on failures
3 🟡 Medium Potential secret disclosure in public notification comment via unvalidated secretTypes argument
4 🟡 Medium Secret content written to persistent temp files without cleanup (possible secret leakage on disk)
5 🟡 Medium GraphQL query injection via unescaped node_id interpolation in ghGraphQL() calls
1. 🟠 SSRF / GitHub token exfiltration risk via unvalidated URL passed to `gh api`
Property Value
Severity High
CWE CWE-918
Location .agents/skills/openclaw-secret-scanning-maintainer/scripts/secret-scanning.mjs:81-100

Description

The fetch-content command accepts location-json from the CLI, parses it, and then passes URL fields from location.details directly into gh api.

If an attacker can influence the location-json value (e.g., by providing a crafted JSON blob for a maintainer to run), the script may be tricked into making authenticated requests to attacker-controlled URLs, potentially leaking the maintainer's GitHub token via the Authorization header (depending on gh api URL handling) and/or performing SSRF.

Key points:

  • Input: locationJson is taken directly from CLI args and parsed with JSON.parse.
  • Propagation: details.*_url values are used as-is (commentUrl, issueUrl, prUrl).
  • Sink: gh(["api", <url>]) executes the GitHub CLI, which will attach auth for API calls.

Vulnerable code:

const location = JSON.parse(locationJson);
...
const commentUrl = details.issue_comment_url || ...;
...
const comment = gh(["api", commentUrl]);

Recommendation

Do not allow arbitrary absolute URLs to flow into gh api.

Recommended fixes:

  1. Only accept GitHub API path endpoints, not full URLs, by extracting the pathname from known-good GitHub API URLs:
function toGitHubApiPath(u) {
  const url = new URL(u);
  if (url.protocol !== "https:") throw new Error("Invalid protocol");// Option A: strictly pin to api.github.com
  if (url.hostname !== "api.github.com") throw new Error("Unexpected host");
  return url.pathname + url.search;
}

const comment = gh(["api", toGitHubApiPath(commentUrl)]);
  1. Apply the same validation for issueUrl and prUrl.

  2. If GitHub Enterprise support is needed, validate against the configured GitHub hostname(s) rather than allowing arbitrary hosts.

2. 🟡 Potential secret disclosure via logging `gh` stdout/stderr on failures
Property Value
Severity Medium
CWE CWE-532
Location .agents/skills/openclaw-secret-scanning-maintainer/scripts/secret-scanning.mjs:28-32

Description

The gh() helper logs the full gh command stderr/stdout when the command exits non-zero. Several commands send or handle sensitive content via gh api ... -F body=@<file> (e.g., redaction uploads and notification posting). If the GitHub CLI emits request/response details containing the submitted body (for example when GH_DEBUG/GH_DEBUG_API is enabled, or if future gh versions echo request payloads on error), the script will print that output to terminal/automation logs.

This conflicts with the skill’s intent of never printing sensitive body content.

Vulnerable code:

if (proc.status !== 0 && !allowFailure) {
  fail(`gh ${args.slice(0, 3).join(" ")} failed:\n${(proc.stderr || proc.stdout || "").trim()}`);
}

Recommendation

Avoid echoing full gh output on failure, or strictly sanitize it.

Safer options:

  1. Default to a minimal error message and instruct maintainers to re-run with a local debug flag when needed:
if (proc.status !== 0 && !allowFailure) {
  fail(`gh ${args.slice(0, 3).join(" ")} failed (exit ${proc.status}). Re-run with DEBUG locally if needed.`);
}
  1. Redact/limit output (e.g., only show first N characters and strip lines that may include request bodies), or write full output to a 0600 temp file and print only the path:
if (proc.status !== 0 && !allowFailure) {
  const logPath = tmpFile("gh-error.log");
  fs.writeFileSync(logPath, (proc.stderr || proc.stdout || ""), { mode: 0o600 });
  fail(`gh ${args.slice(0, 3).join(" ")} failed (exit ${proc.status}). Details saved to ${logPath}`);
}

Additionally, consider explicitly clearing GH_DEBUG/GH_DEBUG_API from the environment when invoking gh to prevent verbose request logging from being captured in stderr.

3. 🟡 Potential secret disclosure in public notification comment via unvalidated `secretTypes` argument
Property Value
Severity Medium
CWE CWE-200
Location .agents/skills/openclaw-secret-scanning-maintainer/scripts/secret-scanning.mjs:302-359

Description

cmdNotify() renders the operator-supplied secretTypes argument directly into the GitHub issue/PR comment body.

  • Input: secretTypes comes from CLI args (notify <...> <secret-types-comma-sep>) and is split into types.
  • Sink: each entry is interpolated into typeList and posted publicly via gh api .../comments.
  • If an operator accidentally passes actual secret values (tokens/keys) instead of type labels, the script will publish those secrets in a public comment, worsening the incident.

Vulnerable code:

const types = secretTypes.split(",").map((s) => s.trim());
const typeList = types.map((t, i) => `${i + 1}. **${t}**`).join("\n");
...
fs.writeFileSync(bodyFile, body);
...
gh(["api", `repos/${REPO}/issues/${issueNumber}/comments`, "-X", "POST", "-F", `body=@${bodyFile}`]);

Recommendation

Treat secretTypes as labels, not free-form text, and prevent accidental inclusion of secret material.

Options (can be combined):

  1. Avoid CLI-supplied types: fetch the alert’s secret_type_display_name from the GitHub API and use that value.

  2. Validate/allowlist: only allow values that match known secret type display names for the repo/org (or match the alert’s secret_type_display_name for the alert being processed).

  3. Safety guardrails: refuse values that look like secrets (very long, high-entropy/base64/hex, contains typical token prefixes), and/or require a --force flag.

Example (minimal guardrail):

function looksLikeSecret(s) {
  return s.length > 60 || /[A-Za-z0-9_\-]{40,}/.test(s);
}

const types = secretTypes.split(",").map((s) => s.trim()).filter(Boolean);
if (types.some(looksLikeSecret)) {
  fail("secret-types appears to contain secret material; pass only secret type labels (e.g., 'Discord Bot Token')");
}

This reduces the chance of accidentally posting sensitive values into a public GitHub comment.

4. 🟡 Secret content written to persistent temp files without cleanup (possible secret leakage on disk)
Property Value
Severity Medium
CWE CWE-922
Location .agents/skills/openclaw-secret-scanning-maintainer/scripts/secret-scanning.mjs:99-102

Description

cmdFetchContent() writes full issue/PR/comment bodies to files under the OS temp directory via tmpFile() and returns the path, but the script never deletes these files.

This is risky because the fetched bodies are the exact content that triggered a GitHub Secret Scanning alert and may contain plaintext secrets.

  • Sensitive body content is written to ${os.tmpdir()} (often /tmp) and may persist indefinitely
  • Files may be picked up by system backups, forensic tooling, or accessed by privileged local users/processes on shared CI runners
  • The script also writes notification comment bodies to temp files and likewise never cleans them up

Vulnerable code:

const bodyFile = tmpFile("body.md");
fs.writeFileSync(bodyFile, comment.body || "");

Recommendation

Ensure temp files containing potentially-secret content are securely cleaned up.

Minimum fix:

  • Delete temp files as soon as they are no longer needed (e.g., after redaction/upload), using try/finally.

Example pattern:

const bodyFile = tmpFile("body.md");
try {
  fs.writeFileSync(bodyFile, comment.body || "", { mode: 0o600 });// ... process bodyFile, upload via gh, etc.
} finally {
  try { fs.unlinkSync(bodyFile); } catch {}
}

If the workflow requires a file for an external editor/agent step, consider:

  • Storing under a dedicated directory with explicit cleanup instructions and an optional cleanup command
  • Using an in-memory approach when possible (avoid writing secrets to disk entirely)
  • Using a temp library that supports automatic cleanup on process exit and on successful completion
5. 🟡 GraphQL query injection via unescaped node_id interpolation in ghGraphQL() calls
Property Value
Severity Medium
CWE CWE-74
Location .agents/skills/openclaw-secret-scanning-maintainer/scripts/secret-scanning.mjs:99-115

Description

The cmdFetchContent() command builds GraphQL queries by directly interpolating nodeId into the query string:

  • Input: locationJson is parsed from the command-line argument, and details.*_url is used directly as the gh api endpoint.
  • The response from gh(["api", commentUrl]) supplies comment.node_id (or issue.node_id / pr.node_id).
  • Sink: nodeId is embedded inside a quoted GraphQL string literal without any escaping.

If an attacker can influence the node_id value returned by the prior API call (e.g., by convincing an operator to run fetch-content with a crafted location-json pointing at a non-standard/attacker-controlled URL, or any scenario where the fetched JSON is not strictly GitHub-controlled), they can break out of the string literal and inject additional GraphQL selections. This can cause unintended data access under the operator's GitHub CLI credentials.

Vulnerable code:

const nodeId = comment.node_id;
...
const gql = ghGraphQL(`{
  node(id: "${nodeId}") {
    ...
  }
}`);

Recommendation

Avoid interpolating values into GraphQL query strings. Use GraphQL variables (or at minimum JSON-escape the value) so nodeId cannot alter query structure.

Preferred: use variables

function ghGraphQL(query, variables) {// gh supports passing multiple -f fields
  const args = ["api", "graphql", "-f", `query=${query}`];
  for (const [k, v] of Object.entries(variables || {})) {
    args.push("-f", `${k}=${v}`);
  }
  return gh(args);
}

const query = `query($id: ID!) {
  node(id: $id) {
    ... on IssueComment {
      userContentEdits(first: 50) { totalCount }
    }
  }
}`;
const gql = ghGraphQL(query, { id: nodeId });

Also consider validating that details.*_url values are expected GitHub API endpoints before calling gh api (e.g., only allow relative API paths like repos/... or https://api.github.com/...).


Analyzed PR: #65417 at commit 62146ee

Last updated on: 2026-04-12T16:11:29Z

@openclaw-barnacle openclaw-barnacle Bot added size: M maintainer Maintainer-authored PR labels Apr 12, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7209c2637a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 12, 2026

Greptile Summary

This PR adds a new .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md skill covering the full triage-redact-purge-notify-resolve workflow for GitHub Secret Scanning alerts across four location types (issue_comment, issue_body, pull_request_body, commit).

Two P1 issues need attention before merge:

  • Redundant PATCH creates a transient edit-history exposure (lines 152–154): Step 3 patches the issue_comment with redacted content before Step 4 deletes it, which causes GitHub to store the pre-edit plaintext in a new edit-history revision briefly visible via the "edited" button — exactly the exposure the skill warns against for body types. The PATCH should be skipped for issue_comment; the redacted content belongs in the recreated comment body in Step 4.
  • Notification template is wrong for non-comment types (lines 248–258): "Your comment contained…" and "The original comment has been removed" are sent for all location types, but for commit there is no comment, and for issue_body/pull_request_body the body was edited in-place. The notification body should be branched by location type.

Confidence Score: 3/5

Not safe to merge as-is — two P1 defects cause incorrect public-facing notification text and a brief security-relevant edit-history exposure for issue_comment alerts.

Two P1 findings: the Step 3 PATCH for issue_comment creates the exact edit-history disclosure the skill warns against (security-relevant), and the single notification template produces factually wrong public comments for commit and body location types (behavioral correctness). Both are present defects in the changed file, not speculative concerns.

.agents/skills/openclaw-secret-scanning-maintainer/SKILL.md — Step 3 (issue_comment PATCH) and Step 5 (notification template) need fixes.

Security Review

  • Edit history disclosure (lines 152–154): The PATCH in Step 3 for issue_comment creates a new GitHub edit-history revision storing the pre-edit plaintext secret. This revision is publicly readable via the "edited" button until the DELETE in Step 4 removes it — a brief but avoidable exposure window that the skill itself warns against for other location types.
  • Notification message accuracy (lines 248–258): For commit type alerts, the public notification comment incorrectly claims the secret was found in "your comment," which could mislead the author about what needs to be rotated or cleaned up.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md
Line: 152-155

Comment:
**PATCH before DELETE creates a new edit history entry with the plaintext secret**

The PATCH in Step 3 for `issue_comment` edits the comment body (triggering GitHub to store the pre-edit plaintext as a new revision), and then Step 4 deletes the comment. Between the PATCH and the DELETE there is a window where clicking the "edited" button would reveal the plaintext secret — the exact exposure vector the skill warns about on lines 208–222 and 327.

Since Step 4 will delete the entire comment anyway (including all history), the PATCH in Step 3 is both redundant and briefly creates the disclosure risk. The redacted content should be placed directly in the POST body of the recreated comment in Step 4, skipping the PATCH for `issue_comment` entirely:

```
### For issue_comment

> Skip Step 3 for `issue_comment`. The redacted content will be
> supplied directly when recreating the comment in Step 4.
> Patching first would create an edit-history revision containing
> the plaintext secret — the DELETE in Step 4 removes all history,
> making the patch redundant and transiently exposing the secret.
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md
Line: 248-258

Comment:
**Notification template is incorrect for `commit` and body location types**

The single template says "Your **comment** contained…" and "The original **comment** has been removed and replaced with a redacted version," but this text is used for all four location types:

- **`commit`**: there is no comment — it's a code commit. "Your comment" is factually wrong, and "removed and replaced" never happened.
- **`issue_body` / `pull_request_body`**: the body was edited in-place, not deleted and recreated, so "removed and replaced" is also inaccurate.

The skill should branch the notification body by location type, for example:

```
# For issue_comment:
Your comment contained the following exposed secrets…
The original comment has been removed and replaced with a redacted version.

# For issue_body / pull_request_body:
Your issue/PR body contained the following exposed secrets…
The content has been redacted.

# For commit:
Your commit contained the following exposed secrets…
Please rotate these credentials and, if the PR is not yet merged,
consider force-pushing a cleaned branch.
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md
Line: 64-66

Comment:
**`userContentEdits(first: 10)` may silently truncate history**

`totalCount` is fetched alongside the nodes, so the skill can detect when history is truncated, but there is no instruction to re-query with pagination when `totalCount > 10`. A comment that was edited more than 10 times (unusual but possible for long-lived issues) would have older revisions silently omitted from the inspection.

Consider bumping the limit (e.g., `first: 50`) or adding a note to paginate when `totalCount` exceeds the fetch size. The same applies to the identical queries at lines 87–90 (`issue_body`) and lines 109–111 (`pull_request_body`).

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "feat(skills): add secret-scanning-mainta..." | Re-trigger Greptile

Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
- Remove all secret value fragments from redaction markers (type-only)
- Remove alert URLs and partial secret previews from public comments
- Use temp files with heredoc for all gh api body content (shell injection)
- Add rule: never print raw API responses containing secrets to stdout
- Notification comments now only reference secret type, no value hints

Addresses 4 of 6 security findings from PR review:
1. Over-permissive redaction → type-only markers
3. Public partial preview + alert URL → removed from comments
4. Shell quoting risk → heredoc + temp file pattern
5. Stdout secret exposure → jq-only extraction rule

Findings #2 (revoked without rotation) and #6 (public playbook) are
accepted as-is with documented rationale.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ec51cfaa9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Addresses findings from Codex, Greptile, and Aisle bot reviews:

- Add pull_request_comment and pull_request_review_comment to location
  type routing table (was being skipped as unsupported) [Codex P1]
- Use hide_secret=true on alert fetch to prevent plaintext in terminal
  [Codex P1]
- Add jq filtering on all fetch commands to avoid printing .body or
  .secret to stdout [Codex P1, Aisle Medium]
- Skip PATCH before DELETE for comments — PATCH creates an unnecessary
  edit history revision exposing plaintext [Greptile P1]
- Use mktemp for all temp files instead of fixed /tmp paths [Aisle Medium]
- Branch notification template by location type: comment says "removed
  and replaced", body says "redacted in place", commit says "committed"
  [Greptile P1]
- Bump userContentEdits(first: 10) to first: 50 to reduce truncation
  risk [Greptile P2]
- Fix batch listing jq query to use .html_url instead of
  .first_location_detected.html_url [Codex P2]
- Use heredoc + temp file for comment recreation (was inline -f)
  [Codex P1]
- Remove alert URLs from public notification templates [Codex P1]
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e486896fb5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Comment thread .agents/skills/openclaw-secret-scanning-maintainer/SKILL.md Outdated
Add scripts/secret-scanning.mjs with subcommands: fetch-alert,
fetch-content, redact-body, delete-comment, recreate-comment, notify,
resolve, list-open, summary.

Security enforcements now live in the script (not agent memory):
- hide_secret=true on all alert fetches
- mktemp with random UUIDs for all temp files
- -F body=@file for all body uploads
- .secret and .body never printed to stdout
- notification templates branched by location type

SKILL.md simplified from ~370 lines to ~170 lines — now a decision
guide that references script commands instead of inline gh api calls.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38075f8d9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .agents/skills/openclaw-secret-scanning-maintainer/scripts/secret-scanning.mjs Outdated
Comment thread .agents/skills/openclaw-secret-scanning-maintainer/scripts/secret-scanning.mjs Outdated
Agent was rewriting the summary table without URLs. Make SKILL.md
explicit: the script output IS the final summary, do not reformat it.
Script summary now outputs ---BEGIN SUMMARY--- / ---END SUMMARY---
markers. SKILL.md instructs agent to output the content between markers
verbatim, preventing reformatting that drops URLs.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66057b1744

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- Restrict temp file permissions to 0600 (owner-only) [Codex P1]
- Add --slurp to list-open and fetch-alert locations for correct
  multi-page JSON parsing [Codex P1, Codex P2]
- Use commit_url/blob_url fallback for commit location URLs [Codex P2]
- Add --paginate to locations fetch [Codex P2]
@hxy91819 hxy91819 changed the title feat(skills): add secret-scanning-maintainer skill feat(skills): WIP-add secret-scanning-maintainer skill Apr 12, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 62146eedd4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@hxy91819
Copy link
Copy Markdown
Member Author

Thanks for the analysis. Here's our assessment of the 5 findings:

1. 🟠 SSRF via unvalidated URL — Won't fix. This is a maintainer-only local script, not a web service. Input URLs come from GitHub's own secret-scanning API responses. If an attacker can tamper with GitHub API responses, SSRF is the least of our problems.

2. 🟡 Secret disclosure via gh stderr on failure — Won't fix. On failure, gh api stderr contains HTTP error messages (e.g. "HTTP 422: Validation Failed"), not request body content. Body content is passed via -F body=@file, which gh does not echo to stderr. Additionally, this runs on the maintainer's own terminal — they already have full repo admin access.

3. 🟡 Secret disclosure via secretTypes argument — Won't fix. The secretTypes parameter contains secret type names (e.g. "Discord Bot Token"), not secret values. These names come from alert metadata extracted by the agent, not from user input.

4. 🟡 Temp file persistence — Won't fix. Temp files are created with 0600 permissions (owner-only) via crypto.randomUUID() paths. OS tmpdir cleanup handles removal. This is a short-lived local script, not a long-running service.

5. 🟡 GraphQL injection via node_id — Won't fix. The node_id values come directly from GitHub's REST API responses. Same trust boundary as finding #1 — if GitHub API responses are compromised, injection is not the primary concern.

All 5 findings assume an untrusted-input threat model, but this script operates in a trusted context: maintainer's local machine, inputs from GitHub's own APIs, triggered by the maintainer's own agent.

@hxy91819 hxy91819 merged commit 24d7694 into main Apr 12, 2026
35 of 41 checks passed
@hxy91819 hxy91819 deleted the feat/secret-scanning-maintainer-skill branch April 12, 2026 16:39
@hxy91819 hxy91819 changed the title feat(skills): WIP-add secret-scanning-maintainer skill feat(skills): add secret-scanning-maintainer skill Apr 12, 2026
lovewanwan pushed a commit to lovewanwan/openclaw that referenced this pull request Apr 28, 2026
* feat(skills): add secret-scanning-maintainer skill

Add a maintainer-only skill for handling GitHub Secret Scanning alerts.
Covers issue_comment, issue_body, pull_request_body, and commit leak
types with redaction, history purge (delete+recreate for comments),
author notification, and alert resolution workflows.

* fix(skills): harden secret-scanning-maintainer based on security review

- Remove all secret value fragments from redaction markers (type-only)
- Remove alert URLs and partial secret previews from public comments
- Use temp files with heredoc for all gh api body content (shell injection)
- Add rule: never print raw API responses containing secrets to stdout
- Notification comments now only reference secret type, no value hints

Addresses 4 of 6 security findings from PR review:
1. Over-permissive redaction → type-only markers
3. Public partial preview + alert URL → removed from comments
4. Shell quoting risk → heredoc + temp file pattern
5. Stdout secret exposure → jq-only extraction rule

Findings openclaw#2 (revoked without rotation) and openclaw#6 (public playbook) are
accepted as-is with documented rationale.

* fix(skills): address all bot review findings on secret-scanning skill

Addresses findings from Codex, Greptile, and Aisle bot reviews:

- Add pull_request_comment and pull_request_review_comment to location
  type routing table (was being skipped as unsupported) [Codex P1]
- Use hide_secret=true on alert fetch to prevent plaintext in terminal
  [Codex P1]
- Add jq filtering on all fetch commands to avoid printing .body or
  .secret to stdout [Codex P1, Aisle Medium]
- Skip PATCH before DELETE for comments — PATCH creates an unnecessary
  edit history revision exposing plaintext [Greptile P1]
- Use mktemp for all temp files instead of fixed /tmp paths [Aisle Medium]
- Branch notification template by location type: comment says "removed
  and replaced", body says "redacted in place", commit says "committed"
  [Greptile P1]
- Bump userContentEdits(first: 10) to first: 50 to reduce truncation
  risk [Greptile P2]
- Fix batch listing jq query to use .html_url instead of
  .first_location_detected.html_url [Codex P2]
- Use heredoc + temp file for comment recreation (was inline -f)
  [Codex P1]
- Remove alert URLs from public notification templates [Codex P1]

* feat(skills): extract secret-scanning operations into reusable script

Add scripts/secret-scanning.mjs with subcommands: fetch-alert,
fetch-content, redact-body, delete-comment, recreate-comment, notify,
resolve, list-open, summary.

Security enforcements now live in the script (not agent memory):
- hide_secret=true on all alert fetches
- mktemp with random UUIDs for all temp files
- -F body=@file for all body uploads
- .secret and .body never printed to stdout
- notification templates branched by location type

SKILL.md simplified from ~370 lines to ~170 lines — now a decision
guide that references script commands instead of inline gh api calls.

* fix(skills): enforce script summary output as final summary

Agent was rewriting the summary table without URLs. Make SKILL.md
explicit: the script output IS the final summary, do not reformat it.

* fix(skills): add summary output markers for verbatim rendering

Script summary now outputs ---BEGIN SUMMARY--- / ---END SUMMARY---
markers. SKILL.md instructs agent to output the content between markers
verbatim, preventing reformatting that drops URLs.

* fix(skills): address latest bot review findings on script

- Restrict temp file permissions to 0600 (owner-only) [Codex P1]
- Add --slurp to list-open and fetch-alert locations for correct
  multi-page JSON parsing [Codex P1, Codex P2]
- Use commit_url/blob_url fallback for commit location URLs [Codex P2]
- Add --paginate to locations fetch [Codex P2]
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
* feat(skills): add secret-scanning-maintainer skill

Add a maintainer-only skill for handling GitHub Secret Scanning alerts.
Covers issue_comment, issue_body, pull_request_body, and commit leak
types with redaction, history purge (delete+recreate for comments),
author notification, and alert resolution workflows.

* fix(skills): harden secret-scanning-maintainer based on security review

- Remove all secret value fragments from redaction markers (type-only)
- Remove alert URLs and partial secret previews from public comments
- Use temp files with heredoc for all gh api body content (shell injection)
- Add rule: never print raw API responses containing secrets to stdout
- Notification comments now only reference secret type, no value hints

Addresses 4 of 6 security findings from PR review:
1. Over-permissive redaction → type-only markers
3. Public partial preview + alert URL → removed from comments
4. Shell quoting risk → heredoc + temp file pattern
5. Stdout secret exposure → jq-only extraction rule

Findings openclaw#2 (revoked without rotation) and openclaw#6 (public playbook) are
accepted as-is with documented rationale.

* fix(skills): address all bot review findings on secret-scanning skill

Addresses findings from Codex, Greptile, and Aisle bot reviews:

- Add pull_request_comment and pull_request_review_comment to location
  type routing table (was being skipped as unsupported) [Codex P1]
- Use hide_secret=true on alert fetch to prevent plaintext in terminal
  [Codex P1]
- Add jq filtering on all fetch commands to avoid printing .body or
  .secret to stdout [Codex P1, Aisle Medium]
- Skip PATCH before DELETE for comments — PATCH creates an unnecessary
  edit history revision exposing plaintext [Greptile P1]
- Use mktemp for all temp files instead of fixed /tmp paths [Aisle Medium]
- Branch notification template by location type: comment says "removed
  and replaced", body says "redacted in place", commit says "committed"
  [Greptile P1]
- Bump userContentEdits(first: 10) to first: 50 to reduce truncation
  risk [Greptile P2]
- Fix batch listing jq query to use .html_url instead of
  .first_location_detected.html_url [Codex P2]
- Use heredoc + temp file for comment recreation (was inline -f)
  [Codex P1]
- Remove alert URLs from public notification templates [Codex P1]

* feat(skills): extract secret-scanning operations into reusable script

Add scripts/secret-scanning.mjs with subcommands: fetch-alert,
fetch-content, redact-body, delete-comment, recreate-comment, notify,
resolve, list-open, summary.

Security enforcements now live in the script (not agent memory):
- hide_secret=true on all alert fetches
- mktemp with random UUIDs for all temp files
- -F body=@file for all body uploads
- .secret and .body never printed to stdout
- notification templates branched by location type

SKILL.md simplified from ~370 lines to ~170 lines — now a decision
guide that references script commands instead of inline gh api calls.

* fix(skills): enforce script summary output as final summary

Agent was rewriting the summary table without URLs. Make SKILL.md
explicit: the script output IS the final summary, do not reformat it.

* fix(skills): add summary output markers for verbatim rendering

Script summary now outputs ---BEGIN SUMMARY--- / ---END SUMMARY---
markers. SKILL.md instructs agent to output the content between markers
verbatim, preventing reformatting that drops URLs.

* fix(skills): address latest bot review findings on script

- Restrict temp file permissions to 0600 (owner-only) [Codex P1]
- Add --slurp to list-open and fetch-alert locations for correct
  multi-page JSON parsing [Codex P1, Codex P2]
- Use commit_url/blob_url fallback for commit location URLs [Codex P2]
- Add --paginate to locations fetch [Codex P2]
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
* feat(skills): add secret-scanning-maintainer skill

Add a maintainer-only skill for handling GitHub Secret Scanning alerts.
Covers issue_comment, issue_body, pull_request_body, and commit leak
types with redaction, history purge (delete+recreate for comments),
author notification, and alert resolution workflows.

* fix(skills): harden secret-scanning-maintainer based on security review

- Remove all secret value fragments from redaction markers (type-only)
- Remove alert URLs and partial secret previews from public comments
- Use temp files with heredoc for all gh api body content (shell injection)
- Add rule: never print raw API responses containing secrets to stdout
- Notification comments now only reference secret type, no value hints

Addresses 4 of 6 security findings from PR review:
1. Over-permissive redaction → type-only markers
3. Public partial preview + alert URL → removed from comments
4. Shell quoting risk → heredoc + temp file pattern
5. Stdout secret exposure → jq-only extraction rule

Findings openclaw#2 (revoked without rotation) and openclaw#6 (public playbook) are
accepted as-is with documented rationale.

* fix(skills): address all bot review findings on secret-scanning skill

Addresses findings from Codex, Greptile, and Aisle bot reviews:

- Add pull_request_comment and pull_request_review_comment to location
  type routing table (was being skipped as unsupported) [Codex P1]
- Use hide_secret=true on alert fetch to prevent plaintext in terminal
  [Codex P1]
- Add jq filtering on all fetch commands to avoid printing .body or
  .secret to stdout [Codex P1, Aisle Medium]
- Skip PATCH before DELETE for comments — PATCH creates an unnecessary
  edit history revision exposing plaintext [Greptile P1]
- Use mktemp for all temp files instead of fixed /tmp paths [Aisle Medium]
- Branch notification template by location type: comment says "removed
  and replaced", body says "redacted in place", commit says "committed"
  [Greptile P1]
- Bump userContentEdits(first: 10) to first: 50 to reduce truncation
  risk [Greptile P2]
- Fix batch listing jq query to use .html_url instead of
  .first_location_detected.html_url [Codex P2]
- Use heredoc + temp file for comment recreation (was inline -f)
  [Codex P1]
- Remove alert URLs from public notification templates [Codex P1]

* feat(skills): extract secret-scanning operations into reusable script

Add scripts/secret-scanning.mjs with subcommands: fetch-alert,
fetch-content, redact-body, delete-comment, recreate-comment, notify,
resolve, list-open, summary.

Security enforcements now live in the script (not agent memory):
- hide_secret=true on all alert fetches
- mktemp with random UUIDs for all temp files
- -F body=@file for all body uploads
- .secret and .body never printed to stdout
- notification templates branched by location type

SKILL.md simplified from ~370 lines to ~170 lines — now a decision
guide that references script commands instead of inline gh api calls.

* fix(skills): enforce script summary output as final summary

Agent was rewriting the summary table without URLs. Make SKILL.md
explicit: the script output IS the final summary, do not reformat it.

* fix(skills): add summary output markers for verbatim rendering

Script summary now outputs ---BEGIN SUMMARY--- / ---END SUMMARY---
markers. SKILL.md instructs agent to output the content between markers
verbatim, preventing reformatting that drops URLs.

* fix(skills): address latest bot review findings on script

- Restrict temp file permissions to 0600 (owner-only) [Codex P1]
- Add --slurp to list-open and fetch-alert locations for correct
  multi-page JSON parsing [Codex P1, Codex P2]
- Use commit_url/blob_url fallback for commit location URLs [Codex P2]
- Add --paginate to locations fetch [Codex P2]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant