Skip to content

fix(plugin-auth): the vendored auth library's client-address source follows server.trustProxy - #24871

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24799-vendor-client-address-trust
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24799-vendor-client-address-trust

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #24799

Clause-②: no

What changed

The vendored auth library (better-auth 1.7.3) resolves a request's client address with its own reader, getIP in @better-auth/core utils/ip. That reader reads request headers only: it never sees the transport's peer address and does not know server.trustProxy. plugin-auth composed the library without configuring that reader, so the library used its default.

  • The library reads one platform-owned header. The auth instance's options now always carry advanced.ipAddress.ipAddressHeaders, which names the one header the platform owns (client-address-vendor.ts, vendorClientAddressOptions). No trustedProxies is set: the header carries one address the server already decided, never a chain for the library to walk.
  • handleRequest writes that header. AuthManager.handleRequest is the one door to the library's HTTP handler. It now rebuilds every request it hands over (withVendorClientAddress). Any copy of the header the caller sent is dropped. The address the client-address trust rule established is written in its place. That address comes from resolveClientAddress (@objectstack/plugin-auth/client-address, after fix(plugin-auth): the auth IP allow-list judges the client address the server established #24795), which is reused here and not copied. When no address was established, nothing is written, and the library is handed no address.
  • Why a header. The library has no resolver hook. Its option surface is a header list, a proxy-CIDR list for its own walk of a forwarded chain, and an off switch. The off switch also turns the library's rate limiter off, so it cannot express "no address where none exists". A header the platform owns is the one channel through which the rule's answer reaches the library.
  • The request is rebuilt from a clone, so the caller's own Request stays readable for the auth mount's 404 hand-on. A request that cannot be rebuilt is not handed to the library with a value its caller chose. There is no catch around the rebuild.

