Skip to content

tcfeed: 33 requests out, 7 answered, none accepted — fix the workflow, ask first, answer the CodeQL question #120

Description

@ralyodio

What happened

The first batch of tcfeed pr requests has now been answered. Six went out; five are closed and none merged.

Repo Outcome What the maintainer said
capstone-engine/capstone#3033 closed "Please open an issue about this first for discussion."
cloudflare/workers-sdk#15168 closed "At this time we're not intending to integrate with ThreatCrush."
miroslavpejic85/mirotalk#343 closed "I really appreciate it, but it's not planned for now."
kane50613/takumi#1232 closed no human comment; CodeRabbit review only
chrisleekr/binance-trading-bot#732 closed no human comment; CodeRabbit scored it 🟡 Moderate
NeptuneHub/AudioMuse-AI#856 open, checks red SonarCloud quality gate failed

Elsewhere a maintainer closed one with: "I'll look into security scanning options in the future, likely GitHub's built-in CodeQL/secret scanning first. Appreciate the transparency on the disclosure."

Three separate signals are worth separating, because only one of them is about the pitch.

1. The workflow we ship fails security review

This is the one to fix first, and it is not a matter of persuasion.

SonarCloud's gate on AudioMuse-AI#856 fails our own file with a C security rating on new code. Two MAJOR vulnerabilities, both on the install line:

githubactions:S6505  Omitting "--ignore-scripts" allows lifecycle scripts to run
                     during package installation.
githubactions:S8543  Using dependencies without locking resolved versions is
                     security-sensitive.

CodeRabbit reached the same conclusion independently on binance-trading-bot#732 — that the PR "gives an unpinned scanner access to a write-scoped job and retained checkout credentials."

Both are correct. The job holds pull-requests: write and security-events: write, and it was installing a floating @latest with lifecycle scripts enabled. Any publish anywhere in the CLI's dependency tree could run arbitrary code inside a write-scoped job on a stranger's runner, with a checkout credential still sitting in .git/config.

A security tool whose own workflow trips a security gate has no argument left to make. Fixed in profullstack/sh1pt: install with --ignore-scripts, render an exact version instead of @latest, and persist-credentials: false on checkout. Verified that scan still runs correctly from an --ignore-scripts install.

2. Ask before sending — capstone asked for it explicitly

Rot127's "please open an issue about this first" is the standard courtesy for unsolicited CI changes, and it costs us nothing. A PR arrives as a decision already made and a diff to review; an issue arrives as a question. Two of the closes ("not planned for now", "not intending to integrate") were project-direction answers that an issue would have surfaced without anyone reviewing a diff.

Proposal: tcfeed pr opens an issue by default and only opens a pull request where the maintainer asks for one. --pr keeps the current behaviour for repos that prefer patches.

3. Say how this differs from CodeQL

The CodeQL objection is real and deserves an answer in the body, because "we already get this from GitHub" is true for a lot of repos and we currently say nothing about it.

The answer should be how it differs, not that it is better. It is not better at what CodeQL does — CodeQL is semantic dataflow analysis and ours is largely pattern matching, which our own output admits (confidence: pattern vs confidence: evidence). Overclaiming to a maintainer who already named CodeQL is how a PR gets closed faster.

What is true and checkable:

  • Code scanning and secret scanning are free on public repos, but need paid GitHub Code Security / Secret Protection on private ones. This is MIT and free on both.
  • CodeQL covers a fixed language set, and for compiled languages it analyzes only the language with the most source files unless configured otherwise — so a polyglot repo is partly unscanned by default.
  • It is report-only and additive; it does not replace anything.

That is a paragraph, not a sales page, and it should end the same way the body already does: closing it is the right answer.

What this does not fix

Three of six closes were "we don't want a third-party integration", and no wording changes that. If the next batch also goes 0-for-N with the issue-first flow, the honest read is that unsolicited CI installs are not a channel that works for large repos, and the tool should aim at repos that ask.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions