Skip to content

fix(plugin-auth): a public plain-HTTP boot says its AS discovery is unencrypted, in English - #19689

Merged
huangyiirene merged 3 commits into
mainfrom
claude/issue-19571-plaintext-as-warning
Sep 23, 2026
Merged

huangyiirene merged 3 commits into
mainfrom
claude/issue-19571-plaintext-as-warning

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #19571

Implements ruling batch #210 item 5 (letter B/B, maintainer 「210 同意」), which governs over the issue body. Upstream ruling: #19489 item ④. PR #19534 is landed and is not reverted here — this is its follow-up.

Clause-②: no

D1 — eligibility decides WHICH sentence, never WHETHER one is emitted

packages/plugins/plugin-auth/src/auth-plugin.ts#registerOidcDiscoveryRoutes.

The structural defect, verified on origin/main before writing code: the three AS discovery routes (/.well-known/oauth-authorization-server, /.well-known/openid-configuration and the RFC 8414 §3.1 path-insertion variant) are mounted unconditionally, while the only line that mentioned a refused transport — MCP server is enabled but the OAuth track is NOT live — sits inside if (readMcpServerEnabledEnv() && ...). A public plain-HTTP boot with OS_MCP_SERVER_ENABLED=false therefore emitted no warning at all, while publishing its authorization-server metadata in the clear. The loudest-needed configuration was the quietest.

The plain-HTTP branch now has a sibling else if, beside it and not inside the MCP block:

  • accepted origin (loopback / private / link-local) — unchanged line, minus its CJK prefix: OAuth is served UNENCRYPTED: ...
  • refused origin (public host) — new, distinct line: OAuth discovery is served over PUBLIC plain HTTP: ..., naming (a) the discovery documents and the issuer, (b) that the MCP OAuth track is DISABLED, (c) that TLS is the remedy.

Both fire once at mount, neither under TLS, and no configuration key or environment variable gates either.

⛔ No admission decision changes. isOAuthEligibleBaseUrl and every transport-rule predicate are byte-unchanged; the discovery routes are mounted exactly where and when they were. ⛔ No new exported symbol (servedOverPlainHttp is a local const) — the Clause-②: no declaration stands, and the mechanical floor at references/contract-review.md:12 is not tripped.

D2 — the emitted string is English; the ruled sentence moves to the comment

The 'OAuth 未加密:仅限可信内网 — ' prefix is out of the executable string. The maintainer's sentence is kept verbatim in the code comment beside the call, cited to the #19489 ruling and to batch #210 item 5, and read as the line's meaning rather than its literal encoding. ⛔ No CJK executable string remains in either file's src tree; ⛔ no bilingual line. AGENTS.md is untouched.

The comment block above the branch previously argued the opposite — that the Chinese belongs in the emitted string "not only in this comment". That paragraph is falsified by this ruling and has been rewritten to state the current rule with its citation.

Carriers moved with it

  1. packages/plugins/plugin-auth/src/auth-plugin.ts — the two branches and the comment.
  2. packages/plugins/plugin-auth/src/mcp-oauth-plaintext-notice.test.ts — NOTICE_MARKER re-anchored to the English line; PUBLIC_NOTICE_MARKER added; the "does NOT fire on a PUBLIC plain-HTTP deployment" leg rewritten to assert the sentence swaps rather than vanishes; new coverage for the MCP-surface-OFF boot, MCP-independence, once-per-mount, warn level, mutual exclusivity and the no-env-var floor. 15 tests, all green.
  3. docs/qa/platform-checklist/areas/ai.json — ai.mcp-oauth-private-host-transport revision 1 → 2 with its history entry. The grep anchors move to the two English lines; case (b) moves from asserting 0 occurrences to asserting exactly 1 of the new line; step text, clause, verify, the negative conflation entry, the source line and the title moved together.
  4. .changeset/19489-oauth-private-host-transport-rule.md — still unreleased on origin/main (the file is present in .changeset/, so changeset version has not consumed it). Its last bullet asserted the startup line carries the Chinese and that a public plain-HTTP boot gets no such line; both become false when this lands, so the sentence is corrected. The Chinese is kept only as a quoted ruling citation.

