The provider skeleton ships an allowlist, and suite E covers it - #105
Merged
Merged
Conversation
1.13.0 gave every bank entry an allowlist so that installing one no longer produces a lab that cannot use it. The skeleton did not get one, so an entry scaffolded from it landed in exactly the state that release was about — and the person hitting it is the least equipped to diagnose it: someone writing their first provider, whose broker file and addon are both new, whose requests are denied by a third file nobody mentioned, and whose natural next move is to copy `hosts` into the allowlist, which is the bare-hostname trap. Suite E looped bank/*/provider.json while suites A, B and D iterate the skeleton too, so this was the one check it fell out of — which also made template/provider/README.md's "validated like real entries" not quite true. Widening the loop turned up a bug in suite E as shipped. It composed the path as bank/$nm/allowlist, but $nm is .name, and a skeleton's directory names its *shape* — static-key — while .name is the placeholder the author renames. So it reported a missing file at bank/acme/allowlist, a path nothing was ever going to write. Both suite E and suite C now derive the path from the manifest's own directory, the way suite B already did. Suite C is widened as well. It was also bank-only, and also hardcoded the same path. The skeleton exposes no route, so it passes on the "declares none and ships no .conf" branch — included so the README's claim holds for every suite rather than for most of them. The skeleton's allowlist is written for a first-time author rather than copied from a bank entry: one placeholder line, and comments carrying why the METHODS column is not optional, why deriving the file from `hosts` produces exactly the broken line, and what belongs here that must never be in `hosts`. Closes #104 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HZotg9g1oPFfkJjGGQLRhJ
Merged
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
1 participant
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.
Closes #104. Confirmed as filed, with one correction and one thing the fix turned up.
1.13.0 gave every bank entry an
allowlist; the skeleton did not get one, so an entry scaffolded from it landed in exactly the state that release was about. The report is right about who pays for it — someone writing their first provider, whose broker file and addon are both new, and whose natural next move is to copyhostsinto the allowlist, which is the bare-hostname trap.Correction to the diagnosis
The issue says Suite E is "the one place the skeleton falls out of the checks". Suite C is also
bank/*/provider.jsononly — it just happens to be benign today, since the skeleton exposes no route and would pass on the "declares none and ships no.conf" branch. Both are widened here, sotemplate/provider/README.md's "validated like real entries" holds for every suite rather than for most of them.What widening turned up — a bug in suite E as shipped in 1.13.0
It composed the path as
bank/$nm/allowlist, where$nmis.name. That is fine for a bank entry, where directory and name are the same string, and wrong for a skeleton, whose directory names its shape (static-key) while.nameis the placeholder the author renames (acme). Widening the loop with no file present produced:— a missing file reported at a path nothing was ever going to write. Both suites now derive it from
$(dirname "$m"), the way suite B already did. Worth noting because the message would have sent someone looking inbank/for a file that belongs intemplate/provider/.The issue's other claim checks out exactly: the skeleton fails Suite E rather than passing vacuously, so adding the file is what makes widening possible rather than merely tidy.
The skeleton's allowlist
Written for a first-time author rather than copied from a bank entry — one placeholder line, and comments carrying the three things that are not guessable:
GET,HEAD,OPTIONS, and denies every write while looking correct — and the resulting symptom sends you back into the broker file and the addon, which are not where the problem is.hosts, for the same reason:hostscarries no methods, so copying it produces exactly that line.hostsand this file are different lists, with the asymmetry spelled out and pointers tobank/github/allowlist(github.comhere, never inhosts) andbank/cloudflare/allowlist(*.workers.devfine to reach, a bug to inject for).Plus a pointer from
PLAYBOOK.md's "Working out an entry's egress" and a paragraph intemplate/provider/README.mdsayingallowlistis one of the blanks, not an optional extra.Tests
tests/run.sh— 16 suites, all pass; the lint goes 401 → 405 assertions, the four new ones being the skeleton under suites C and E.Targets
release/1.13.1as a fix.🤖 Generated with Claude Code
https://claude.ai/code/session_01HZotg9g1oPFfkJjGGQLRhJ