fix: give the remediation search a route when the runtime exposes no search tool - #244
fix: give the remediation search a route when the runtime exposes no search tool#244topij wants to merge 1 commit into
Conversation
…search tool Found by running the step for real, on cs-toolkit, rather than by reading it. The step said to search the archives with "your runtime's own file-search facility, passing the subject as a parameter". A session with no such tool exposed — only MCP search for unrelated systems — has no route at all: the preferred one is unavailable and the stated fallback is to read #241 and treat building a shell command as its own problem. That is a description of a hazard, not an instruction. The withdrawal in #241 was right about the shell and wrong about its scope. It treated every subject as equally dangerous. In practice the risk is a property of WHERE THE SUBJECT CAME FROM: - a cron job name, a branch, a path, a scope — from your own repo or config, operator-controlled, and an ordinary shell search is fine - an issue or PR title — from a tracker that may accept issues from outside, which is the attacker-influenceable case #241 is actually about That distinction restores a usable route for the majority of candidates without reopening anything. The three recipes #241 records were all defeated by *title* inputs — an apostrophe, a quote, a delimiter line — and none of those arrive in a job name you wrote in your own automations.yaml. For the tracker case the step now also says to match on the candidate's ID rather than its title where possible. An id is the part whose shape you control, and it is what actually identifies the item: the live run that found this defect matched `CUS-1107` and `nightly-fixer`, never a free-text title. MEASURED, not assumed. Running the step on cs-toolkit exercised three different branches of it: `nightly-fixer` (dropped — a dated diagnosis in the archive plus a "DISABLED … monthly spend cap" note in its own status line), `weekly-slides` (candidate stands — the archive hits were about something else, which is the generic-subject case the step warns about), and `CUS-1107` (replaced by CUS-1110, the residual follow-up rule firing). Every subject that worked was an id or a config-derived name. VERIFIED: `make test` 721 passed; `python scripts/kit_doctor.py` 32 unchanged, 0 differ, 0 missing. Neither executes this prose; the run on cs-toolkit is the only thing that did, and it is what produced the finding.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Closing unmerged, under a bound declared before the second lens reportedBoth panel lenses returned HIGH/Major on the same paragraph, independently. 1 · The "operator-controlled ⇒ shell is fine" bucket is not safe, disproven on its own example. A lens ran The other lens reached the same class from the opposite end, splicing
My live-run evidence — 2 · It contradicted unedited text three lines below. The doc still says the subject is "candidate-derived text — a ticket title, a PR title, a job name". Both lenses flagged it, and both judged the result worse than the pre-PR state, which was one unambiguous rule. I edited the paragraph stating the new rule and not the one stating the old one. 3 · The fallback-within-a-fallback had no exit for a tracker subject with no parameter tool — it points at a tool the branch has already said doesn't exist. 4 · This repo's doctrine predicted it. Why closed rather than fixedThis is the fourth round on one paragraph across two PRs — three recipes withdrawn in #241, then this classifier. My obvious next move was a fifth version testing characters instead of origin, which is a tightened matcher too. I declared the bound before the adversarial lens reported: another Major on this paragraph ⇒ close unmerged and refile rather than write a fifth version. Two arrived. #230 set the precedent last session and the reason holds — a bound honoured only when convenient is not a bound. What survivesThe defect is real and stays open. A runtime with no search tool still has no route through this step. Refiled as #245 with every finding, the constraints any next attempt must meet, and the observation that fail-closed is probably cheapest: "no parameter tool ⇒ skip this candidate's archive check and say so in the briefing" needs no shell guidance at all, and is the only option that adds no rule for a later round to hole. Also recorded there: a lens mutated the classification in an isolated copy and Separately, CodeRabbit found a genuine fail-open in the same file on cs-toolkit's copy — a failed Process note: the first copy of this comment landed on the wrong repository — my working directory was cs-toolkit, so |
1 participant
Found by running the remediation check for real — on cs-toolkit, converting its
forked
/session-startadapter to a pointer at this workflow — rather than byreading it.
The defect
The step said to search the archives using "your runtime's own file-search
facility, passing the subject as a parameter."
A session with no such tool exposed — only MCP search for unrelated systems —
has no route at all. Its preferred route is unavailable and its stated fallback is
to read #241 and treat building a shell command as its own problem. That is a
description of a hazard, not an instruction, and an agent in that position is
stuck with a step it cannot perform.
What the withdrawal got wrong
#241 was right about the shell and wrong about its scope. It treated every
subject as equally dangerous. The risk is actually a property of where the subject
came from:
All three recipes #241 records were defeated by title inputs: an apostrophe, a
bare quote, a heredoc delimiter line. None of those arrives in a job name you wrote
into your own
automations.yaml.So the step now splits on provenance, which restores a usable route for most
candidates without reopening anything.
For the tracker case it also says to match on the candidate's id rather than
its title where possible. An id is the part whose shape you control, and it is what
actually identifies the item.
This is measured, not reasoned
The live run on cs-toolkit exercised three different branches of the step:
nightly-fixerfriction-log-archive.md:199, plus "DISABLED 2026-07-19 (monthly spend cap)" in its own status lineweekly-slidesCUS-1107CUS-1110— the residual-follow-up rule firingEvery subject that worked was an id or a config-derived name. Not one was a
free-text title. That is the evidence behind the split above, and it is also why
the id-first advice is in the step rather than in a comment.
Without this step,
nightly-fixerwould have rendered as a fresh 🔴 on a jobdeliberately disabled two weeks earlier.
Verification
make test→ 721 passed.python scripts/kit_doctor.py→ 32 unchanged, 0 differ,0 missing.
Neither executes this prose. The cs-toolkit run is the only thing that did, and
it is what produced the finding — which is the argument for that step of the
sequence existing at all.
#241 stays open: nothing here makes a shell recipe for tracker titles safe, and its
constraints are unchanged.