Plus a new changeset, .changeset/19571-plaintext-oauth-public-host-notice.md (@objectstack/plugin-auth: patch).

⚠️ One gate is red BY DESIGN and needs a human word

node scripts/check-empty-changeset.mjs --base origin/main — exit 1, on carrier 4:

.changeset/19489-oauth-private-host-transport-rule.md present on the merge base and CHANGED by this PR

This is the gate's DELIBERATE CORRECTION class, not the COLLISION class. Its own text says the remedy is not to restore the file — restoring it from the base republishes a sentence this PR makes false — and that correcting a pending release note "is a decision about a release rather than a refactor — say so on the PR, naming the note and what changed under it, and get it confirmed". Naming it here: the note is 19489-oauth-private-host-transport-rule.md, and what changed under it is the D1 branch above. The gate stays red until a person confirms.

Verification

check result
pnpm --filter @objectstack/plugin-auth test 114 files / 2439 passed
pnpm --filter @objectstack/plugin-auth run typecheck exit 0 (after building the package; tsconfig.examples.json resolves through dist)
pnpm lint (repo-wide eslint . --no-inline-config) exit 0
pnpm --filter '@objectstack/plugin-auth^...' build exit 0
derived gate families (scripts/pm/dispatch-gates.mjs --commands), 66 commands 63 green · 1 red by design (above) · 2 PREREQUISITE NOT MET

pnpm check:platform-checklist, check:nul-bytes, check:doc-authoring, check:test-source-alias, check:cross-package-test-inputs, check:published-files, check:type-check-coverage, check:adr-0087-registration, check:changeset-no-major all exit 0.

NOT MEASURED (each prints PREREQUISITE NOT MET — explicitly "not a pass and not a finding" — and needs a full workspace build, which is CI's): pnpm check:dual-build-cjs-loads (exit 3), pnpm check:type-check-debt (exit 3). This diff changes no exports, no package.json and no build shape.

Reverse verification (both legs from the committed state; restored byte-identically)

Run through scripts/ablation-replace.mjs, which proves the mutation landed on disk by blob hash and proves the restore by git diff HEAD being empty. The subject resolves from src through a relative import, so no dist leg applies.

  1. The new branch can fail. } else if (servedOverPlainHttp) { → } else if (servedOverPlainHttp && false) {. Blob 17bd405104b2 → eb7d11ae9d01. Result: 9 failed | 6 passed (15) — every public-face assertion red. Restored: blob back to 17bd405104b2, git diff HEAD 0 bytes.
  2. The D2 pin can fail. The CJK prefix re-added to the accepted line. Blob 17bd405104b2 → c7aba397fff2. Result: 1 failed | 14 passed (15) — fires on a private-address deployment red on the no-CJK assertion. Restored: ok restored: blob == HEAD (17bd405104b2) and git diff HEAD is empty.

Predicted direction was RED for both, and RED is what was observed.

Acceptance notes

  • The comment now states explicitly that whether the .well-known discovery routes should be mounted at all on a refused origin is pre-existing main behaviour neither ruling touched, and is not decided at that call site — per the ruling, that would be a card on its own evidence, ⛔ not folded here.

Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ai/agents.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/ai/connect-mcp.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/ai/index.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/ai/natural-language-queries.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/api/index.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/capabilities/ai.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/deployment/cli.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/deployment/environment-variables.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/deployment/index.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/deployment/self-hosting.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/getting-started/build-with-claude-code.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/permissions/authorization.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v13.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))
  • content/docs/releases/v16.mdx (via /api/v1/mcp (route, a path literal in registerOidcDiscoveryRoutes))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 14 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 1f53b0b685bc5477f2eb702c7938e94b7009e59d → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 71d043664b366c353db5c71551fa2b0bacb5a9be — the merge of head 629178648adebd181ee031ae4b0412839231583c into base 1f53b0b685bc5477f2eb702c7938e94b7009e59d, 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 71d043664b366c353db5c71551fa2b0bacb5a9be && git checkout 71d043664b366c353db5c71551fa2b0bacb5a9be
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1f53b0b685bc5477f2eb702c7938e94b7009e59d 629178648adebd181ee031ae4b0412839231583c && git checkout -B drift-repro 1f53b0b685bc5477f2eb702c7938e94b7009e59d && git merge --no-ff 629178648adebd181ee031ae4b0412839231583c

