Repository navigation
Actions collector: refuse rather than degrade, and keep inputs out of the shell (ADR-0033) - #23
Merged
Merged
Conversation
…t of the shell
The first thing in this repository that spends a person's money on their behalf,
unattended, from a button. collect.yml implements ADR-0031 against ADR-0032's contract.
Separate from live-collection.yml on purpose. That one asks "does the adapter still
work"; this one asks "collect this, for a person, now". One file doing both would give
two audiences one approval list.
THE SPEND GATE IS THE ENVIRONMENT, AND THE ENVIRONMENT IS CHECKED FOR EXISTENCE
`environment: research-collection` is the only enforcement in the file that does not
depend on a step behaving - GitHub evaluates required reviewers before the first step
runs. It also has a hole the corpus names: a workflow referencing an environment that
does NOT exist does not fail, it CREATES one with no protection rules and no secrets
(E-06). A typo in the name would remove the approval gate and nothing would say so.
So the environment must prove it exists before anything is spent: an environment variable
an auto-created environment cannot have. The honest limit is written into the file rather
than implied - GitHub exposes no scope information to a job, so the marker proves the
environment was configured and CANNOT prove a repository-level value did not serve the
secret. Defeating it means deliberately setting a repository variable of that name.
A MISSING CREDENTIAL REFUSES; IT NEVER DEGRADES
No http-keyless fallback, and a test asserts the string does not appear. Falling back
would return a measurably worse corpus under the same artifact name, and the person who
asked for research would have no way to tell.
NO DISPATCH INPUT REACHES A SHELL - AND ONE ALREADY DID
`${{ inputs.x }}` inside a `run:` block is textual substitution performed BEFORE the shell
parses the script, so a topic containing a backtick or $(...) executes on the runner.
Inputs now arrive through `env:` and then through an argv ARRAY.
The test enforcing this scans EVERY workflow, and it immediately found that
live-collection.yml had been interpolating four inputs into shell bodies since it was
written - including max_pages, which is free text. Dispatching needs write access, which
bounds the severity and does not make it correct. Fixed in this commit.
A DEFECT THIS SURFACED OUTSIDE ITSELF
Feeding the collector a deliberately hostile topic found a bug in lib/scaffold.mjs that
has nothing to do with workflows. template/research/plan.json holds `"topic": "{{TOPIC}}"`
and the substitution is textual, so a topic containing a double quote wrote a plan.json
that does not parse - after which research.mjs reads an empty plan, collects nothing, and
reports a corpus problem about a file the operator never edited.
Any operator typing `Why "agentic" search costs more` hit this. Tokens are now JSON-escaped
for .json templates and left verbatim for markdown, because escaping everywhere would put
\" into prose. Two regression tests, both confirmed RED against the unfixed module and
green again after - the fix was reverted on purpose to watch them fail first.
THE WORKFLOW CANNOT ASSERT AUTHORIZATION
It packages with artifact.mjs create, which derives every authorization field from the
corpus and refuses --build-authorized outright. A freshly collected corpus is
HUMAN_REVIEW_REQUIRED with buildAuthorized false, and the ZIP says so at its top.
Names disclose nothing: neither run-name nor the artifact name carries the topic, because
E-08 measured run and artifact listings as readable by an anonymous caller on a public
repository. The workflow also says plainly what that does NOT buy - dispatch inputs are
visible on the run page, so this reduces incidental exposure rather than making a public
run private.
Verified without spending a credit: the scaffold, plan, map and packaging steps were run
locally against a topic containing backticks, $(...), an ampersand and quotes. The topic
survived byte-identical into the manifest, appeared nowhere in the filename, and nothing
executed. 25 workflow tests, offline.
Still unproven, and now the only gap: the runner-side `npm install -g firecrawl@<version>`
path has never executed. Closing it needs the environment, its secret, and one real
dispatch - all of which are the repository owner's to create.
753 passed, 0 failed. Repository preflight PASS. No gate override used.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 5. The Actions collector, implementing ADR-0031 against ADR-0032's contract. Based on
main; both prerequisites are merged.Scope, item by item
windows-latestruns-oninput,shell: bashpinned so one script serves bothFIRECRAWL_API_KEYenvironment: research-collection, secret required non-emptyresearch.mjs$RUNNER_TEMP, withgit status --porcelainchecked afterartifact.mjs createif-no-files-found: error, validated before uploadX-GitHub-Api-Version: 2026-03-10source.apiVersionworkflow_run_idgithub.run_id→source.workflowRunIdworkflow_dispatchonly, assertedThe environment is checked for existence
E-06, verbatim: a workflow referencing an environment that does not exist doesn't fail —
GitHub creates one, with no protection rules and no secrets. A typo in the name would
remove the approval gate and nothing would say so.
So the environment must prove it exists: an environment variable
(
RESEARCH_KIT_COLLECTION_ENV) that an auto-created environment cannot have.The honest limit, written into the workflow rather than implied. GitHub exposes no
scope information to a job —
secrets.Xresolves environment-first and falls back, and nosyntax or API available to the job distinguishes them. The marker proves the environment
was configured; it cannot prove a repository-level value didn't serve the secret.
Defeating it requires deliberately setting a repository variable of that name. The
enforcement that matters stays where GitHub actually enforces it: the protection rules.
I'd rather state that than claim a guarantee the platform doesn't offer.
A live defect this found in already-merged code
The injection test scans every workflow, and
live-collection.ymlhad beeninterpolating four dispatch inputs straight into shell bodies since it was written:
max_pagesis free text, so'; curl … | sh; 'would have executed on the runner.Dispatching needs write access, which bounds severity — it doesn't make it correct. Fixed
here: four
env:blocks, zero interpolations left in any shell body.A defect outside workflows entirely
Feeding the collector a hostile topic found a
lib/scaffold.mjsbug:template/research/plan.jsonholds"topic": "{{TOPIC}}"and substitution is textual, soa topic with a double quote wrote a plan.json that does not parse.
research.mjsthenreads an empty plan, collects nothing, and blames a file the operator never edited.
Any operator typing
Why "agentic" search costs morehit this. Tokens are now JSON-escapedfor
.jsonand verbatim for markdown. Two regression tests — both confirmed red againstthe unfixed module first, then green after restoring it.
Verified without spending a credit
Scaffold → plan → map → package → validate, run locally against a topic containing
backticks,
$(…),&and quotes:PASS/buildAuthorized: false, stateCOLLECTION_FAILED(correct — the paid step was skipped)753 passed, 0 failed. Repository preflight PASS. No gate override.
Setup the owner must do, and the gap that remains
Settings → Environments →
research-collection: required reviewers, theFIRECRAWL_API_KEYsecret, andRESEARCH_KIT_COLLECTION_ENV=research-collectionas avariable. Deliberately not automated — adding a credential location is yours.
Still unproven, and now the only gap: the runner-side
npm install -g firecrawl@<version>path has never executed. Closing it needs thatenvironment and one real dispatch.
🤖 Generated with Claude Code