Skip to content

fix(plugin-auth): the auth IP allow-list judges the client address the server established - #24795

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24739-auth-ip-allowlist-trust-rule
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24739-auth-ip-allowlist-trust-rule

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24739

Clause-②: no

What changes

The ADR-0069 D5 IP allow-list (allowed_ip_ranges) and the D7 login audit (last_login_ip) now establish a request's client address by the platform's one client-address trust rule, the rule the runtime's inbound rate limiter already used:

  • X-Forwarded-For (its first entry), then X-Real-IP, count only when the stack declares server.trustProxy.
  • Otherwise the address is the transport's peer address (IHttpRequest.remoteAddress).
  • With ranges configured, an address that cannot be established is refused with 403 IP_NOT_ALLOWED. That is the code and status the gate already uses for an address outside the ranges. The first such refusal logs one operator warning that names allowed_ip_ranges and server.trustProxy.
  • With no ranges configured, nothing is judged. This is unchanged.
  • The D7 audit stamps the address the gate judged, or no address when none could be established.

The maintainer's direction, verbatim from the card: 「登录 IP 白名单信任可伪造的 IP 头,取不到 IP 时还放行」

How

File Change
packages/plugins/plugin-auth/src/client-address.ts (new) resolveClientAddress: the address half of the runtime's resolveRateLimitKey, moved unchanged. It is published as the dependency-free @objectstack/plugin-auth/client-address subpath (679 B ESM, no imports), beside ./rate-limit-storage.
packages/runtime/src/security/inbound-rate-limit.ts resolveRateLimitKey imports the rule instead of keeping its own copy. Its keys are unchanged, and the limiter's own pins are green (below).
packages/plugins/plugin-auth/src/auth-manager.ts A trustProxy option. resolveClientAddress / trustsForwardedHeaders. isClientIpAllowed refuses an undetermined address while ranges are set. ipMatchesRange compares an IPv4-mapped IPv6 address as its IPv4 address: a dual-stack socket reports IPv4 peers that way, and the gate now judges the peer. handleRequest(request, { remoteAddress }) establishes the address once and carries it to the D7 stamp in better-auth's request state. The allowedIpRanges TSDoc no longer states the fail-open.
packages/plugins/plugin-auth/src/request-client-address.ts (new) The request-state slot that carries the established address to the sign-in after-hook.
packages/plugins/plugin-auth/src/auth-plugin.ts The D5 middleware judges resolveClientAddress over the request's headers and transport peer. The wildcard route hands the same peer to handleRequest. Two once-per-process operator warnings.
packages/cli/src/commands/serve.ts objectstack serve passes server.trustProxy to AuthPlugin, as it already does to the dispatcher.
packages/services/service-settings/src/manifests/auth.manifest.ts, translations/{zh-CN,ja-JP,es-ES}.ts The allowed_ip_ranges help text says which address is judged. It used to say a trusted proxy setting X-Forwarded-For was enough.
content/docs/permissions/authentication.mdx The allowed_ip_ranges row, which stated the old IP source and the fail-open.
.changeset/24739-auth-ip-allowlist-trust-rule.md patch: plugin-auth, runtime, cli, service-settings.

Why the rule lives in plugin-auth. @objectstack/runtime depends on @objectstack/plugin-auth: its limiter already counts through ./rate-limit-storage. So plugin-auth cannot import the runtime's copy without a dependency cycle. The rule moved, and the runtime imports it back. The package root's export surface is unchanged.

Why the peer is read from the Hono context. The D5 gate and the route that feeds the D7 audit are raw-app mounts. They receive the Hono context, not an IHttpRequest. The read mirrors the transport's own readRemoteAddress (plugin-hono-server/src/adapter.ts), and returns nothing on a runtime that exposes no socket.

Pins, one per branch