node scripts/docs-audit/affected-docs.mjs --json 1f53b0b685bc5477f2eb702c7938e94b7009e59d

⚠️ 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 1f53b0b685bc5477f2eb702c7938e94b7009e59d → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

Gate on the record: Check Changeset is red BY DESIGN and will stay red

domain:services PM seat · session_01AhQASwqJr2Z7XfGWUdvnbF · 2026-09-22T14:15Z · head 629178648a

The gate: Check Changeset (pr-automation.yml), running scripts/check-empty-changeset.mjs.

The reason: this PR changes .changeset/19489-oauth-private-host-transport-rule.md, a changeset present on the merge base and added by another PR. The gate sorts that into two classes and this is the DELIBERATE CORRECTION one: the note describes behaviour this PR changes. Its last bullet asserted that the startup line carries 「OAuth 未加密:仅限可信内网」 and that a public plain-HTTP boot gets no such line — both false the moment ruling batch #210 item 5 lands, in a note that is still unreleased and therefore still ships.

Why it is not "fixable" to green. The gate says so itself, verbatim:

For the DELIBERATE CORRECTION class there is no second command to run, and step 1 above is the one thing not to do: the note you rewrote describes behaviour THIS PR changed, so restoring it from the base republishes a sentence that is now false, and no label and no diff shape makes that safe. … this gate stays red either way, and staying red is what puts the decision in front of a person instead of routing around it.

⇒ Green is reachable only by restoring the false sentence or deleting the note, and the gate refuses deletion explicitly ("the same act one step further"). ⛔ Neither was done.

Enqueue basis. SKILL.md:639-641, the third case — a gate red by design may be carried into the queue only when three conditions all hold. Measured, not recalled:

  1. The source self-describes as red by design on a pushed branch — the quotation above, read from origin/main:scripts/check-empty-changeset.mjs.
  2. It does not run in merge_group — pr-automation.yml declares on: pull_request: types: [opened, synchronize, reopened, labeled, unlabeled, edited] and nothing else, so this check never gates the queue itself.
  3. A PR comment records the gate and the reason — this comment.

The section's own scope condition also holds: this is a dev PR dispatched by this loop, ⛔ not a PM tooling PR.

Everything else is green. 34 check-runs at this head: 30 success, 3 skipped, 1 failure — this one. All three skips are in the roster (check-expected-skips.mjs, exit 0): Build Docs, Console Pin Gate, Packed-tarball smoke (opt-in).

Still un-supplied, stated plainly: the gate asks for a person's confirmation of the release-note correction, and no one has given it. ⛔ This seat does not supply it and does not claim it was given. The three conditions above are what license the enqueue; the confirmation is a separate word that is still owed. If the maintainer would rather the correction were dropped, say so and it comes out — but then #19489's pending note ships two sentences that this PR makes false.


Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review September 22, 2026 14:15
@huangyiirene
huangyiirene added this pull request to the merge queue Sep 23, 2026
Merged via the queue into main with commit a754563 Sep 23, 2026
35 of 36 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-19571-plaintext-as-warning branch September 23, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[decision] #19489 落地带出两条裁量分叉:公网明文 AS 要不要自己的告警;裁定的中文日志措辞与 AGENTS.md 语言惯例相抵

2 participants