Skip to content

test(cli): a test seam for os serve's unknown-hostname guard — refusal literal stays inline (#9442) - #9583

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-9442-serve-hostname-guard-test-seam
Aug 18, 2026
Merged

test(cli): a test seam for os serve's unknown-hostname guard — refusal literal stays inline (#9442)#9583
os-zhuang merged 2 commits into
mainfrom
claude/issue-9442-serve-hostname-guard-test-seam

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#9442

What was missing

os serve's OS_ROOT_DOMAIN unknown-hostname guard was a plugin object literal
built inside Serve.run(), closing over __rootDomain, RESERVED,
getEnvRegistry and process.env.OS_CLOUD_URL, installing itself on a
http.server service resolved from the plugin context. Nothing about it was
exported or constructible, so reaching any of it meant booting a real
os serve — and nothing did. Every branch was untested, including the one whose
own comment says a 404 there "would kill the container".

What changed

createUnknownHostnameGuardPlugin() is now exported from serve.ts, the way its
sibling helpers (resolveTenancyPostureOrRefusal, resolveStorageCapabilityArg,
…) already are, with run() calling it. That is the established extraction
pattern in this file, and it is applicable here because it keeps the guard — and
its refusal literal — inside serve.ts.

Pure extraction, proved rather than asserted. Normalizing indentation and
applying the five declared rebinds to the pre-change source makes the two
middleware bodies identical over all 177 significant lines, and the embedded
404 HTML page is byte-identical (2089 bytes both sides). The five rebinds:

beforeafterwhy it is not a behaviour change
__rootDomainrootDomainsame value; the factory re-applies the same trim().toLowerCase(), which is idempotent on the call site's already-normalized input
const RESERVED = new Set([…])UNKNOWN_HOSTNAME_GUARD_RESERVED_SUBDOMAINSsame members, now exported so the test iterates the source of truth
p === '/api/v1/health' || …HEALTH_PATHS.includes(p)same three paths, same order-independent membership test
process.env.OS_CLOUD_URLreadCloudUrl()a function, defaulting to the same env read, still called per request — capturing a string at install time would have been a behaviour change wearing a refactor's clothes, and there is a test that fails if it ever becomes one
const guardPlugin: any = { … }return { … } from the factorythe object is built in a function instead of a block

Zone 1, condition 1 — the refusal literal stayed inline, and here is how I know the pin still bites

"CI is green" is necessary and not sufficient, so I measured the pin in both
directions on the post-change tree:

