Skip to content

docs(spec,adr-0069): server.trustProxy names both client-address consumers; D5 mechanism amended - #24859

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-24818-trustproxy-consumers
Oct 21, 2026
Merged

os-zhuang merged 1 commit into
mainfrom
claude/issue-24818-trustproxy-consumers

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Closes #24818

Clause-②: no

Two texts were behind the code after PR #24795 (25e16ab00c). This PR fixes both. No code, schema shape, key, default or accepted value changes.

What changed

  1. packages/spec/src/system/stack-server.zod.ts, the server.trustProxy .describe() and TSDoc. The describe named only the inbound rate limiter. It now names both consumers of the one client-address rule: the inbound rate limiter (when server.security.rateLimit.enabled is set), and the authentication plugin. That plugin's IP allow-list (allowed_ip_ranges) judges the address the rule establishes, and its login audit stamps it as last_login_ip. The TSDoc names the shared function (resolveClientAddress from @objectstack/plugin-auth/client-address) and says that a header believed without a rewriting proxy bypasses the allow-list as well as the rate limit.
  2. content/docs/references/system/stack-server.mdx, regenerated with pnpm --filter @objectstack/spec gen:docs. check:generated named this as the only stale artifact. The diff is the one table row.
  3. docs/adr/0069-enterprise-authentication-hardening.md, D5. I added a dated amendment note under the D5 table. It follows the in-file precedent: the D3 note "What the gate holds (amended 2026-10-14 ...)", plus the matching · **Amended** (...) entry on the Status line. The original mechanism cell is unchanged, apart from an appended pointer, (client-address mechanism amended 2026-10-20 — see the note below). The note records:
    • X-Forwarded-For (leftmost entry), then X-Real-IP, are believed only under server.trustProxy. Otherwise the address is the transport's peer address.
    • CF-Connecting-IP is no longer read.
    • While any range is set, an address that cannot be established is refused with 403 IP_NOT_ALLOWED. With no ranges set, nothing is judged.
    • The D7 audit stamps the same address and stamps none when it is undetermined.
    • objectstack serve passes server.trustProxy to AuthPlugin.
  4. .changeset/24818-trustproxy-consumers-describe.md, a patch for @objectstack/spec, because the describe string ships in the spec's published dist.

Each claim checked against the code at origin/main 41646a5788

  • The rule: packages/plugins/plugin-auth/src/client-address.ts resolveClientAddress reads x-forwarded-for, then x-real-ip, only when input.trustProxy is true. Otherwise it uses remoteAddress. With neither available it returns undefined.
  • The rule is published as a subpath (packages/plugins/plugin-auth/package.json, exports["./client-address"], a tsup entry) and imported back by packages/runtime/src/security/inbound-rate-limit.ts:40.
  • serve wiring: packages/cli/src/commands/serve.ts:4263 passes trustProxy: (config as any).server?.trustProxy === true to new AuthPlugin(...).
  • The allow-list judges the established address: packages/plugins/plugin-auth/src/auth-plugin.ts, in the D5 middleware, calls mgr.resolveClientAddress(...). AuthManager.resolveClientAddress applies the shared rule with this.config.trustProxy.
  • Undetermined means refused: AuthManager.isClientIpAllowed returns true when no ranges are set, and if (!ip) return false otherwise. The middleware then answers 403 with error.code: 'IP_NOT_ALLOWED'.
  • CF-Connecting-IP: git grep -i cf-connecting-ip -- packages finds only CHANGELOG.md entries and no source.
  • D7: the sign-in after-hook calls stampLastLogin(uid, await readRequestClientAddress()). stampLastLogin writes last_login_ip only if (ip).

Governed surface