Landing: packages/plugins/plugin-auth/src/auth-manager.ts (the composed option and the handleRequest write) and a new client-address-vendor.ts beside client-address.ts. packages/spec/**, packages/runtime/** and auth-plugin.ts are untouched.

Reachability (the card's first step)

Measured on origin/main at 7c2eab929 with the installed better-auth 1.7.3 and @better-auth/core 1.7.3, through the real auth mounts (AuthPlugin.registerAuthRoutes on Hono, a real AuthManager).

Three things read the library's client address in a stock composition:

  1. the session row's ip_address, and through it the login / logout audit rows;
  2. the login_failed / account_locked audit address (signInClientFingerprint applies the same reader on purpose);
  3. the key of the library's own per-address rate limiter (ADR-0069 D2; on by default in production).

getIP has four callers in the installed library family: the rate limiter, createSession, the captcha plugin, and a re-export. plugin-auth composes no captcha.

In both the untrusted-proxy deployment and the trusted-proxy deployment, the address all three answered differed from the address the D5 allow-list judged and the D7 audit stamped. The detail is withheld under the disclosure ruling recorded on #9387.

M4 (does the peer address reach the library?): it does not. The library sees the Fetch Request only. The peer reaches handleRequest as its transport.remoteAddress argument.

Pins: one per branch, ablation-verified

client-address-vendor-trust-rule.test.ts uses the D5/D7 suite's fixture: real mounts, a real AuthManager, the peer on c.env.incoming. Each pin reads all three seams off one composition: the minted session's ip_address, the login_failed event's address, and every key the library's limiter consumed (recorded through rateLimitStorage). It also reads D7 last_login_ip for agreement. Every request also carries the platform-owned header, set by the caller. The refused sign-in asserts code + status (INVALID_EMAIL_OR_PASSWORD, 401).

  • behind a declared trusted proxy, it records and keys on the forwarded client address
  • without a declared trusted proxy, it records and keys on the connection peer, whatever the request headers say
  • when the server established no address, it is handed none, whatever the request headers say. This pin compares against a per-deployment control request that carries no header at all, never against a literal, because the library's own no-address answer depends on its environment.

Ablations ran on 2604658d1 (fix committed first) through scripts/ablation-replace.mjs. Each mutation was proven on disk (anchor count 1 to 0, blob changed), and each was restored to the HEAD blob with git diff HEAD empty. Each "10 passed" / "12 passed" counts the 10 tests the imported fixture module registers.

# forbidden behaviour put back result
A1 the composed ipAddress option removed (library default reader) 3 failed / 10 passed: all three branches red
A2 the caller's copy of the header not dropped 1 failed / 12 passed: no-address branch red
A3 the library handed the raw transport peer instead of the established address 1 failed / 12 passed: trusted-proxy branch red
A4 the forwarded header handed over without declared trust 2 failed / 11 passed: untrusted-proxy and no-address branches red
A5 the handleRequest write removed 3 failed / 10 passed: all three branches red

Tests and gates

All readings below are on head 9d43edafb, which merges origin/main at 9a06f0be0. That range includes #24846 and #24843, which edit other regions of auth-manager.ts.

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 exited 0: 171 files, 3273 passed, 10 skipped.
  • pnpm --filter @objectstack/plugin-auth typecheck exited 0: tsc on src and examples, and check:test-typecheck OK (no new debt).
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 70 commands. Each ran with its exit code captured before any pipe, and all 70 exited 0. --ran printed: 70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN.
  • Two existing suites moved with the behaviour:
    • auth-manager.test.ts: the advanced passthrough expectations now include the always-composed ipAddress.
    • auth-account-event-audit.test.ts: the fixture hands its client as the transport peer. It had supplied the client through a forwarded header on a deployment that declares no trusted proxy, which is the behaviour this card removes.
  • Repo-wide scans (pnpm lint and the full check:* farm) are left to CI.

Acceptance notes

  • Programmatic calls. A programmatic auth.api.* call is handed whatever headers its caller passes. Only withVendorClientAddress writes the header, which is documented at the constant and in the changeset. Measured in-repo, no programmatic call reaches an address reader with a client's headers. The calls are getSession (reads no address), boot-time signUpEmail (no headers) and requestPasswordReset (reads no address).
  • The library's own no-address answer. In production it is no session address and one shared rate-limit bucket, plus a one-time library warning whose remedy names library options that the platform composes itself. In development and test it is a loopback placeholder, and its limiter is off there by default. This is library-owned behaviour, unchanged.
  • IPv6. The library normalizes IPv6 addresses to their /64 before it keys or stores them. D7 last_login_ip stores the established address verbatim. This is pre-existing library behaviour, unchanged.
  • Peerless bridges. Filed as plugin-auth: seven in-process bridges call AuthManager.handleRequest without the request's transport, so the client-address trust rule cannot use the peer address there (detail withheld under the disclosure ruling) #24877 (security family; detail withheld under the disclosure ruling recorded on skills: five principle gaps the #9296 wave exposed in checklist-test / checklist-author / dogfood-verification #9387). The bullet was rewritten by the domain:services seat 2 at review.
  • Operator note (in the changeset). Behind a reverse proxy, declare server.trustProxy. Otherwise sessions and audit rows record the proxy's address, and the library's per-address rate limit counts every client behind the proxy as one client. This is the same posture the D5 allow-list and the runtime's inbound limiter already take.

Changeset: .changeset/24799-vendor-client-address-trust.md (@objectstack/plugin-auth: patch).


Generated by Claude Code

…ollows server.trustProxy

better-auth resolves a request's client address with its own header
reader, which knows neither the transport's peer address nor
server.trustProxy. The auth instance now tells it to read one
platform-owned header, and AuthManager.handleRequest -- the one door to
the library's HTTP handler -- writes that header from the address the
client-address trust rule established, after dropping any copy the
caller sent. With no address established, the library is handed none.

Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y
Co-authored-by: Claude <noreply@anthropic.com>
…s the transport peer

The fixture supplied its client through a forwarded header on a
deployment that declares no trusted proxy. The library's reader now
follows the client-address trust rule, which does not believe that
header, so the fixture hands the address the rule does believe: the
transport's peer address.

Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m 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 1 package(s): @objectstack/plugin-auth, touching 7 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/plugins/plugin-auth/src/client-address.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • 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))
  • content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class), handleRequest (symbol, a method of class AuthManager))
What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-auth/src/client-address.ts) — pages documenting those are invisible to this run
  • 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 — 16 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 3fbef41b4a573f751fda81a875a6dd3bc4dd5009 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 3fbef41b4a573f751fda81a875a6dd3bc4dd5009

⚠️ 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 3fbef41b4a573f751fda81a875a6dd3bc4dd5009 → 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 · #24799 · domain:services seat 2 · os-support-ai · session_01UmqzD97e14iws6hmVhgZ5y · 2026-10-20T12:44Z · head 9d43edafb9, first round, the review of record for claim 6288560527. Security family: the disclosure ruling recorded on #9387 (5323310017) applies to this record too.

  1. Form:
    • The first line is Fixes #24799; the seat's grep of the body finds no other closing keyword beside an issue number.
    • Clause-②: no at line start in the body, in the changeset and in the claim. The seat confirmed it: the new module client-address-vendor.ts is imported only by auth-manager.ts, is not re-exported from index.ts, and is not a subpath of the package's exports map (., ./rate-limit-storage, ./client-address). Dropping a caller's copy of the platform-owned header narrows what handleRequest accepts; it widens nothing.
    • One patch changeset for @objectstack/plugin-auth.
    • 7 files, +376/−21. No packages/spec/**, no packages/runtime/**, and nothing in auth-plugin.ts.
    • Path face read with the files endpoint on this head: check-governed-merges --pr 24871 reports 0 of 7 paths governed, 397 changed lines. Ordinary queue landing.
  2. Premise and fix:
    • Reach measured before the fix, the card's first step. Three library seams read the address in a stock composition: the session row's ip_address with the login/logout audit rows copied from it; the login_failed / account_locked audit address; and the library's per-address rate-limiter key. The library never sees the transport peer. Detail is withheld per skills: five principle gaps the #9296 wave exposed in checklist-test / checklist-author / dogfood-verification #9387.
    • The fix reuses the one rule and copies nothing. The auth instance always composes advanced.ipAddress.ipAddressHeaders naming one platform-owned header, with no trustedProxies. handleRequest, the one door to the library's handler, rebuilds each request from a clone: the caller's copy of that header is dropped, and the address resolveClientAddress established is written in its place. No address established means no header is written. The rebuild has no catch, so a request that cannot be rebuilt never reaches the library with a caller-chosen value. The seat read client-address-vendor.ts (84 lines) and the auth-manager.ts hunks whole.
    • Pins and ablation. One real-mount pin per branch of the rule (declared trusted proxy, no declared trust, no address source). Each reads all three seams plus D7 last_login_ip; the refused sign-in asserts code and status. Five ablations went red on exactly the predicted branches (A1–A5, table in the PR body), each restored to the HEAD blob.
  3. Gates:
    • CI on 9d43edafb9, read by the seat after the last suite completed: 48 check-runs, 39 success, 9 skipped, 0 failed. Every skip is on the roster in scripts/pm/check-expected-skips.mjs (read from the file): Auto Label and Check PR Size (the re-runs the body edit triggered; their first runs passed), Dogfood Verify CLI, Temporal Conformance, Console Pin Gate, the Dogfood Regression Gate matrix, Build Docs, Packed-tarball smoke, and the 18.x upgrade harness.
    • Local: 70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN.
    • Package suite: plugin-auth 171 files, 3273 passed and 10 skipped; typecheck exit 0, with no new test-typecheck debt.
    • Docs drift (6290753831): authentication.mdx:600 already states the trust rule for the allow-list and last_login_ip. No named page says the library's own records follow any other rule, so no page is now false. The PR does not extend that row to the session address and the library's limiter; that is a possible docs follow-up, not a defect.
  4. Accepted deviations:
  5. Out of scope:
  6. Landing: ordinary queue landing. Every check on this head is green or an expected skip, so the seat flips it (pr_ready + automerge_enable) on this record. Seat 1's Impersonating a user who must change their password strands the admin on that user's change-password screen #24762 edits another region of auth-manager.ts; whichever lands second merges main.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 12:45
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 12:45
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit ad8cf92 Oct 20, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24799-vendor-client-address-trust branch October 20, 2026 13:13
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

2 participants