Vacuity probe (what condition 1 forbids). I hoisted the refusal body into a
local const refusalBody and left c.json(refusalBody, 404) in place, then ran
node scripts/check-route-envelope.mjs: exit 0, still "11 module(s) audited".
The file stays discovered — bodies counts the c.json( call — while all six
counters read nothing, so the {} entry keeps passing and stops asserting.
Exactly the silent vacuity the card describes. Reverted.

Non-vacuity probe (the one that proves the pin sees this file). With the
literal inline as shipped, I injected the pre-#9364 dialect into it
({ error: 'environment_not_found', message, hostname }) and re-ran the gate:

✗ Route-envelope conformance (#3843)
packages/cli/src/commands/serve.ts
unenveloped: found 1, declared 0 — a NEW non-conforming body.
packages/cli/src/commands/serve.ts
stringError: found 1, declared 0 — a NEW non-conforming body.

Red, naming this file, with the counters moving. Reverted; the tree is back to
the committed state. A defect injected into the shipped literal turns the gate
red, which is what "the #9364 pin still pins" means.

The tests, and why they pin both directions

packages/cli/src/commands/serve-unknown-hostname-guard.test.ts — 34 tests. They
mount the real middleware on a real Hono app (HonoHttpServer, already a
dependency of this package and the same class production resolves as
http.server), driving it with app.fetch(new Request(…)). No mock of the
middleware anywhere.

A suite that only asserted the refusal would stay green if the guard started
refusing everything — the container-killer the card warns about. So:

  • refusal — pinned by error.codeand HTTP status together, plus
    success: false, the message, error.details.hostname, and that the body has
    no top-level key besides success and error; the text/html variant too;
  • pass-through — every bypass asserted as an explicit 200 PASSED_THROUGH
    from a sentinel route mounted after the guard, never "not a 404" (Hono's own
    unmatched answer is a 404 and would read as a refusal): a mapped hostname,
    every reserved subdomain and the apex, every health/readiness path, /_admin
    and /.well-known/*, non-platform hostnames, and all four env-registry
    failure modes;
  • the reserved list and the probe-path list are iterated from the exported
    constants the middleware itself branches on
    , so a path added to the guard is
    covered the moment it is added rather than the day someone copies it here.

Break it and watch it go red — predicted vs actual

Both ablations were applied on top of the committed implementation and reverted
with git checkout afterwards (git status clean, back at 751ee771c).

ablationpredictedactual
delete the health/readiness bypassexactly the 3 parametrized probe-path cases go red, expected 404 to be 200; the reserved-host /api/v1/health case stays green because it returns at the reserved branch first3 failed / 31 passed — precisely those three, AssertionError: expected 404 to be 200
make a registry miss fall through, so nothing is ever refusedthe 5 cases asserting a 404 go red, expected 200 to be 4045 failed / 29 passed — precisely those five

Ratchet moved DOWN, by one number, by hand

scripts/slot-lookup-baseline.json: packages/cli/src/commands/serve.ts 10 → 9.
The old const guardPlugin: any = { … } declarator contained
ctx.getService?.('http.server') inside its initializer, which is one of the
erasure sites the rule counts; the declarator now initializes from a factory call
and contains no lookup. pnpm check:slot-lookup demanded the ratchet-down
itself ("erasure count fell 10 → 9 — ratchet DOWN"). Edited as a single number
rather than regenerated with --update, so nothing else in the baseline could
move; the diff is one line.

Verification

Union re-run at 89b8cfd4a (the final commit):

  • pnpm --filter @objectstack/cli typecheck — exit 0
  • vitest run src/commands/serve-14 files, 152 tests passed (the new 34
    plus every sibling that imports serve.ts)
  • check:route-envelope · check:slot-lookup · check:query-options-erasure ·
    check:engine-double-contract · check:where-matcher ·
    check:type-check-coverage · check:cross-package-test-inputs ·
    check:nul-bytes · check:objectui-changeset ·
    check:changeset-gate-self-tests — all pass
  • check-empty-changeset · check-changeset-no-major ·
    check-adr-0087-registration — pass on the committed diff

The whole packages/cli suite and check:type-check-debt --re-measure (which
needs the entire workspace closure built) are left to CI's Test Core and
Lint & Repo Gates.


Generated by Claude Code

The OS_ROOT_DOMAIN guard was a plugin object literal built inside
Serve.run(), closing over its locals and installing itself on a
`http.server` service resolved from the plugin context. Nothing about it
was exported or constructible, so reaching any of it meant booting a real
`os serve` — and every branch went untested, including the health/
readiness bypass whose own comment says a 404 there "would kill the
container".
Extract the SEAM, not the refusal: `createUnknownHostnameGuardPlugin()`
is exported from serve.ts the way its sibling helpers are, with run()
calling it. The middleware is byte-identical modulo indentation and five
declared rebinds (`__rootDomain` → `rootDomain`, the reserved set and the
health-path list to exported constants, and `process.env.OS_CLOUD_URL` to
an injectable reader that is still called PER REQUEST). The
`c.json({ ... }, 404)` refusal body stays an INLINE OBJECT LITERAL:
check-route-envelope.mjs judges the literal passed to `c.json(...)`, so
hoisting it would leave the file discovered while every counter read
zero — the #9364 conformance pin would go vacuous with the gate green.
The new suite mounts the real middleware on a real Hono app and pins both
directions: every bypass as an explicit pass-through (a sentinel 200,
never "not a 404" — Hono's own unmatched answer is a 404 too), and the
refusal by error.code AND HTTP status together.
The slot-lookup ratchet moves DOWN 10 → 9 for serve.ts: the
`const guardPlugin: any = { … }` declarator no longer contains a
service-lookup call, so one erasure site is gone. Hand-edited by one
number rather than regenerated.
Co-Authored-By: Claude <noreply@anthropic.com>
`rawApp.fetch()` is typed `Response | Promise<Response>`, so the harness
returned something the declared `Harness` interface did not accept.
`pnpm --filter @objectstack/cli typecheck` now exits 0.
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 16 documentable anchor(s).

25 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json a0d24bf8813b38e2bd17bc626186296c3bf255bc.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json a0d24bf8813b38e2bd17bc626186296c3bf255bcpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 9a0a4409ec041d4210042f4bedf57730bdf07bfd — the merge of head 89b8cfd4a59f8234191c6d16393df8db4ef872c2 into base a0d24bf8813b38e2bd17bc626186296c3bf255bc, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9a0a4409ec041d4210042f4bedf57730bdf07bfd && git checkout 9a0a4409ec041d4210042f4bedf57730bdf07bfd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a0d24bf8813b38e2bd17bc626186296c3bf255bc 89b8cfd4a59f8234191c6d16393df8db4ef872c2 && git checkout -B drift-repro a0d24bf8813b38e2bd17bc626186296c3bf255bc && git merge --no-ff 89b8cfd4a59f8234191c6d16393df8db4ef872c2
node scripts/docs-audit/affected-docs.mjs --json a0d24bf8813b38e2bd17bc626186296c3bf255bc

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs a0d24bf8813b38e2bd17bc626186296c3bf255bc → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 18, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 18, 2026 12:08
@os-zhuang
os-zhuang enabled auto-merge August 18, 2026 12:08
@os-zhuang
os-zhuang added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit a5d2593Aug 18, 2026
25 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-9442-serve-hostname-guard-test-seam branch August 18, 2026 12:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os serve's unknown-hostname guard has no test seam — the whole middleware, refusal body included, is unreachable from a test

2 participants

@os-zhuang@claude