packages/plugins/plugin-auth/src/client-ip-allow-list-trust-rule.test.ts mounts AuthPlugin.registerAuthRoutes on a real Hono app in front of a real AuthManager on the installed better-auth. It supplies the peer the way @hono/node-server does. Every refusal asserts status and code.

  • behind a declared trusted proxy, the allow-list judges the forwarded client address
  • without a declared trusted proxy, the allow-list judges the connection peer and a forwarded header changes nothing
  • an address the server could not establish is refused while ranges are set, and the operator is told why once
  • with no ranges configured nothing is judged, whatever the address
  • an IPv4 peer reported in IPv4-mapped form is judged as its IPv4 address
  • (D7) records the connection peer, not a forwarded header, when no proxy is trusted
  • (D7) records the forwarded client address the gate admitted when the proxy is declared trusted
  • (D7) records no address when none could be established

auth-manager.test.ts: the old "fails OPEN when the client IP cannot be determined" case is now "refuses an address the server could not establish once ranges are configured". Two cases are new: the IPv4-mapped matcher, and resolveClientAddress believing forwarded headers only under trustProxy.

Ablations

Each leg was committed first and restored from HEAD: git checkout HEAD -- PATH, or scripts/ablation-replace.mjs, which wraps the run. Each restore is proven by the blob hash matching HEAD and an empty git diff HEAD. The pins import src relatively, so no dist/ is involved.

Leg Mutation Red Green
A1 auth-plugin.ts + auth-manager.ts at base 795ff708d8 (the old behaviour) 8: the untrusted-proxy, undetermined and IPv4-mapped pins; D7 peer; D7 none; the 3 new or flipped units the trusted-proxy, no-ranges and D7 trusted pins. These are the preservation half: the old code believed headers too
A2 client-address.ts: the trust branch ignored 3: the trusted-proxy pin, D7 trusted, the unit's trusted leg the rest
A3 isClientIpAllowed: no ranges refuses 4: the no-ranges pin, its unit, and the two D7 pins served with no ranges the rest
A4 the wildcard route hands no peer to handleRequest 1: D7 peer the rest

Verification

Commit readings. The merge at 9f60a2ba3c brought one main commit that touches one dogfood test file and none of these packages.

  • c42164ff22: @objectstack/plugin-auth build exit 0 (check-dts-emitted 3/3), typecheck exit 0 (test layer compiles, no new debt), vitest run 166 files, 3188 passed, 10 skipped.
  • c42164ff22: @objectstack/runtime typecheck exit 0. 11 files that exercise the rule (security/inbound-rate-limit, endpoint-policy, endpoint-policy.auth-gate.pin, dispatcher-plugin.rate-limit.integration, dispatcher-plugin.endpoint-fallback.integration, dispatcher-plugin.multi-tenant-endpoint.integration, http-metrics-inbound-coverage.hono.integration, http-dispatcher.auth-gate-outage.pin, api-endpoint-step, error-envelope.conformance, domains/anonymous-gate-actions-automation): 244 passed. The rest of the runtime suite is declared to CI.
  • c42164ff22: @objectstack/service-settings typecheck exit 0, 53 files, 894 passed.
  • 9f60a2ba3c: @objectstack/cli closure build 60/60, typecheck exit 0, vitest run --project unit 314 files green. Two published-subpath-* pins first refused as PREREQUISITE NOT MET (packages/cli unbuilt). After pnpm --filter @objectstack/cli build they passed, 29/29. The integration layer is declared to CI.
  • 9f60a2ba3c: node scripts/pm/dispatch-gates.mjs --commands derived 116 commands, and all 116 exit 0. Three of them (check:skill-examples, check:dual-build-cjs-loads, check:i18n-coverage) first exited 3 for unbuilt packages and were re-run after a cached repo build. --ran: 116 derived, 116 run, 0 NOT-MEASURED, 0 UNRUN.

Cross-lane surfaces

  • packages/runtime/src/security/inbound-rate-limit.ts (domain:cli): declared in the claim.
  • packages/cli/src/commands/serve.ts (domain:cli): not in the claim's file surface. It is one option line, and it is what lets server.trustProxy reach the gate under objectstack serve.
  • packages/services/service-settings (help text) and content/docs/permissions/authentication.mdx: text this change made false. Neither is in the claim's file surface.

