From 4b3692adab453c29edd876ac35618b548e37cad0 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Fri, 14 Aug 2026 04:05:01 +0000 Subject: [PATCH] feat(tcfeed): make the ask short enough to read, and true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "too much to process, obviously ai generated we are not doing this" Both halves of that were right. The issue and request together ran to 1,122 words for a two-file CI change, sent unsolicited to a stranger, and they were written by an AI. Arguing at length with someone who said it was too long only proves the point, so this sends less instead. before 1,122 words 7,042 characters after 330 words 2,443 characters and the 330 includes the dry-run framing and the diffstat; the text actually sent is about 110 words for the issue and 150 for the request. Gone: the CodeQL comparison, the permissions paragraph, the narrative about why pinning matters, and the demo block. The demo was the largest single piece and the one most likely to read as a threat — it reported 353 findings and 3 criticals against a repository nobody had asked us to scan. Its own docstring worried about becoming exactly that. Everything it claimed is still checkable in the diff, which is where it belongs. Two things added rather than cut: The scope is now stated. Every one of sixty-nine requests said the workflow "scans the diff". scanPath is ".", so it scans the whole checked-out repository. SAG asked for that correction and they were reading the same false sentence everyone else was. The AI assistance is disclosed. It is true, it is checkable, and a maintainer who works it out for themselves is entitled to be annoyed. Some will decline on the disclosure alone; that is a fair trade for not having it discovered later. Also says that an install or scan failure fails the job, which the old body left to be inferred from "report-only". Co-Authored-By: Claude Opus 5 --- bin/tcfeed.ts | 232 +++++++------------------------------------------- 1 file changed, 32 insertions(+), 200 deletions(-) diff --git a/bin/tcfeed.ts b/bin/tcfeed.ts index 2b0c7bbf..05e6d130 100755 --- a/bin/tcfeed.ts +++ b/bin/tcfeed.ts @@ -1015,39 +1015,6 @@ async function prTarget( const PR_TITLE = 'ci: scan pull requests for credentials and injection with ThreatCrush'; -/** - * The CodeQL answer, because "we already get this from GitHub" is the reason - * maintainers actually give, and saying nothing about it reads as not having - * an answer. - * - * It says *different*, never *better*, and the restraint is deliberate. It is - * not better at what CodeQL does: CodeQL is semantic dataflow analysis and - * most of this is pattern matching, which our own output admits every time it - * prints `confidence: pattern` next to a finding. Told "we'll likely try - * CodeQL first" by somebody who has read both, an overclaim here is checkable - * on the spot and loses the rest of the paragraph with it. - * - * So: two differences that are true, dated, and verifiable by the reader - * without taking our word for anything, and then the offer to be closed. - */ -const alongsideCodeql = [ - '**This is not a CodeQL replacement, and it is worth saying where it differs.**', - 'CodeQL does semantic dataflow analysis and is better at it than this is — a', - 'repository already running it is not missing much by closing this. Two gaps it', - 'does fill:', - '', - '- Code scanning and secret scanning are free on public repositories, but need', - ' paid GitHub Code Security / Secret Protection on private ones. This is MIT and', - ' free on both, so the same gate can run across a mixed set of repositories.', - '- CodeQL analyses a fixed set of languages, and among compiled ones it analyses', - " only the language with the most source files unless it's explicitly configured", - ' otherwise. In a polyglot repository the rest goes unscanned by default; this', - ' reads every file it is pointed at.', - '', - 'It is additive and report-only, so running both costs a few CI minutes and', - 'changes nothing else.', -]; - /** * Written to be easy to say no to. It quotes no findings and asserts nothing * about the code — a pull request that opens with "your repo has 30 high @@ -1056,60 +1023,31 @@ const alongsideCodeql = [ * workflow, discloses who wrote the workflow, and says it will not be sent * twice. Everything past that is the maintainer's call. */ -const prBody = (spec: string, issue: string, ran: string): string => +const prBody = (spec: string, issue: string): string => [ - 'Adds a pull-request workflow that scans the diff for hardcoded credentials,', - 'injection, SSRF and unsafe deserialisation. Results go to the Security tab as', - 'SARIF and to a comment on the pull request.', - ...(ran ? [ran] : []), + 'Adds one workflow. On each pull request it scans the checked-out repository for', + 'hardcoded credentials, injection, SSRF and unsafe deserialisation, and writes', + 'findings to the Security tab and a comment.', '', - // The diff exists so the question in the issue can be answered by reading - // it rather than imagining it. Which of the two gets closed is the - // maintainer's choice, and saying so costs nothing. - ...(issue - ? [ - 'Opened alongside the question in', - `${issue}, which is the place to say no or ask for`, - 'changes. This is only the diff, so it is there to read rather than imagine —', - 'closing either one is a fine answer.', - '', - ] - : []), - ...alongsideCodeql, + '- `.github/workflows/threatcrush-scan.yml`', + '- `.github/scripts/threatcrush-to-sarif.py` — SARIF shim for older CLI versions', '', - "**It is report-only.** `failOn` is empty, so it annotates and never fails a build.", - 'A repository with pre-existing findings should get a report on its first install,', - 'not a blocked pull request — a gate that fires on everything gets switched off', - 'within a day. Tighten it to `critical,high` in the workflow once any backlog is', - 'triaged.', + '**Report-only.** `failOn` is empty, so findings never fail the build. An install', + 'or scan failure does fail the job: a scanner that reports clean when it did not', + 'run is worse than no scanner.', '', - '- `.github/workflows/threatcrush-scan.yml` — the workflow', - '- `.github/scripts/threatcrush-to-sarif.py` — a compatibility shim for CLI versions', - ' older than native SARIF output; unused once the installed CLI can emit it itself', + // Said plainly because it was said wrongly for sixty-nine requests. scanPath + // is ".", and "scans the diff" was in every one of them. + '**Scope:** it scans the whole checked-out repository, not only the diff.', '', - 'Permissions are least-privilege (`contents: read`, `pull-requests: write`,', - '`security-events: write`). It runs on `pull_request`, not `pull_request_target`,', - 'so contributor code never executes with your secrets in scope. The SARIF upload', - 'is `continue-on-error` and degrades quietly where code scanning is unavailable.', + `**Supply chain.** Pinned to \`${spec}\`; the tarball is hashed and checked against`, + 'a value in the workflow before install (`npm view` it yourself), installed with', + '`--ignore-scripts`, actions pinned to commit SHAs, and it runs on `pull_request`', + 'rather than `pull_request_target`.', + ...(issue ? ['', `Asked first in ${issue}.`] : []), '', - `The CLI is pinned to \`${spec}\`, and the workflow does not install it on the`, - "registry's word. It downloads that tarball, hashes it, checks the SHA-512", - 'against a value committed in the workflow file, and refuses to install on a', - 'mismatch. A pinned version says which release to fetch; it does not say the', - 'bytes are the ones that release was published with, and the party answering the', - 'first question is the party serving the bytes. You can check the pinned hash', - `against the registry yourself with \`npm view ${spec} dist.integrity\`.`, - '', - 'It installs with `--ignore-scripts`, and checkout runs with', - "`persist-credentials: false`. A scanner that installs a floating version, runs", - "its dependencies' lifecycle scripts and leaves a token in `.git/config` is", - 'asking you to trust more than it is worth, and none of that is needed to read a', - 'diff. Bump the pin and its hash together whenever you like — nothing here', - 'updates itself.', - '', - 'Disclosure: I maintain [ThreatCrush](https://github.com/profullstack/threatcrush).', - 'It is free and MIT, and the workflow installs it from npm — nothing here phones', - 'home. If this is not something you want, closing it is the right answer, and I', + 'Disclosure: I maintain [ThreatCrush](https://github.com/profullstack/threatcrush);', + 'MIT and free. Written with AI assistance. Closing this is a fine answer and I', 'will not send another.', ].join('\n'); @@ -1193,93 +1131,6 @@ interface ScanJson { findings: { severity: string; confidence: string; ruleId: string }[]; } -/** - * Run the scanner over the tree we just cloned, and describe what it did. - * - * This exists because of the single most damning number in the whole - * experiment: of 24 open requests, **none** had ever run a scan. Nineteen sat - * at "waiting for the maintainer to approve the run" and five had no runs at - * all, because GitHub withholds workflow runs from first-time contributors. - * So every maintainer who looked saw a pending or red check and no output - * whatsoever. The offer was "here is a scanner for your pull requests" and the - * demonstration had never once executed. That explains nine straight declines - * far better than any wording did. - * - * What it must not become is the thing prBody() has always refused to be. - * Quoting findings at somebody as if they were defects is the fastest way to - * be ignored, and this project's own sample says why: every such claim in it - * was false. So this reports *what the run did* — files, time, counts, and the - * confidence tier the CLI itself assigns — and states plainly that none of it - * is a claim. `confidence: pattern` means a regex matched. That is a fact - * about the scanner, not about their code, and it is the honest half of what a - * maintainer wanted to see. - * - * Failure is silence. A scan that breaks, hangs or takes too long omits the - * section rather than blocking the request or, worse, guessing at a number. - */ -async function demo(dir: string, spec: string): Promise { - const seconds = num('TCFEED_DEMO_TIMEOUT', 120); - if (seconds <= 0) return ''; - - const began = Date.now(); - const said = await run('npx', ['--yes', spec, 'scan', dir, '--format', 'json'], { - maxBuffer: 64 * 1024 * 1024, - timeout: seconds * 1000, - }) - // Exit 1 is "findings at or above --fail-on", which is a result and not a - // failure. The JSON is on stdout either way; only an empty stdout is a - // genuine miss. - .then(({ stdout }) => stdout) - .catch((error: { stdout?: string }) => error.stdout ?? ''); - - let scan: ScanJson; - try { - scan = JSON.parse(said) as ScanJson; - if (typeof scan.filesScanned !== 'number' || !Array.isArray(scan.findings)) return ''; - } catch { - return ''; - } - - const took = ((Date.now() - began) / 1000).toFixed(1); - const order = ['critical', 'high', 'medium', 'low', 'info']; - const counted = order - .filter((name) => (scan.summary?.[name] ?? 0) > 0) - .map((name) => `${scan.summary[name]} ${name}`) - .join(', '); - - const evidence = scan.findings.filter((one) => one.confidence === 'evidence').length; - const pattern = scan.findings.length - evidence; - - return [ - '', - '### What it does on this repository', - '', - '```', - `${spec} scan .`, - `${scan.filesScanned} files in ${took}s — ${scan.findings.length} finding(s)` + - (counted ? `: ${counted}` : ''), - ...(scan.findings.length > 0 - ? [`confidence: ${evidence} evidence, ${pattern} pattern`] - : []), - '```', - '', - ...(scan.findings.length === 0 - ? ['Nothing flagged. That is the whole report — the comment on a pull request'] - : [ - '**None of that is a claim about your code, and I have not verified any of it.**', - '`confidence: pattern` means a regex matched and nothing more; expect false', - 'positives in that tier. It is here because the check on this pull request may', - ]), - ...(scan.findings.length === 0 - ? ['would say the same.'] - : [ - 'never run at all — GitHub withholds workflow runs from first-time contributors,', - 'and across 24 open requests elsewhere not one has been approved. Rather than ask', - 'you to approve a run to find out what it produces, that is what it produces.', - ]), - ].join('\n'); -} - const ISSUE_TITLE = 'Would you take a pull-request security scan workflow?'; /** @@ -1299,36 +1150,21 @@ const ISSUE_TITLE = 'Would you take a pull-request security scan workflow?'; */ const issueBody = (spec: string): string => [ - 'Hello — would a pull-request security scan be useful here, or is this', - 'already covered?', + 'Would a pull-request security scan be useful here, or is this already covered?', '', - 'The offer is one workflow that scans each pull request diff for hardcoded', - 'credentials, injection, SSRF and unsafe deserialisation, and writes results to', - 'the Security tab as SARIF plus a comment on the pull request. It is report-only', - "(`failOn` empty), so it annotates and never fails a build — a first install on a", - 'repository with a backlog should produce a report, not a blocked pull request.', + 'One workflow. On each pull request it scans the checked-out repository for', + 'hardcoded credentials, injection, SSRF and unsafe deserialisation, and writes', + 'findings to the Security tab. Report-only — findings never fail the build.', '', - ...alongsideCodeql, + `Two files under \`.github/\`, a pinned \`${spec}\` whose tarball is hashed before`, + 'install, and `pull_request` rather than `pull_request_target`.', '', - 'How it is wired, since this is the part worth objecting to:', + 'A pull request is open alongside this with the diff, if reading it is easier', + 'than discussing it.', '', - '- runs on `pull_request`, not `pull_request_target`, so contributor code never', - ' executes with your secrets in scope', - `- installs a pinned \`${spec}\` with \`--ignore-scripts\``, - '- `contents: read`, `pull-requests: write`, `security-events: write`, and', - ' `persist-credentials: false` on checkout', - '- two files, both under `.github/`; nothing else in the tree is touched', - '', - 'Disclosure: I maintain [ThreatCrush](https://github.com/profullstack/threatcrush).', - 'It is free and MIT, and the workflow installs it from npm — nothing here phones', - 'home. I am opening a pull request alongside this so the diff is there to read if', - 'you want it, and it can be closed and this discussed instead. That pull request', - 'also carries the output of an actual run against this repository, so the thing', - 'being offered can be judged without enabling anything: GitHub withholds workflow', - 'runs from first-time contributors, so the check on it may never run by itself.', - '', - 'If this is not something you want, saying so is the right answer and I will not', - 'ask again.', + 'Disclosure: I maintain [ThreatCrush](https://github.com/profullstack/threatcrush);', + 'MIT and free. Written with AI assistance. Closing this is a fine answer and I', + 'will not ask again.', ].join('\n'); /** @@ -1409,10 +1245,6 @@ async function openPr( if (stdout.trim()) return `already scans with threatcrush (${stdout.trim().split('\n')[0]})`; } - // Scanned before our own files are written, so the report describes their - // repository rather than the workflow this is about to add to it. - const ran = await demo(src, spec); - await git(['checkout', '--quiet', '-b', PR_BRANCH]); for (const file of files) { const full = path.join(src, file.destination); @@ -1435,7 +1267,7 @@ async function openPr( const { stdout } = await git(['show', '--stat', '--oneline', 'HEAD']); console.log(`\n--- ${repo} (dry run, nothing pushed) — base ${base}`); console.log(stdout.trimEnd()); - console.log(`\n${PR_TITLE}\n\n${prBody(spec, issue, ran)}`); + console.log(`\n${PR_TITLE}\n\n${prBody(spec, issue)}`); return 'dry run'; } @@ -1528,7 +1360,7 @@ async function openPr( '--title', PR_TITLE, '--body', - prBody(spec, issue, ran), + prBody(spec, issue), ]); } finally { fs.rmSync(tmp, { recursive: true, force: true });