Repository navigation
Multi-tenant mode: per-tenant account pools + path-prefix tenant keys - #36
Conversation
Tenants live in <state-dir>/tenants.json with per-tenant state dirs under <state-dir>/tenants/<id>/ mirroring the single-tenant layout. Keys are srt_<32 hex>; only SHA-256 hashes plus a display prefix are stored, so a key is shown once at creation. Reads are cached on file modtime+size so per-request resolution is one stat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MultiTenant wraps the legacy handler. A tenant key arrives as a /t/<key>/... path prefix (agent CLIs can only override base URLs) or as an Authorization Bearer / x-api-key value; the wrapper resolves it, strips the prefix, and dispatches to a lazily built per-tenant Server whose codex/claude stores, session store, scheduler, active sessions, read cache, and transcripts all live under the tenant dir, so isolation falls out of instantiating the existing single-tenant machinery per tenant. Unknown or revoked keys get 401; keyless requests keep today's behavior. Global admin gains /_subrouter/tenants CRUD, and tenant keys authorize a small read-only /_subrouter allowlist plus whoami on the tenant base URL. A global loopback reload-accounts also reloads instantiated tenants so uploads hot-reload. Tests cover two-tenant pool isolation over httptest, bad-key 401s, key rotation and revocation through the admin API, sticky sessions scoped per tenant, legacy fallthrough, and admin auth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
serve wires the MultiTenant wrapper over the state dir registry and adds --multi-tenant to reject unknown srt_ header keys before the first tenant exists. sr tenant create/list/key create/key revoke talk to a named or default server's admin endpoints and fall back to the local state dir on the server host. sr server add --tenant-key stores the key on a server entry (preserved across metadata updates like the admin token); with a key set, codex/claude base URLs gain the /t/<key> prefix, Claude proxy env uses the key as ANTHROPIC_AUTH_TOKEN, _subrouter reads go through the tenant-scoped endpoints, and account/claude uploads land in tenants/<id>/ on the server, resolved via the tenant whoami endpoint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds multi-tenant support through a hashed-key tenant registry, tenant-aware HTTP routing and admin APIs, ChangesMulti-tenant mode implementation
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MultiTenant
participant Registry
participant TenantServer
Client->>MultiTenant: Send tenant key in path or header
MultiTenant->>Registry: Resolve key
Registry-->>MultiTenant: Tenant or unknown
MultiTenant->>TenantServer: Forward to tenant-scoped handler
TenantServer-->>Client: Return response
sequenceDiagram
participant User
participant srRunner
participant AdminAPI
participant Registry
User->>srRunner: Run tenant command
alt remote mode
srRunner->>AdminAPI: Execute tenant operation
AdminAPI-->>srRunner: Return result
else local mode
srRunner->>Registry: Execute tenant operation
Registry-->>srRunner: Return result
end
srRunner-->>User: Print result or one-time key
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a48fc3410
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| base := strings.TrimRight(server.URL, "/") | ||
| if key := strings.TrimSpace(server.TenantKey); key != "" { | ||
| return base + "/t/" + key |
There was a problem hiding this comment.
When a tenant-scoped server is stored with a /v1 suffix (which codexBaseURLForServer accepts and the new test covers for data-plane URLs), this builds control URLs like http://host:31415/v1/t/<key>/_subrouter/.... The multi-tenant router only recognizes tenant prefixes at /t/<key>/..., so sr server status and upload flows that call serverTenantID/whoami for that entry get routed to the upstream or fail instead of reaching the tenant control handler. Normalize server.URL with the same root-stripping logic before appending /t/<key>.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6718aa5: serverControlBaseURL (and tenantAdminRequest) now normalize the stored URL through codexProxyRootURL before appending /t/, so /v1- or /backend-api-suffixed entries reach control endpoints at the server root. Covered in TestTenantScopedBaseURLs.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
internal/tenant/tenant_test.go (1)
38-44: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a regression test for short revoke refs.
The current revoke test only covers the exact stored prefix. Add a case proving
RevokeKey(created.ID, "srt_")does not revoke all keys.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tenant/tenant_test.go` around lines 38 - 44, The existing revoke coverage only verifies the exact stored prefix, so extend the tenant test to cover short revoke refs as a regression case. In the same test around RevokeKey and Resolve, add a check that calling registry.RevokeKey(created.ID, "srt_") does not revoke all keys and that the key still resolves afterward. Use the existing test flow and registry.RevokeKey/registry.Resolve symbols to keep the assertion close to the current revoke behavior.internal/proxy/multitenant_test.go (1)
25-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert header tenant keys are not forwarded upstream.
The
X-Api-Keyrouting test only validates the selectedAuthorizationheader. Capture upstreamX-Api-Keytoo and assert it is empty or provider-replaced, so ansrt_...leak cannot pass unnoticed.Also applies to: 118-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/proxy/multitenant_test.go` around lines 25 - 29, The routing test only checks the upstream Authorization header, so it can miss leaking tenant header keys. In the multitenant test setup around the upstream handler, capture the incoming X-Api-Key alongside the existing Authorization value, and assert it is empty or has been replaced by the provider before the request reaches upstream. Apply the same update to the related test block referenced in the diff so any srt_... forwarding issue is caught by both cases.cmd/subrouter/sr_tenant_test.go (1)
115-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the remote admin-API branch too.
Local create/list/revoke is tested, but
tenantAdminRequest/resolveRemoteTenant(the--serverpath building/_subrouter/tenants...requests) has no test coverage here.An
httptest.Serverstub returning the admin JSON shapes (tenantAdminView,{tenant, key}) would exercisetenantCreate/tenantList/tenantKeyCreate/tenantKeyRevokewithremote=true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/subrouter/sr_tenant_test.go` around lines 115 - 153, Add test coverage for the remote admin-API path in sr_tenant_test by exercising the --server flow instead of only the local store path. Create an httptest.Server stub that serves the admin JSON responses used by tenantAdminRequest and resolveRemoteTenant, then drive srRunner through tenantCreate, tenantList, tenantKeyCreate, and tenantKeyRevoke with remote=true so the /_subrouter/tenants requests are covered. Use the existing tenantAdminView and key response shapes in the test to verify the remote branch behaves the same as the local one.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/subrouter/sr_server.go`:
- Around line 27-38: serverControlBaseURL currently only strips trailing
slashes, which can leave a stored base path like /v1 in place and build
incorrect tenant-scoped control URLs. Update serverControlBaseURL to use the
same normalization as serverProxyRootURL by delegating to
serverProxyRootURL(server) or codexProxyRootURL(server.URL), then append
/t/<key> only after that normalized root so whoami, accounts, and status still
resolve correctly for tenant entries.
In `@cmd/subrouter/sr_tenant.go`:
- Around line 241-276: In tenantKeyRevoke, both the remote and local revoke
flows currently treat a zero-match result as success; update the logic after
tenantAdminRequest and registry.RevokeKey so that a revoked count of 0 returns
an error instead of printing a success message. Use the existing identifiers
tenantKeyRevoke, tenantAdminRequest, registry.RevokeKey, and the
revoked/result.Revoked value to locate the two branches, and apply the same
zero-matches check in both paths.
In `@internal/proxy/multitenant.go`:
- Around line 139-143: The cloned request in multitenant dispatch still carries
tenant credentials, so scrub them before calling handler.ServeHTTP. In
internal/proxy/multitenant.go, update the flow around r.Clone, cloneURL, and the
inner proxy handoff to remove any tenant-formatted values from Authorization and
X-Api-Key after tenant resolution, using a helper like
stripTenantCredentialHeaders so downstream proxy/logging paths never see the
srt_... credential.
In `@internal/tenant/tenant.go`:
- Around line 194-223: `Registry` operations are only protected by `r.mu`, which
does not prevent concurrent processes from clobbering the same `tenants.json`
state. Add an interprocess file lock around the full load/mutate/save flow in
`Registry.Create`, `Registry.CreateKey`, and `Registry.RevokeKey`, so the
critical sections stay serialized across processes. Keep the lock held from
`r.load()` through `r.save()` (including tenant/key validation and updates) to
avoid stale reads and lost writes.
- Line 279: The revocation match in tenant key lookup is too loose because the
`strings.HasPrefix(key.Prefix, keyRef)` check in the tenant key matching logic
can revoke unrelated keys for short refs. Update the matching condition in the
tenant key revocation path to require an exact match against the stored display
prefix, while still allowing a match on the full plaintext key hash; use the
relevant tenant key matching/revocation helper around the `key.Hash` and
`key.Prefix` comparison to locate the change.
---
Nitpick comments:
In `@cmd/subrouter/sr_tenant_test.go`:
- Around line 115-153: Add test coverage for the remote admin-API path in
sr_tenant_test by exercising the --server flow instead of only the local store
path. Create an httptest.Server stub that serves the admin JSON responses used
by tenantAdminRequest and resolveRemoteTenant, then drive srRunner through
tenantCreate, tenantList, tenantKeyCreate, and tenantKeyRevoke with remote=true
so the /_subrouter/tenants requests are covered. Use the existing
tenantAdminView and key response shapes in the test to verify the remote branch
behaves the same as the local one.
In `@internal/proxy/multitenant_test.go`:
- Around line 25-29: The routing test only checks the upstream Authorization
header, so it can miss leaking tenant header keys. In the multitenant test setup
around the upstream handler, capture the incoming X-Api-Key alongside the
existing Authorization value, and assert it is empty or has been replaced by the
provider before the request reaches upstream. Apply the same update to the
related test block referenced in the diff so any srt_... forwarding issue is
caught by both cases.
In `@internal/tenant/tenant_test.go`:
- Around line 38-44: The existing revoke coverage only verifies the exact stored
prefix, so extend the tenant test to cover short revoke refs as a regression
case. In the same test around RevokeKey and Resolve, add a check that calling
registry.RevokeKey(created.ID, "srt_") does not revoke all keys and that the key
still resolves afterward. Use the existing test flow and
registry.RevokeKey/registry.Resolve symbols to keep the assertion close to the
current revoke behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aafd8a1b-e788-4694-b807-937708014922
📒 Files selected for processing (14)
README.mdcmd/subrouter/codex.gocmd/subrouter/main.gocmd/subrouter/main_test.gocmd/subrouter/sr.gocmd/subrouter/sr_claude_upload.gocmd/subrouter/sr_claude_upload_test.gocmd/subrouter/sr_server.gocmd/subrouter/sr_tenant.gocmd/subrouter/sr_tenant_test.gointernal/proxy/multitenant.gointernal/proxy/multitenant_test.gointernal/tenant/tenant.gointernal/tenant/tenant_test.go
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 14 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Review fixes: RevokeKey now matches the stored display prefix exactly (or the full plaintext key's hash), so a loose ref like srt_ cannot wipe every key on a tenant. Create/CreateKey/RevokeKey take an exclusive flock on tenants.json.lock (same pattern as accounts.lockStoredAccount) and re-read the file under the lock, so the server's admin API and a local sr tenant run on the same host cannot lose each other's writes or resurrect a revoked key. load/save hand out deep copies of the cached registry so a failed save cannot leave mutated key state in memory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review fixes: after tenant resolution the wrapper strips key-shaped credentials from Authorization/X-Api-Key on the scoped request. setAccountAuthHeaders already overwrites Authorization, but Codex-routed forwarding leaves X-Api-Key untouched, so a tenant key parked there would have been forwarded upstream; regression test asserts the upstream never sees the key. The tenant-pool side of a global reload-accounts POST is now gated on the same loopback check as the endpoint itself, so a rejected non-loopback caller triggers no reload work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review fix (Codex P2, CodeRabbit, cubic): serverControlBaseURL and tenantAdminRequest now strip a stored /v1 or /backend-api suffix via codexProxyRootURL before appending /t/<key> or /_subrouter/..., so a server entry saved with a Codex-style URL still reaches whoami, status, and tenant admin endpoints at the server root instead of building /v1/t/<key>/_subrouter/... paths the router never matches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review fixes (cubic): switching a Claude profile from a tenant-scoped server back to a plain one now replaces a leftover srt_ token in ANTHROPIC_AUTH_TOKEN with the dummy value instead of sending the old tenant key to the new server; unrelated custom tokens stay untouched. Tenant creation output shows the plaintext key on one line only, with a placeholder in the base-URL hint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 515d9032b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| func (r srRunner) fetchServerUsageStatuses(ctx context.Context, server srServerConfig) ([]remoteServerUsageStatus, bool, error) { | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.URL+"/_subrouter/usage-status", nil) | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, serverControlBaseURL(server)+"/_subrouter/usage-status", nil) |
There was a problem hiding this comment.
When the selected server has a TenantKey, pickSmartResetCandidateRemote calls this tenant-scoped usage fetch and may choose an account from that tenant, but the actual reset path in resetRemoteSweep still POSTs to server.URL + "/_subrouter/rate-limit-reset" on the global handler. In tenant-scoped defaults, sr reset or sr reset <email> can therefore fail against an empty global pool or, worse, redeem the reset credit for a same-email account in the global pool rather than the tenant; route or disable reset consistently for tenant-scoped servers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
3 issues found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/tenant/lock_unix.go">
<violation number="1" location="internal/tenant/lock_unix.go:26">
P1: A blocked flock wait here can hold the in-process registry mutex and pause tenant auth/resolve work until another process releases the lock. Using a non-blocking lock attempt (and returning/retrying) avoids freezing request-path lookups during cross-process contention.</violation>
</file>
<file name="internal/tenant/lock_windows.go">
<violation number="1" location="internal/tenant/lock_windows.go:15">
P1: lockRegistry on Windows opens the .lock file but never acquires an exclusive lock. Without syscall.LockFileEx (the Windows equivalent of flock(LOCK_EX)), concurrent writes from the admin API and CLI can corrupt tenants.json. Add syscall.LockFileEx(syscall.Handle(file.Fd()), syscall.LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, &syscall.Overlapped{}) after OpenFile, and syscall.UnlockFileEx in Close before file.Close.</violation>
</file>
<file name="cmd/subrouter/sr_server.go">
<violation number="1" location="cmd/subrouter/sr_server.go:34">
P2: The `serverControlBaseURL` helper now correctly routes control requests through the tenant path prefix, but the rate-limit reset flow (`resetRemoteSweep` / `pickSmartResetCandidateRemote`) appears to POST directly to `server.URL + "/_subrouter/rate-limit-reset"` without going through `serverControlBaseURL`. For tenant-scoped server entries this would hit the global pool rather than the tenant pool, potentially failing against an empty global pool or redeeming the reset credit for a same-email account in the wrong pool. The reset path should use `serverControlBaseURL(server)` to ensure it's routed to the correct tenant scope.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX); err != nil { |
There was a problem hiding this comment.
P1: A blocked flock wait here can hold the in-process registry mutex and pause tenant auth/resolve work until another process releases the lock. Using a non-blocking lock attempt (and returning/retrying) avoids freezing request-path lookups during cross-process contention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/tenant/lock_unix.go, line 26:
<comment>A blocked flock wait here can hold the in-process registry mutex and pause tenant auth/resolve work until another process releases the lock. Using a non-blocking lock attempt (and returning/retrying) avoids freezing request-path lookups during cross-process contention.</comment>
<file context>
@@ -0,0 +1,40 @@
+ if err != nil {
+ return nil, err
+ }
+ if err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX); err != nil {
+ _ = file.Close()
+ return nil, err
</file context>
|
|
||
| // lockRegistry mirrors the unix flock variant; like | ||
| // accounts.lockStoredAccount, Windows gets a best-effort lock file only. | ||
| func (r *Registry) lockRegistry() (*registryLock, error) { |
There was a problem hiding this comment.
P1: lockRegistry on Windows opens the .lock file but never acquires an exclusive lock. Without syscall.LockFileEx (the Windows equivalent of flock(LOCK_EX)), concurrent writes from the admin API and CLI can corrupt tenants.json. Add syscall.LockFileEx(syscall.Handle(file.Fd()), syscall.LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, &syscall.Overlapped{}) after OpenFile, and syscall.UnlockFileEx in Close before file.Close.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/tenant/lock_windows.go, line 15:
<comment>lockRegistry on Windows opens the .lock file but never acquires an exclusive lock. Without syscall.LockFileEx (the Windows equivalent of flock(LOCK_EX)), concurrent writes from the admin API and CLI can corrupt tenants.json. Add syscall.LockFileEx(syscall.Handle(file.Fd()), syscall.LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, &syscall.Overlapped{}) after OpenFile, and syscall.UnlockFileEx in Close before file.Close.</comment>
<file context>
@@ -0,0 +1,28 @@
+
+// lockRegistry mirrors the unix flock variant; like
+// accounts.lockStoredAccount, Windows gets a best-effort lock file only.
+func (r *Registry) lockRegistry() (*registryLock, error) {
+ if err := os.MkdirAll(r.stateDir, 0o700); err != nil {
+ return nil, err
</file context>
| // same root-stripping as data-plane URLs, so an entry saved with a /v1 or | ||
| // /backend-api suffix still reaches control endpoints at the server root. | ||
| func serverControlBaseURL(server srServerConfig) string { | ||
| base := codexProxyRootURL(server.URL) |
There was a problem hiding this comment.
P2: The serverControlBaseURL helper now correctly routes control requests through the tenant path prefix, but the rate-limit reset flow (resetRemoteSweep / pickSmartResetCandidateRemote) appears to POST directly to server.URL + "/_subrouter/rate-limit-reset" without going through serverControlBaseURL. For tenant-scoped server entries this would hit the global pool rather than the tenant pool, potentially failing against an empty global pool or redeeming the reset credit for a same-email account in the wrong pool. The reset path should use serverControlBaseURL(server) to ensure it's routed to the correct tenant scope.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/subrouter/sr_server.go, line 34:
<comment>The `serverControlBaseURL` helper now correctly routes control requests through the tenant path prefix, but the rate-limit reset flow (`resetRemoteSweep` / `pickSmartResetCandidateRemote`) appears to POST directly to `server.URL + "/_subrouter/rate-limit-reset"` without going through `serverControlBaseURL`. For tenant-scoped server entries this would hit the global pool rather than the tenant pool, potentially failing against an empty global pool or redeeming the reset credit for a same-email account in the wrong pool. The reset path should use `serverControlBaseURL(server)` to ensure it's routed to the correct tenant scope.</comment>
<file context>
@@ -27,9 +27,11 @@ import (
+// /backend-api suffix still reaches control endpoints at the server root.
func serverControlBaseURL(server srServerConfig) string {
- base := strings.TrimRight(server.URL, "/")
+ base := codexProxyRootURL(server.URL)
if key := strings.TrimSpace(server.TenantKey); key != "" {
return base + "/t/" + key
</file context>
138 commits of drift since this branch was opened. Twelve conflict hunks across five files, resolved as follows. Additive on both sides, kept both: the tenant/tenants and team command names, the --multi-tenant flag alongside the Bedrock and --cloud-config flags, the tenant CLI case alongside daemon/setup, and the tenant and broker imports. Combined rather than chosen: main made remote deploy commands use dynamic "$sr_owner"/"$sr_group" instead of a hardcoded subrouter:subrouter, while this branch made the same commands tenant-aware via remoteStatePath(stateSubdir, …). Both properties are kept, with shell-quoted paths. Repointed: main moved selectacct and session out of internal/, so internal/proxy/multitenant.go and its test now import subrouter/selectacct and subrouter/session. Usage text takes main's serve and supervise lines with --multi-tenant re-inserted. The sr passthrough case takes main's longer verb list with tenant/tenants added. go build ./..., go vet ./... and go test ./... green.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/subrouter/sr.go (1)
1842-1853: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace deprecated
strings.Title.Provider values come from
account.Provider, and the current constants are all lowercase ASCII. Usestrings.ToUpper(string(provider))for the fallback, and update the nearby identicalstrings.Title(string(usageProvider(row)))label path if it should not emit"Codex accounts"for providers likekimi/zai.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/subrouter/sr.go` around lines 1842 - 1853, Replace the deprecated strings.Title fallback in providerCountNoun with strings.ToUpper(string(provider)). Also update the nearby label path using strings.Title(string(usageProvider(row))) to use uppercase provider values, preserving the existing account-label formatting for all providers.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/subrouter/sr_server.go`:
- Line 1511: Reverse the stat fallback order in the owner/group shell commands:
update cmd/subrouter/sr_server.go lines 1511 and 1536 and
cmd/subrouter/sr_claude_upload.go line 182 to try GNU stat -c '%U'/'%G' first,
then BSD stat -f '%Su'/'%Sg'. Update cmd/subrouter/sr_claude_upload_test.go
lines 54-56 to assert the GNU stat -c prefix.
In `@internal/proxy/multitenant.go`:
- Around line 306-377: Update handleTenantAdmin to fail closed when the admin
token is unset, rejecting the request before authorizeAdmin can allow it; retain
authorizeAdmin for validating configured tokens and preserve the existing
unauthorized response.
---
Nitpick comments:
In `@cmd/subrouter/sr.go`:
- Around line 1842-1853: Replace the deprecated strings.Title fallback in
providerCountNoun with strings.ToUpper(string(provider)). Also update the nearby
label path using strings.Title(string(usageProvider(row))) to use uppercase
provider values, preserving the existing account-label formatting for all
providers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b7a90e9-2214-4a8e-bbfa-581f7aa5137f
📒 Files selected for processing (14)
cmd/subrouter/codex.gocmd/subrouter/main.gocmd/subrouter/main_test.gocmd/subrouter/sr.gocmd/subrouter/sr_claude_upload.gocmd/subrouter/sr_claude_upload_test.gocmd/subrouter/sr_server.gocmd/subrouter/sr_tenant.gocmd/subrouter/sr_tenant_test.gointernal/proxy/multitenant.gointernal/proxy/multitenant_test.gointernal/tenant/lock_unix.gointernal/tenant/lock_windows.gointernal/tenant/tenant.go
| @@ -1417,10 +1511,10 @@ func (r srRunner) uploadServerAccount(ctx context.Context, server srServerConfig | |||
| "sr_owner=subrouter; sr_group=subrouter; if [ -e /var/lib/subrouter ]; then sr_owner=$(stat -f '%Su' /var/lib/subrouter 2>/dev/null || stat -c '%U' /var/lib/subrouter); sr_group=$(stat -f '%Sg' /var/lib/subrouter 2>/dev/null || stat -c '%G' /var/lib/subrouter); elif id -u _subrouter >/dev/null 2>&1; then sr_owner=_subrouter; sr_group=_subrouter; fi", | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'sr_server.go|sr_claude_upload.go|sr_claude_upload_test.go' . || true
echo "== relevant lines =="
for f in cmd/subrouter/sr_server.go cmd/subrouter/sr_claude_upload.go cmd/subrouter/sr_claude_upload_test.go; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
rg -n "stat -f|stat -c|sr_owner|sr_group|install|chown" "$f" -C 2 || true
else
echo "missing $f"
fi
done
echo "== behavioral probe with system stat semantics if present =="
if command -v stat >/dev/null 2>&1 && command -v useradd >/dev/null 2>&1 && command -v chown >/dev/null 2>&1; then
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
touch "$tmpdir/x"
echo "gnu/fmt -f '%Su': " "$(stat -f '%Su' "$tmpdir/x" 2>&1 || true)"
echo "gnu/fmt -c '%U': " "$(stat -c '%U' "$tmpdir/x" 2>&1 || true)"
else
echo "not enough system utilities to probe stat semantics"
fiRepository: manaflow-ai/subrouter
Length of output: 8242
🌐 Web query:
GNU stat format -c %U %G owner group vs BSD stat -f %Su %Sg owner group
💡 Result:
The stat command exists in two primary, incompatible implementations: the GNU Coreutils version (commonly found on Linux) and the BSD version (found on macOS, FreeBSD, NetBSD, and OpenBSD). Because these utilities have different codebases and origin histories, their format specifiers for retrieving file owner and group information are not interchangeable [1][2]. GNU stat (Coreutils) The GNU implementation uses the following specifiers to print the owner and group names [3][4]: %U: User name of the file owner [3][5]. %G: Group name of the file owner [3][5]. BSD stat The BSD implementation (including macOS) uses a different syntax, where the 'S' prefix is generally used to indicate that the field should be formatted as a string rather than a raw ID [6][7]: %Su: User name of the file owner [6][7]. %Sg: Group name of the file owner [6][7]. Summary Table Feature | GNU stat (Linux) | BSD stat (macOS/FreeBSD/etc.) --- | --- | --- Owner Name | %U [3] | %Su [6] Group Name | %G [3] | %Sg [6] When writing portable shell scripts, you cannot rely on these specifiers working across both platforms [1][2]. If your script must run on both, you may need to detect the OS or the version of the stat command being used [2][5].
Citations:
- 1: https://linuxcommandlibrary.com/man/stat
- 2: https://unix.stackexchange.com/questions/175325/output-of-stat-on-osx
- 3: https://manpages.debian.org/unstable/coreutils/stat.1.en.html
- 4: https://linux.die.net/man/1/stat
- 5: https://linuxize.com/post/stat-command-in-linux/
- 6: https://man.freebsd.org/stat
- 7: https://man.netbsd.org/stat.1
🌐 Web query:
GNU stat %S format specifier block size string %Su invalid on GNU stat
💡 Result:
The %Su format specifier is invalid in GNU stat because it is a feature specific to BSD/macOS stat, not GNU coreutils stat [1]. GNU stat and BSD stat are different implementations of the stat utility with distinct syntax and supported format specifiers [1]. The confusion typically arises when scripts written for macOS (which uses BSD stat) are run on Linux systems (which use GNU stat) [1]. Key differences include: 1. BSD/macOS stat: Uses the -f flag to specify a format string and supports specifiers like %Su (User name of owner) and %Sg (Group name of owner) [1][2][3]. 2. GNU stat: Uses the -c (or --format) flag to specify a format string and uses %U for user name and %G for group name [4][5][1]. If you see an error indicating that %Su is invalid or that GNU stat cannot find a file named %Su, it is because GNU stat is interpreting the literal string %Su as a filename rather than a format directive [1]. To fix this in a script, you should detect the type of stat being used or use compatible alternatives [1]. A common way to check for GNU stat is to test if it supports the -c flag [1]. For your reference, here are the correct GNU stat equivalents for the common BSD %S-prefixed specifiers: - BSD %Su (User name): Use GNU %U [4][1] - BSD %Sg (Group name): Use GNU %G [4] - BSD %Sp (Permissions): Use GNU %A (human readable) or %a (octal) [4] - BSD %Sm (Modification time): Use GNU %y [4] Note: While GNU stat does support the %S format specifier, it is only valid when using the --file-system flag, where it represents the fundamental block size of the file system [5][6][7]. It does not function as a prefix for other specifiers like %Su [4][5].
Citations:
- 1: BSD
statand GNUstattake different options postmodern/ruby-install#63 - 2: https://manned.org/man/freebsd-12.4/stat.1
- 3: https://leancrew.com/all-this/man/man1/stat.html
- 4: https://devdoc.net/linux/coreutils-8.28/stat-invocation.html
- 5: https://man7.org/linux/man-pages/man1/stat.1.html
- 6: https://manpages.debian.org/bullseye/coreutils/stat.1.en.html
- 7: https://man.archlinux.org/man/core/coreutils/stat.1.en
Use GNU stat formatting before the BSD fallback.
On GNU/Linux, stat -f '%Su' does not print the file owner; use stat -c '%U' / '%G' first, then fall back to stat -f '%Su' / '%Sg' for BSD hosts. Otherwise install -o receives data from a filesystem-size format rather than the owner name.
cmd/subrouter/sr_server.go#L1511andcmd/subrouter/sr_server.go#L1536cmd/subrouter/sr_claude_upload.go#L182- Update
cmd/subrouter/sr_claude_upload_test.go#L54-56to assert the GNUstat -cprefix.
📍 Affects 3 files
cmd/subrouter/sr_server.go#L1511-L1511(this comment)cmd/subrouter/sr_server.go#L1536-L1536cmd/subrouter/sr_claude_upload.go#L182-L182cmd/subrouter/sr_claude_upload_test.go#L54-L56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/subrouter/sr_server.go` at line 1511, Reverse the stat fallback order in
the owner/group shell commands: update cmd/subrouter/sr_server.go lines 1511 and
1536 and cmd/subrouter/sr_claude_upload.go line 182 to try GNU stat -c '%U'/'%G'
first, then BSD stat -f '%Su'/'%Sg'. Update
cmd/subrouter/sr_claude_upload_test.go lines 54-56 to assert the GNU stat -c
prefix.
| // handleTenantAdmin serves the global-admin tenant CRUD: | ||
| // | ||
| // GET /_subrouter/tenants list tenants (key prefixes only) | ||
| // POST /_subrouter/tenants {"name":..} create tenant, returns key once | ||
| // POST /_subrouter/tenants/<id>/keys mint an extra key, returns it once | ||
| // DELETE /_subrouter/tenants/<id>/keys/<prefix> revoke keys matching prefix | ||
| func (m *MultiTenant) handleTenantAdmin(w http.ResponseWriter, r *http.Request) { | ||
| if !m.Base.authorizeAdmin(r) { | ||
| http.Error(w, "admin token required", http.StatusUnauthorized) | ||
| return | ||
| } | ||
| rest := strings.TrimPrefix(r.URL.Path, "/_subrouter/tenants") | ||
| rest = strings.Trim(rest, "/") | ||
| switch { | ||
| case rest == "": | ||
| switch r.Method { | ||
| case http.MethodGet: | ||
| tenants, err := m.Registry.List() | ||
| if err != nil { | ||
| http.Error(w, err.Error(), http.StatusInternalServerError) | ||
| return | ||
| } | ||
| views := make([]tenantView, 0, len(tenants)) | ||
| for _, t := range tenants { | ||
| views = append(views, viewOf(t)) | ||
| } | ||
| writeJSON(w, views) | ||
| case http.MethodPost: | ||
| var payload struct { | ||
| Name string `json:"name"` | ||
| } | ||
| if err := json.NewDecoder(io.LimitReader(r.Body, tenantAdminMaxBodyBytes)).Decode(&payload); err != nil { | ||
| http.Error(w, "invalid JSON body", http.StatusBadRequest) | ||
| return | ||
| } | ||
| created, key, err := m.Registry.Create(payload.Name) | ||
| if err != nil { | ||
| http.Error(w, err.Error(), http.StatusBadRequest) | ||
| return | ||
| } | ||
| writeJSON(w, map[string]any{"tenant": viewOf(created), "key": key}) | ||
| default: | ||
| http.Error(w, "method not allowed", http.StatusMethodNotAllowed) | ||
| } | ||
| default: | ||
| parts := strings.Split(rest, "/") | ||
| if len(parts) >= 2 && parts[1] == "keys" { | ||
| tenantID := parts[0] | ||
| switch { | ||
| case len(parts) == 2 && r.Method == http.MethodPost: | ||
| updated, key, err := m.Registry.CreateKey(tenantID) | ||
| if err != nil { | ||
| http.Error(w, err.Error(), http.StatusNotFound) | ||
| return | ||
| } | ||
| writeJSON(w, map[string]any{"tenant": viewOf(updated), "key": key}) | ||
| return | ||
| case len(parts) == 3 && r.Method == http.MethodDelete: | ||
| revoked, err := m.Registry.RevokeKey(tenantID, parts[2]) | ||
| if err != nil { | ||
| http.Error(w, err.Error(), http.StatusNotFound) | ||
| return | ||
| } | ||
| writeJSON(w, map[string]any{"ok": true, "revoked": revoked}) | ||
| return | ||
| } | ||
| } | ||
| http.NotFound(w, r) | ||
| } | ||
| } | ||
|
|
||
| const tenantAdminMaxBodyBytes = 1 << 16 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B2 -A15 'func.*authorizeAdmin' --type=goRepository: manaflow-ai/subrouter
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -e
printf 'go files:\n'
git ls-files '*.go' | sed -n '1,120p'
printf '\nproxy-related files:\n'
git ls-files | rg '(^|/)proxy[^/]*(\.(go|md)|$)|internal/proxy|multitenant' || true
printf '\nSearch authorizeAdmin/authAdmin/admin token symbols:\n'
rg -n 'authorizeAdmin|Auth(enticate|oriz)|AdminToken|admin-token|SUBROUTER_ADMIN_TOKEN|multi-tenant' --glob '*.go' .Repository: manaflow-ai/subrouter
Length of output: 30616
🏁 Script executed:
#!/bin/bash
set -e
printf 'internal/proxy/proxy.go authorizeAdmin section:\n'
sed -n '1680,1725p' internal/proxy/proxy.go | cat -n
printf '\nlocalProxyAuthorized:\n'
sed -n '2058,2075p' internal/proxy/proxy.go | cat -n
printf '\ninternal/proxy/multitenant.go relevant code:\n'
sed -n '300,360p' internal/proxy/multitenant.go | cat -n
printf '\nadmin token behavior tests:\n'
sed -n '120,155p' internal/proxy/lifecycle_test.go | cat -n
sed -n '294,350p' internal/proxy/multitenant_test.go | cat -nRepository: manaflow-ai/subrouter
Length of output: 9022
🏁 Script executed:
#!/bin/bash
set -e
printf 'internal/proxy/proxy.go authorizeAdmin section:\n'
sed -n '1680,Repository: manaflow-ai/subrouter
Length of output: 267
🏁 Script executed:
#!/bin/bash
set -e
printf 'isLoopbackRemote definition:\n'
rg -n -B3 -A10 'func isLoopbackRemote|localProxyAuthorized' internal/proxy/proxy.go
printf '\nBehavior probe for authorizeAdmin predicate if AdminToken is empty:\n'
python3 - <<'PY'
from urllib.parse import urlparse
import socket
def is_loopback_remote(remote_addr):
host, _, _ = remote_addr.partition(':')
try:
ip = socket.gethostbyname(host)
except socket.gaierror:
return False
return ip.startswith("127.") or ip == "::1"
def authorize_admin(admin_token, remote_addr, headers):
if is_loopback_remote(remote_addr):
return True
token = admin_token.strip()
if token == "":
return True
got = headers.get("X-Subrouter-Admin-Token", "").strip()
if not got:
auth = headers.get("Authorization", "").strip()
before, sep, after = auth.partition(" ")
if sep and before.lower() == "bearer":
got = after.strip()
if not got or len(got) != len(token):
return False
return got == token
for remote in ("127.0.0.1:54321", "100.64.0.2:12345"):
for auth_header in ("", "Bearer secret"):
print({"remote": remote, "admin_token": "", "auth": auth_header, "authorized": authorize_admin("", remote, {"Authorization": auth_header})})
PYRepository: manaflow-ai/subrouter
Length of output: 2106
Require an explicit admin token for tenant admin CRUD.
authorizeAdmin accepts all remote requests when AdminToken is empty, so handleTenantAdmin lets unauthenticated clients list tenants, create tenants, mint keys, and revoke keys. This CRUD surface exits the legacy “no admin token is OK for read-only/drainer” posture by exposing credential issuance and tenant-wide denial-of-service.
🔒 Fail-closed guard
func (m *MultiTenant) handleTenantAdmin(w http.ResponseWriter, r *http.Request) {
- if !m.Base.authorizeAdmin(r) {
+ if strings.TrimSpace(m.Base.AdminToken) == "" || !m.Base.authorizeAdmin(r) {
http.Error(w, "admin token required", http.StatusUnauthorized)
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // handleTenantAdmin serves the global-admin tenant CRUD: | |
| // | |
| // GET /_subrouter/tenants list tenants (key prefixes only) | |
| // POST /_subrouter/tenants {"name":..} create tenant, returns key once | |
| // POST /_subrouter/tenants/<id>/keys mint an extra key, returns it once | |
| // DELETE /_subrouter/tenants/<id>/keys/<prefix> revoke keys matching prefix | |
| func (m *MultiTenant) handleTenantAdmin(w http.ResponseWriter, r *http.Request) { | |
| if !m.Base.authorizeAdmin(r) { | |
| http.Error(w, "admin token required", http.StatusUnauthorized) | |
| return | |
| } | |
| rest := strings.TrimPrefix(r.URL.Path, "/_subrouter/tenants") | |
| rest = strings.Trim(rest, "/") | |
| switch { | |
| case rest == "": | |
| switch r.Method { | |
| case http.MethodGet: | |
| tenants, err := m.Registry.List() | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusInternalServerError) | |
| return | |
| } | |
| views := make([]tenantView, 0, len(tenants)) | |
| for _, t := range tenants { | |
| views = append(views, viewOf(t)) | |
| } | |
| writeJSON(w, views) | |
| case http.MethodPost: | |
| var payload struct { | |
| Name string `json:"name"` | |
| } | |
| if err := json.NewDecoder(io.LimitReader(r.Body, tenantAdminMaxBodyBytes)).Decode(&payload); err != nil { | |
| http.Error(w, "invalid JSON body", http.StatusBadRequest) | |
| return | |
| } | |
| created, key, err := m.Registry.Create(payload.Name) | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusBadRequest) | |
| return | |
| } | |
| writeJSON(w, map[string]any{"tenant": viewOf(created), "key": key}) | |
| default: | |
| http.Error(w, "method not allowed", http.StatusMethodNotAllowed) | |
| } | |
| default: | |
| parts := strings.Split(rest, "/") | |
| if len(parts) >= 2 && parts[1] == "keys" { | |
| tenantID := parts[0] | |
| switch { | |
| case len(parts) == 2 && r.Method == http.MethodPost: | |
| updated, key, err := m.Registry.CreateKey(tenantID) | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusNotFound) | |
| return | |
| } | |
| writeJSON(w, map[string]any{"tenant": viewOf(updated), "key": key}) | |
| return | |
| case len(parts) == 3 && r.Method == http.MethodDelete: | |
| revoked, err := m.Registry.RevokeKey(tenantID, parts[2]) | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusNotFound) | |
| return | |
| } | |
| writeJSON(w, map[string]any{"ok": true, "revoked": revoked}) | |
| return | |
| } | |
| } | |
| http.NotFound(w, r) | |
| } | |
| } | |
| const tenantAdminMaxBodyBytes = 1 << 16 | |
| func (m *MultiTenant) handleTenantAdmin(w http.ResponseWriter, r *http.Request) { | |
| if strings.TrimSpace(m.Base.AdminToken) == "" || !m.Base.authorizeAdmin(r) { | |
| http.Error(w, "admin token required", http.StatusUnauthorized) | |
| return | |
| } | |
| rest := strings.TrimPrefix(r.URL.Path, "/_subrouter/tenants") | |
| rest = strings.Trim(rest, "/") | |
| switch { | |
| case rest == "": | |
| switch r.Method { | |
| case http.MethodGet: | |
| tenants, err := m.Registry.List() | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusInternalServerError) | |
| return | |
| } | |
| views := make([]tenantView, 0, len(tenants)) | |
| for _, t := range tenants { | |
| views = append(views, viewOf(t)) | |
| } | |
| writeJSON(w, views) | |
| case http.MethodPost: | |
| var payload struct { | |
| Name string `json:"name"` | |
| } | |
| if err := json.NewDecoder(io.LimitReader(r.Body, tenantAdminMaxBodyBytes)).Decode(&payload); err != nil { | |
| http.Error(w, "invalid JSON body", http.StatusBadRequest) | |
| return | |
| } | |
| created, key, err := m.Registry.Create(payload.Name) | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusBadRequest) | |
| return | |
| } | |
| writeJSON(w, map[string]any{"tenant": viewOf(created), "key": key}) | |
| default: | |
| http.Error(w, "method not allowed", http.StatusMethodNotAllowed) | |
| } | |
| default: | |
| parts := strings.Split(rest, "/") | |
| if len(parts) >= 2 && parts[1] == "keys" { | |
| tenantID := parts[0] | |
| switch { | |
| case len(parts) == 2 && r.Method == http.MethodPost: | |
| updated, key, err := m.Registry.CreateKey(tenantID) | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusNotFound) | |
| return | |
| } | |
| writeJSON(w, map[string]any{"tenant": viewOf(updated), "key": key}) | |
| return | |
| case len(parts) == 3 && r.Method == http.MethodDelete: | |
| revoked, err := m.Registry.RevokeKey(tenantID, parts[2]) | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusNotFound) | |
| return | |
| } | |
| writeJSON(w, map[string]any{"ok": true, "revoked": revoked}) | |
| return | |
| } | |
| } | |
| http.NotFound(w, r) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/proxy/multitenant.go` around lines 306 - 377, Update
handleTenantAdmin to fail closed when the admin token is unset, rejecting the
request before authorizeAdmin can allow it; retain authorizeAdmin for validating
configured tokens and preserve the existing unauthorized response.
1 participant
One hosted subrouter can now serve many isolated users, each with their own Codex/Claude account pool. Built for cmux Cloud VMs: each user's VM gets a tenant-scoped base URL and routes only through that user's accounts.
Tenant state lives under the server state dir:
tenants/<id>/mirrors the single-tenant layout (codex/accounts/*.json,codex/claude/,sessions.json), andtenants.jsonmaps tenant id to name, createdAt, and key hashes. Keys aresrt_<32 hex>, shown once at creation; only SHA-256 hashes (plus a short display prefix for listing/revoking) are stored.Data-plane auth is a URL path prefix, since agent CLIs can only override base URLs:
https://host/t/<key>/v1/...,/t/<key>/backend-api/..., and/t/<key>/for Claude. The key is also accepted as Authorization Bearer or x-api-key, which is where Claude Code putsANTHROPIC_AUTH_TOKEN. Unknown or revoked keys get 401. Keyless requests keep today's single-tenant behavior; header-shapedsrt_tokens only get strict 401 semantics once tenants exist orserve --multi-tenantis set.Isolation comes from instantiating the existing single-tenant
proxy.Serverper tenant (lazy, cached): each tenant gets its own account stores, session store, scheduler ref, active-session tracking, read cache, and transcript recorder (under<transcripts>/tenants/<id>), rather than threading tenant ids through the proxy internals. The global loopbackreload-accountsalso reloads instantiated tenants so uploads hot-reload.Admin plane: the existing global admin token gates
GET/POST /_subrouter/tenants,POST /_subrouter/tenants/<id>/keys, andDELETE /_subrouter/tenants/<id>/keys/<prefix>. Tenant-scoped reads (/t/<key>/_subrouter/{accounts,account-status,usage-status,sessions}pluswhoami) are authorized by key possession, so existingsr serverclient code works by pointing at the tenant base URL. Other_subrouterendpoints 404 on tenant URLs.CLI:
sr tenant create/list,sr tenant key create/revoketalk to a named or default server's admin endpoints and fall back to the local state dir when run on the server host.sr server add --tenant-key srt_...stores the key on a server entry; the Codex config writer and Claude proxy env writer then emit/t/<key>base URLs (Claude env also sets the key asANTHROPIC_AUTH_TOKEN), and account/Claude-profile uploads land intenants/<id>/on the server, with the id resolved via the tenantwhoamiendpoint.Deviations from the sketch:
tenants.jsonstores key objects (hash, display prefix, createdAt) instead of bare hash strings, because revoke-by-prefix and listing need the prefix; key revocation isDELETE /_subrouter/tenants/<id>/keys/<prefix>(no tenant delete endpoint yet);--multi-tenantonly controls strict handling of header-borne keys, since path routing and admin CRUD are inert until a tenant exists anyway.Tests: httptest coverage for two-tenant pool isolation, bad-key 401 (path and header), key rotation plus revocation through the admin API, sticky sessions scoped per tenant, legacy fallthrough, tenant-scoped control endpoints, and admin auth; registry unit tests; CLI tests for tenant-key storage/validation, tenant-scoped base URLs, Claude env, and local tenant CRUD.
go build ./... && go vet ./... && go test ./...green.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds multi-tenant mode so one Subrouter serves many isolated tenants, each with its own accounts, sessions, scheduler, and transcripts. Tenant keys work via path or headers, are scrubbed before proxying, and invalid keys return 401 (enforced early with
--multi-tenant).New Features
<state-dir>/tenants/<id>/withtenants.jsonholding SHA-256 key hashes and display prefixes; exact-prefix or full-hash revocation; interprocess.lockand deep-copied cache to avoid lost or stale writes./t/<key>/...orAuthorization: Bearer/X-Api-Key; unknown/revoked keys return 401. Keyless requests keep legacy behavior;--multi-tenantrejects header-bornesrt_keys even before the first tenant exists.proxy.Serverper tenant (accounts, sessions, scheduler, active sessions, read cache, transcripts under<transcripts>/tenants/<id>). Global loopback/_subrouter/reload-accountsalso reloads tenant servers. Tenant keys are stripped fromAuthorization/X-Api-Keybefore dispatch so they never reach upstreams.GET/POST /_subrouter/tenants,POST /_subrouter/tenants/<id>/keys,DELETE /_subrouter/tenants/<id>/keys/<prefix>(admin token required). Tenant-scoped reads (/_subrouter/{accounts,account-status,usage-status,sessions}) plus/_subrouter/whoamiare authorized by the tenant key.sr tenant create/list,sr tenant key create/revoke;sr server add --tenant-key srt_...stores the key so Codex/Claude use/t/<key>base URLs automatically. Claude setsANTHROPIC_BASE_URLandANTHROPIC_AUTH_TOKENto the tenant, and clears a stalesrt_token when switching back to a non-tenant server. Account/Claude uploads land in the tenant dir, resolved viawhoami. Control and data URLs are normalized to the server root by stripping any/v1or/backend-apisuffix.Migration
sr tenant create <name>(store the printed key).sr server add <name> --url <url> --tenant-key srt_....https://host/t/<key>/v1ANTHROPIC_BASE_URL=https://host/t/<key>,ANTHROPIC_AUTH_TOKEN=<key>sr serve --multi-tenantto reject unknownsrt_header keys even before tenants exist.Written for commit f7cd575. Summary will update on new commits.
Summary by CodeRabbit
New Features
sr tenant/sr tenantsCLI commands and enhanced server setup with--tenant-key.Bug Fixes
401in multi-tenant mode, while tenantless traffic preserves prior single-tenant behavior.Tests