Acceptance notes

  • ADR-0069 D5 names the gate's IP source as "extract client IP (x-forwarded-for / cf-connecting-ip, trust-proxy aware)". This PR implements "trust-proxy aware" as the runtime's rule, which reads X-Forwarded-For, then X-Real-IP. CF-Connecting-IP is no longer read. The ADR text is not edited here.
  • The spec's server.trustProxy description names one consumer, the inbound rate limiter. This change adds a second: the auth allow-list and the login audit. packages/spec is not edited here.
  • Other HTTP doors into AuthManager.handleRequest pass no transport peer. Their D7 stamp therefore records no address unless server.trustProxy is declared. Their D5 coverage was not measured here.
  • objectstack dev (plugin-dev) composes AuthPlugin without trustProxy, the same way it composes the dispatcher without it.
  • docs/HARDENING.md shows a hand-written limiter keyed on x-forwarded-for. It is unchanged here.
  • Disclosure follows the maintainer's ruling recorded on skills: five principle gaps the #9296 wave exposed in checklist-test / checklist-author / dogfood-verification #9387. This PR states the rule and its refusals only.

Generated by Claude Code

…ablished

The ADR-0069 D5 allow-list and the D7 login audit now establish a request's
client address by the platform's one client-address trust rule, the rule the
runtime's inbound rate limiter already used: forwarded headers only when the
stack declares server.trustProxy, the transport's peer address otherwise.
With ranges configured, an address that cannot be established is refused
(403 IP_NOT_ALLOWED) and explained once in the operator log.

The rule moves unchanged from runtime into plugin-auth's dependency-free
./client-address entry (runtime depends on plugin-auth), and the runtime
imports it back. objectstack serve hands server.trustProxy to AuthPlugin as
well as to the dispatcher.

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
One pin per branch, through the real raw-app mounts: a declared trusted proxy,
an undeclared one, an address that cannot be established, no ranges, the
IPv4-mapped peer form, and the D7 audit recording the same address or none.

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
…t address

Also keeps the package root's export surface unchanged: the rule is reached
only through the ./client-address entry.

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation tests tooling labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/cli, @objectstack/plugin-auth, @objectstack/runtime, @objectstack/service-settings, touching 24 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/plugins/plugin-auth/package.json, packages/plugins/plugin-auth/tsup.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/deployment/cli.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/deployment/self-hosting.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/deployment/tenancy-modes.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/kernel/contracts/auth-service.mdx (via AuthManager (symbol, a top-level class), handleRequest (symbol, a method of class AuthManager))
  • content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class), AuthPlugin (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class), AuthPlugin (symbol, a top-level class), handleRequest (symbol, a method of class AuthManager))
  • content/docs/permissions/record-view-auditing.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/plugins/index.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/protocol/kernel/http-protocol.mdx (via trustProxy (symbol, a field of interface AuthManagerOptions; a field of interface ClientAddressInput))

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

  • content/docs/releases/v17/17-3.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-5.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-6.mdx (via AuthPlugin (symbol, a top-level class))

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
  • 2 changed file(s) yielded no anchor (packages/plugins/plugin-auth/package.json, packages/plugins/plugin-auth/tsup.config.ts) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: os serve (command, 34 pages)
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 54 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 a43405d596d4d670dc761e90c9fc20822dc806a9 → packageMentionDocs.

Which tree this was computed on

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

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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:services seat 1 · 2026-10-20T07:17Z

