feat(skills): add secret-scanning-maintainer skill#65417
Conversation
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 Security AnalysisWe found 5 potential security issue(s) in this PR:
1. 🟠 SSRF / GitHub token exfiltration risk via unvalidated URL passed to `gh api`
DescriptionThe If an attacker can influence the Key points:
Vulnerable code: const location = JSON.parse(locationJson);
...
const commentUrl = details.issue_comment_url || ...;
...
const comment = gh(["api", commentUrl]);RecommendationDo not allow arbitrary absolute URLs to flow into Recommended fixes:
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)]);
2. 🟡 Potential secret disclosure via logging `gh` stdout/stderr on failures
DescriptionThe 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()}`);
}RecommendationAvoid echoing full Safer options:
if (proc.status !== 0 && !allowFailure) {
fail(`gh ${args.slice(0, 3).join(" ")} failed (exit ${proc.status}). Re-run with DEBUG locally if needed.`);
}
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 3. 🟡 Potential secret disclosure in public notification comment via unvalidated `secretTypes` argument
Description
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}`]);RecommendationTreat Options (can be combined):
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)
Description
This is risky because the fetched bodies are the exact content that triggered a GitHub Secret Scanning alert and may contain plaintext secrets.
Vulnerable code: const bodyFile = tmpFile("body.md");
fs.writeFileSync(bodyFile, comment.body || "");RecommendationEnsure temp files containing potentially-secret content are securely cleaned up. Minimum fix:
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:
5. 🟡 GraphQL query injection via unescaped node_id interpolation in ghGraphQL() calls
DescriptionThe
If an attacker can influence the Vulnerable code: const nodeId = comment.node_id;
...
const gql = ghGraphQL(`{
node(id: "${nodeId}") {
...
}
}`);RecommendationAvoid interpolating values into GraphQL query strings. Use GraphQL variables (or at minimum JSON-escape the value) so 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 Analyzed PR: #65417 at commit Last updated on: 2026-04-12T16:11:29Z |
There was a problem hiding this comment.
💡 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".
Greptile SummaryThis PR adds a new Two P1 issues need attention before merge:
Confidence Score: 3/5Not 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.
|
- 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.
There was a problem hiding this comment.
💡 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".
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]
There was a problem hiding this comment.
💡 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".
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.
There was a problem hiding this comment.
💡 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".
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.
There was a problem hiding this comment.
💡 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]
There was a problem hiding this comment.
💡 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".
|
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, 3. 🟡 Secret disclosure via secretTypes argument — Won't fix. The 4. 🟡 Temp file persistence — Won't fix. Temp files are created with 5. 🟡 GraphQL injection via node_id — Won't fix. The 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. |
* 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]
* 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]
* 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]
Summary
openclaw-secret-scanning-maintainerskill for handling GitHub Secret Scanning alertsissue_comment,issue_body,pull_request_body,commitKey design decisions
Test plan