docs/adr/** is a governed surface at Tier H. This PR lands only after an authorized approval by an account in GOVERNED_APPROVERS. The seat handles that approval.

Verification (at d4393185dd)

  • pnpm --filter @objectstack/spec build passed (VERDICT command-exit 0).
  • pnpm --filter @objectstack/spec check:generated: before regenerating, it reported "1 of 14 artifact(s) stale: content/docs/references/**". After gen:docs it reported "✓ All 14 generated artifacts are up to date".
  • pnpm --filter @objectstack/spec typecheck passed (VERDICT command-exit 0).
  • Spec tests, narrowed: vitest run src/system/stack-server.test.ts src/system/http-server.test.ts src/compose-stacks-key-loss.test.ts scripts/build-schemas-check-mode.test.ts gave Test Files 4 passed (4), Tests 154 passed (154). These are the spec tests that name stack-server or trustProxy. The full spec suite did not finish inside the container's foreground cap on a shared box, so it is left to CI's Test Core. No test pins the old describe text: git grep "Consumed by the inbound" has no hit outside the changed source.
  • node scripts/pm/dispatch-gates.mjs --ran gave "111 derived famil(ies) accounted for — 109 run, 2 NOT-MEASURED". Every command that ran exited 0, after building the @objectstack/lint... and @objectstack/client-react... closures so four of them could measure.
    • NOT MEASURED: check:dual-build-cjs-loads, reason: needs a whole-workspace dist (exit 3, prerequisite not met). Left to CI.
    • NOT MEASURED: check:lean-entry-closure, reason: needs packages/objectql/dist (exit 3). Left to CI.
  • pnpm check:nul-bytes: OK.

Acceptance notes

  • None. No out-of-scope findings.

维护者速读(草稿)

  • 改了什么:只改文字,不改代码。server.trustProxy 的说明原来只写了"限流器读它";现在写明认证插件的 IP 白名单和登录审计(last_login_ip)也按同一条规则取客户端地址。ADR-0069 D5 补一条带日期的修订注记,写明现在的取址方式。
  • 为什么改:PR fix(plugin-auth): the auth IP allow-list judges the client address the server established #24795 之后代码已经这样运行,但对外文档和 ADR 还是旧说法(还列着已不再读取的 CF-Connecting-IP)。运维照旧文档配置,可能不知道这个开关同时影响登录白名单。
  • 风险与代价(含回滚):无运行时行为变化,风险极低。回滚即 revert 本 PR。
  • 席位意见:
  • 你要做的:ADR 属 Tier H 受管面,需要你批准后才能落地。

Generated by Claude Code

…umers; D5's mechanism cell amended

The `server.trustProxy` describe named only the inbound rate limiter; since
the auth allow-list fix, `objectstack serve` passes it to AuthPlugin too,
whose D5 IP allow-list and D7 `last_login_ip` audit use the same
client-address rule (`@objectstack/plugin-auth/client-address`). The
generated reference is regenerated with gen:docs.

ADR-0069 D5 gains a dated amendment note: forwarded headers
(X-Forwarded-For, then X-Real-IP) only under server.trustProxy, otherwise
the transport peer; CF-Connecting-IP no longer read; an undetermined
address refused (403 IP_NOT_ALLOWED) while ranges are set.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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 — 143 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 abd5acdaf788df2092064c56583fd73d837fb858 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json abd5acdaf788df2092064c56583fd73d837fb858

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

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 11:22
@objectstack-fleet
objectstack-fleet Bot requested a review from hotlong as a code owner October 20, 2026 11:22
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Seat check: PASS · domain:spec seat 3 (#18883) · baozhoutao · session session_01D6F9axfoPzR7t8G1J7Ytzz · 2026-10-20T11:22Z

Governed (Tier H): docs/adr/0069-enterprise-authentication-hardening.md hits the governed-surface register, so this PR lands only after an approval from os-zhuang or hotlong. The seat does not approve or merge it. Auto-merge is enabled only after that approval.

Reviewed head d4393185dd, three-dot against origin/main 7cacad5348. Merges clean (git merge-tree), 40 lines, 4 files.

  • The ADR change is an amendment, not a rewrite. D5's original mechanism cell keeps its text and gains a pointer. A dated note (2026-10-20, PR fix(plugin-auth): the auth IP allow-list judges the client address the server established #24795) records the mechanism as it now runs:

    • resolveClientAddress reads X-Forwarded-For, then X-Real-IP, only under server.trustProxy; otherwise it uses the transport peer.
    • CF-Connecting-IP is no longer read.
    • An undetermined address answers 403 IP_NOT_ALLOWED while ranges are set.
    • D7's last_login_ip stamps the same address.

    The decision is unchanged. The dev verified each claim against plugin-auth/src/client-address.ts, serve.ts:4263 and runtime/security/inbound-rate-limit.ts:40.

  • server.trustProxy's describe and TSDoc name both consumers. The generated reference was regenerated with gen:docs.

  • No code, schema shape or key change. There is a patch changeset for @objectstack/spec.


Generated by Claude Code

@os-zhuang
os-zhuang added this pull request to the merge queue Oct 21, 2026
@os-zhuang
os-zhuang self-requested a review October 21, 2026 04:13
Merged via the queue into main with commit 55d7aab Oct 21, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-24818-trustproxy-consumers branch October 21, 2026 04:45
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: d4393185dd890b3ee4de3b53ad4d44ba85b1eea5
Local-runs: none
Scope: packages/spec/src/system/stack-server.zod.ts, content/docs/references/system/stack-server.mdx, docs/adr/0069-enterprise-authentication-hardening.md + .changeset/24818-trustproxy-consumers-describe.md

Retroactive. This record is written after the merge, as 55d7aaba4c. The seat's rule owed a contract review before landing, because the PR edits a non-test file under packages/spec/src/**, and none was done. The review covered the merge commit against its first parent (157ebd008b..55d7aaba4c, 4 files, +35/−5). The content at d4393185dd is byte-identical to what landed.

① Derived judgments

  • The only spec hunk adds TSDoc lines and rewrites the tail of the server.trustProxy .describe() string. z.boolean().default(false), the strict schema around it, every sibling key, every refinement and the inferred StackServerConfig type are unchanged. No test pins the old string.
  • Every runtime claim in the new text and in the ADR-0069 D5 amendment was checked against origin/main:
    • the shared resolveClientAddress rule (plugin-auth/src/client-address.ts:72-89, client-address-forwarding.ts:24);
    • its use by the inbound rate limiter (runtime/src/security/inbound-rate-limit.ts:156-159) and by AuthPlugin (cli/src/commands/serve.ts:4263, auth-plugin.ts:2769-2790, auth-manager.ts:9898-9922);
    • the D7 last_login_ip stamp (auth-manager.ts:2909, :9663);
    • that nothing in packages reads CF-Connecting-IP any more.
  • The generated stack-server.mdx row is byte-equal to the evaluated describe string.
  • Non-blocking accuracy finding. The phrase "two consumers" leaves out a third caller: the per-endpoint declarative rate limiter (runtime/src/endpoint-policy.ts:366-372, threaded at dispatcher-plugin.ts:2278-2282). It uses the same rule under the same trustProxy whenever an endpoint declares rateLimit, whether or not server.security.rateLimit.enabled is set. No key, default or behaviour is misstated. A follow-up card has been filed.

② Semver level

@objectstack/spec patch is correct. The describe string ships in the published Zod runtime, so a changeset is owed. Nothing breaks, so no ADR-0087 marker is needed.

③ Boundary flags

Clause-②: no is correct: no contract member, accepted set, key, default or export moves. Tier H (docs/adr/**): approved by os-zhuang on this exact head and merged by os-zhuang.

Implemented-by: claude/issue-24818-trustproxy-consumers
Reviewed-by: session_01D6F9axfoPzR7t8G1J7Ytzz

VERDICT: PASS

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 protocol:system size/s tooling

Projects

None yet

3 participants