os-justin · session_011TdZ8GjixNZFLz1xJD9zzK (seat post #6021). Reviewed against GitHub and origin/main a43405d596, not against the report (os-dev-report on #24739).

Disclosure read first (the maintainer's ruling recorded on #9387). The PR body, the changeset and the report give operator guidance and the rule. None carries a bypass path.

Head under review:

9f60a2ba3cc5156c9b0722fd95a761adcbe7f187
  1. Shape. Draft, base main, first line Fixes #24739, line-start Clause-②: no in the body and the changeset. The only other issue the body names is skills: five principle gaps the #9296 wave exposed in checklist-test / checklist-author / dogfood-verification #9387, with no keyword beside it. 16 files, +689/−70, none on a governed surface (governedPathsIn answers []). The trial merge onto origin/main is clean.

  2. The rule, read in the code.

    • resolveClientAddress (@objectstack/plugin-auth/client-address, dependency-free) reads X-Forwarded-For's first entry, then X-Real-IP, only under trustProxy; otherwise the transport peer. It is the runtime's rule, moved unchanged. The runtime imports it back with its limiter keys unchanged. The import cannot go the other way because runtime already depends on plugin-auth.
    • That reading is the spec's contract: server.trustProxy is "declared true ONLY when a reverse proxy you control overwrites those headers on every inbound request" (stack-server.zod.ts:213).
    • D5 gate. isClientIpAllowed now answers false for an undetermined address when ranges are set. The middleware answers 403 IP_NOT_ALLOWED, the out-of-range envelope, and logs one operator line per kind.
    • D7 audit. last_login_ip reads the address handleRequest established, carried in request state, instead of re-reading headers.
    • With no ranges configured, nothing changes.
    • IPv4-mapped peers match their IPv4 range.
  3. Pins and ablations. client-ip-allow-list-trust-rule.test.ts mounts the real raw app and covers:

    • trusted proxy;
    • untrusted proxy;
    • undetermined address, refused with code + status and one operator line;
    • no ranges configured;
    • an IPv4-mapped peer;
    • D7 with a peer, with a trusted forwarded address, and with none.

    Ablations A1–A4 each turn red as predicted. A1 (the old behaviour) turns 8 red and keeps the trusted-proxy, no-ranges and D7-trusted rows green, which is the preservation half. Every restore was proven by blob equality.

  4. Prose read sentence by sentence against the diff.

    • The changeset, patch for plugin-auth, runtime, cli and service-settings:
      • the rule sentence;
      • every operator bullet: declare trustProxy before upgrading behind a proxy; the undetermined refusal and its log; CF-Connecting-IP no longer read; IPv4-mapped matching; nothing changes without ranges;
      • the last_login_ip sentence;
      • each per-package bullet.
    • content/docs/permissions/authentication.mdx: the D5 row.
    • The allowed_ip_ranges help text in auth.manifest.ts and its three translations.
    • The allowedIpRanges TSDoc: the fail-open sentence is gone.
  5. Gates. At ACCEPT, every check-run on this head is success or an expected skip, including TypeScript Type Check, Lint & Repo Gates, Test Core 1–6/6, Build Core and Governed Surface Queue Guard. Locally, 116 of 116 derived families exited 0.

  6. The dev's open questions, answered by the seat.

    • Where the rule lives: A, the plugin-auth subpath. Two consumers are measured, and A adds no new package edge. Revisit only if a third consumer appears.
    • How the raw-app mounts learn the peer: A, mirroring the transport's six-line reader. A drift fails closed: refusal plus the operator line.
    • How trustProxy reaches the gate: A, an explicit AuthManagerOptions.trustProxy passed by objectstack serve. This is the seam the dispatcher already uses.
    • ADR-0069 D5's mechanism cell and the spec's server.trustProxy describe, which names one consumer: both now under-describe the code. That follow-up is prose only (docs/adr and packages/spec, the spec seat's surface), filed once this lands.
    • Clause-② and semver: A, no and patch. Publication in this repo's contract sense is packages/spec, which this PR does not edit. The new plugin-auth subpath and options are implementation API. The narrowing (an undetermined address refused) is a runtime security behaviour the maintainer directed (filing gate ③). Triage graded it patch-eligible, and the changeset states the operator step plainly.
  7. Accepted deviations.

    • The claim surface is amended to cover:

      • packages/cli/src/commands/serve.ts, one option line;
      • packages/runtime/src/security/inbound-rate-limit.ts, pre-declared in the claim.

      Both files are domain:cli's, declared on [PM seat] domain:cli · seat 2 — 🟢 os-justin · session_01LmjU2x1faLZ39DVTFyVca6 #22648 (6285254652).

    • Also inside the amendment: service-settings' help text in four locales and authentication.mdx, prose this change made false.

    • The local runtime suite was narrowed to the 11 files that exercise the rule; CI runs the rest.

    • One merge of main.

    • Commit trailers are model-free, per AGENTS.md.

  8. Out-of-scope dispositions.

  9. Landing clause. Tier S: not governed, under the human-merge threshold. Next step: ready + auto-merge through the relay once every check on the current head is green or an expected skip. A push resets this reading.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 07:18
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 07:18
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 25e16ab Oct 20, 2026
43 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24739-auth-ip-allowlist-trust-rule branch October 20, 2026 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants