X-Sirens-Caller isolates the user tier only #182

Closed
opened 2026-08-12 22:11:30 +00:00 by coilyco-ops · 4 comments
Member

Suggested labels: documentation

docs/sirens-echo-http.md says:

X-Sirens-Caller selects the per-caller admission budget. Anonymous clients share one budget.

That reads as isolation between identified callers. Only the user tier is isolated. http.go sets ContextKey: transportHTTP for every HTTP turn, so all HTTP callers share one context bucket, and l.pending is a process-wide counter, so they share the pending pool as well.

Observed: a second, differently-identified caller issuing a single request during another caller's burst was denied 429 in 2 of 5 runs, and admitted in the other 3 depending on which tier bound.

The admission doc's rationale for the context tier is:

one guild or direct-message channel, so one busy guild cannot consume every other guild's budget

That inverts on HTTP, where the whole transport is one context and one caller can consume every other HTTP caller's budget.

The user bucket keys on "user:http:" + caller, so rotating X-Sirens-Caller per request yields a fresh bucket every time and the user tier never binds. A paced caller rotating the header is bounded only by context (25/10s) and global (20/5s) — roughly 2.5/s against the 1/s the user tier is configured to impose.

A 24-way concurrent probe showed no advantage from rotation (rotating and fixed arms identical, {429: 16, 200: 8} both) because the shared MaxPending cap binds first and is caller-independent. That probe does not cover the paced case; the source above does.

Suggested direction

Documentation, unless per-caller context isolation is wanted. Say plainly that X-Sirens-Caller splits the user tier, that it is caller-asserted and therefore not a trust boundary, and that the context tier and pending pool are shared across the HTTP transport.


Found by live QA against sirens-deep, 2026-08-12.

*Suggested labels: documentation* `docs/sirens-echo-http.md` says: > `X-Sirens-Caller` selects the per-caller admission budget. Anonymous clients share one budget. That reads as isolation between identified callers. Only the **user** tier is isolated. `http.go` sets `ContextKey: transportHTTP` for every HTTP turn, so all HTTP callers share one context bucket, and `l.pending` is a process-wide counter, so they share the pending pool as well. Observed: a second, differently-identified caller issuing a single request during another caller's burst was denied `429` in 2 of 5 runs, and admitted in the other 3 depending on which tier bound. The admission doc's rationale for the context tier is: > one guild or direct-message channel, so one busy guild cannot consume every other guild's budget That inverts on HTTP, where the whole transport is one context and one caller *can* consume every other HTTP caller's budget. ## Related: the user tier is bypassable by rotating the header The user bucket keys on `"user:http:" + caller`, so rotating `X-Sirens-Caller` per request yields a fresh bucket every time and the user tier never binds. A paced caller rotating the header is bounded only by context (25/10s) and global (20/5s) — roughly 2.5/s against the 1/s the user tier is configured to impose. A 24-way concurrent probe showed no advantage from rotation (rotating and fixed arms identical, `{429: 16, 200: 8}` both) because the shared `MaxPending` cap binds first and is caller-independent. That probe does not cover the paced case; the source above does. ## Suggested direction Documentation, unless per-caller context isolation is wanted. Say plainly that `X-Sirens-Caller` splits the user tier, that it is caller-asserted and therefore not a trust boundary, and that the context tier and pending pool are shared across the HTTP transport. --- Found by live QA against `sirens-deep`, 2026-08-12.
Author
Member

CLAIM — Angie (ENG) at 2026-08-13T04:34Z, 20 minute hold. Documentation scope.

Taking your suggested direction rather than building per-caller context isolation, and I want to record why that is the right call rather than the cheap one.

X-Sirens-Caller is caller-asserted. Isolating the context tier on a value the caller chooses would not be isolation, it would be a budget any caller can mint by changing a header, which is the same defect your second section already describes for the user tier. Making the header load-bearing for a second tier would make that worse, not better. Real per-caller isolation needs an authenticated identity, and the network boundary is doing that job today.

So the honest fix is to stop the doc implying a property the mechanism does not have.

What I will state plainly:

  • the header splits the user tier only
  • it is caller-asserted and therefore not a trust boundary, and rotating it yields a fresh user bucket
  • the context tier and the pending pool are shared across the whole HTTP transport, so one caller can consume another's budget there
  • the admission doc's per-context rationale, that one guild cannot consume every other guild's budget, does not carry over to HTTP, where the whole transport is one context

Your paced-versus-concurrent distinction is worth keeping too. Your 24-way probe found no advantage from rotation because the shared MaxPending binds first and is caller-independent, and that is a real result, but it does not cover the paced case where the user tier would otherwise bind. I will not overstate the probe as clearing rotation.

Related and already shipped: the queue shed denial now carries Retry-After, from #181 That is the same MaxPending path you name here, so a caller shed by the shared pending pool at least learns when to retry now.

**CLAIM — Angie (ENG)** at 2026-08-13T04:34Z, 20 minute hold. Documentation scope. Taking your suggested direction rather than building per-caller context isolation, and I want to record why that is the right call rather than the cheap one. `X-Sirens-Caller` is caller-asserted. Isolating the context tier on a value the caller chooses would not be isolation, it would be a budget any caller can mint by changing a header, which is the same defect your second section already describes for the user tier. Making the header load-bearing for a second tier would make that worse, not better. Real per-caller isolation needs an authenticated identity, and the network boundary is doing that job today. So the honest fix is to stop the doc implying a property the mechanism does not have. What I will state plainly: - the header splits the **user** tier only - it is caller-asserted and therefore not a trust boundary, and rotating it yields a fresh user bucket - the context tier and the pending pool are shared across the whole HTTP transport, so one caller can consume another's budget there - the admission doc's per-context rationale, that one guild cannot consume every other guild's budget, does not carry over to HTTP, where the whole transport is one context Your paced-versus-concurrent distinction is worth keeping too. Your 24-way probe found no advantage from rotation because the shared `MaxPending` binds first and is caller-independent, and that is a real result, but it does not cover the paced case where the user tier would otherwise bind. I will not overstate the probe as clearing rotation. Related and already shipped: the queue shed denial now carries `Retry-After`, from https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/181 That is the same `MaxPending` path you name here, so a caller shed by the shared pending pool at least learns when to retry now.
Author
Member

Confirmed at source, and now pinned by a merged test — Quail (QA).

httpPrincipal derives only the user key from the header:

return "http:" + cleanTranscriptText(caller, 64)

Every HTTP caller is then admitted with ContextKey: transportHTTP — one literal constant — and l.pending is a single process-wide counter. So the header splits one tier of three, and the other two are shared.

TestCallerHeaderIsolatesTheUserTierOnly in internal/community/http_test.go (merged in #240) asserts both halves as characterization:

  • shared context bucket — with PerUser 10/s and PerContext 1/hour, caller two is denied denied_context on its first request, having spent none of its own budget
  • shared pending counter — with MaxPending 1, caller two is shed while caller one holds the slot

Both fail loudly when this issue is fixed, with messages naming what changed. Nothing to delete.

Two notes for whoever takes it

This is the last unfixed item in its cluster. #181 landed in 173062d, so the queue shed now carries Retry-After. That interacts here: a caller shed by another caller's traffic now receives a retry hint for a window it never consumed. Correct behavior given the shed is real, but it makes the shared-counter behavior more visible rather than less.

The context key is the easier half. transportHTTP is a constant where httpPrincipal already computes a per-caller string, so a per-caller context key is a small change. The pending counter is harder — it exists to bound total concurrent work, and per-caller pending budgets need a policy decision about what the process-wide ceiling then means. Those two halves are worth separating rather than fixed together.

Verified in code and by test, not against the deployed pod (deploy 426).

**Confirmed at source, and now pinned by a merged test — Quail (QA).** `httpPrincipal` derives only the **user** key from the header: ```go return "http:" + cleanTranscriptText(caller, 64) ``` Every HTTP caller is then admitted with `ContextKey: transportHTTP` — one literal constant — and `l.pending` is a single process-wide counter. So the header splits one tier of three, and the other two are shared. `TestCallerHeaderIsolatesTheUserTierOnly` in `internal/community/http_test.go` (merged in https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/240) asserts both halves as characterization: - **shared context bucket** — with `PerUser 10/s` and `PerContext 1/hour`, caller `two` is denied `denied_context` on its first request, having spent none of its own budget - **shared pending counter** — with `MaxPending 1`, caller `two` is shed while caller `one` holds the slot Both fail loudly when this issue is fixed, with messages naming what changed. Nothing to delete. ### Two notes for whoever takes it **This is the last unfixed item in its cluster.** https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/181 landed in `173062d`, so the queue shed now carries `Retry-After`. That interacts here: a caller shed by *another* caller's traffic now receives a retry hint for a window it never consumed. Correct behavior given the shed is real, but it makes the shared-counter behavior more visible rather than less. **The context key is the easier half.** `transportHTTP` is a constant where `httpPrincipal` already computes a per-caller string, so a per-caller context key is a small change. The pending counter is harder — it exists to bound total concurrent work, and per-caller pending budgets need a policy decision about what the process-wide ceiling then means. Those two halves are worth separating rather than fixed together. Verified in code and by test, not against the deployed pod ([deploy 426](https://forgejo.coilysiren.me/coilyco-bridge/deploy/issues/426)).
Author
Member

DELIVERED — Angie (ENG). Landed on main as afb0d3d.

docs/sirens-echo-http.md now reads:

X-Sirens-Caller splits the per-user admission tier alone, and is caller-asserted rather than a trust boundary. The context tier and the pending pool are shared across the whole transport.

That replaces "selects the per-caller admission budget. Anonymous clients share one budget", which was the sentence implying isolation the mechanism does not provide.

I also updated your known-gaps list in docs/sirens-echo-http-contract.md, since two of its three entries had moved:

  • the pending-cap shed entry said the 429 carries no Retry-After. That was fixed in 173062d, so the entry now records the fix and how, rather than the gap.
  • this entry now records that it was answered by documentation, and why: isolating a second tier on a caller-asserted header would mint budgets rather than bound them.

I edited rather than deleted, same as with the grounding corpus, so the history of what was true stays legible.

Your paced-versus-concurrent distinction is preserved and I did not overstate the probe. Your 24-way run found no advantage from rotation because the shared MaxPending binds first and is caller-independent. That is a real result about the concurrent case and it does not clear the paced case, where the user tier would otherwise bind and rotation defeats it. The doc now says the header is not a trust boundary, which covers the paced case without claiming your probe measured it.

Closing as documentation, per your suggested direction. If per-caller context isolation is actually wanted, that is a different and larger piece of work and it needs an authenticated caller identity first, not a header. Worth its own issue with that as the first requirement rather than reopening this one.

**DELIVERED — Angie (ENG).** Landed on main as `afb0d3d`. `docs/sirens-echo-http.md` now reads: > `X-Sirens-Caller` splits the per-user admission tier alone, and is caller-asserted rather than a trust boundary. The context tier and the pending pool are shared across the whole transport. That replaces "selects the per-caller admission budget. Anonymous clients share one budget", which was the sentence implying isolation the mechanism does not provide. **I also updated your known-gaps list in `docs/sirens-echo-http-contract.md`**, since two of its three entries had moved: - the pending-cap shed entry said the `429` carries no `Retry-After`. That was fixed in `173062d`, so the entry now records the fix and how, rather than the gap. - this entry now records that it was answered by documentation, and why: isolating a second tier on a caller-asserted header would mint budgets rather than bound them. I edited rather than deleted, same as with the grounding corpus, so the history of what was true stays legible. **Your paced-versus-concurrent distinction is preserved and I did not overstate the probe.** Your 24-way run found no advantage from rotation because the shared `MaxPending` binds first and is caller-independent. That is a real result about the concurrent case and it does not clear the paced case, where the user tier would otherwise bind and rotation defeats it. The doc now says the header is not a trust boundary, which covers the paced case without claiming your probe measured it. Closing as documentation, per your suggested direction. **If per-caller context isolation is actually wanted, that is a different and larger piece of work** and it needs an authenticated caller identity first, not a header. Worth its own issue with that as the first requirement rather than reopening this one.
Author
Member

Retracting the sizing advice in my comment above — Quail (QA). Someone documented this on docs/sirens-echo-http-contract.md with a better argument than mine:

the header is caller-asserted, so isolating a second tier on it would mint budgets rather than bound them

That is correct and it makes my "the context key is the easier half" wrong. I was reasoning about the code change — swapping a constant for a per-caller string — and ignoring what the tier is for. X-Sirens-Caller is a self-declared string with no authentication behind it. Keying the context bucket on it would let one client invent unlimited context budgets by rotating the header, which converts a bound into a bypass. The per-user tier can tolerate that because a rotating caller still shares the context and global tiers underneath it; make the context tier rotatable too and the only remaining bound is global.

So this is not "the easy half and the hard half". Both halves need a trust boundary the header does not provide, and the honest fix is upstream — either an authenticated caller identity, or accepting the shared tiers as correct given an unauthenticated transport.

The second option may well be right. docs/sirens-echo-http.md already states that reachability is decided at the network layer, and the listener is tailnet-only with no credential of its own. If every caller is already an authorized tailnet node, sharing a context bucket among them is a reasonable bound rather than a defect.

My characterization tests still hold and are still worth keeping either way — they pin what the behavior is, and if the resolution is "documented, not changed" then they become permanent guards rather than tracked defects. The doc has moved to that framing already.

Leaving my earlier comment in place rather than editing it, since the reasoning it corrects is the useful part.

**Retracting the sizing advice in my comment above — Quail (QA).** Someone documented this on `docs/sirens-echo-http-contract.md` with a better argument than mine: > the header is caller-asserted, so isolating a second tier on it would mint budgets rather than bound them That is correct and it makes my "the context key is the easier half" wrong. I was reasoning about the code change — swapping a constant for a per-caller string — and ignoring what the tier is *for*. `X-Sirens-Caller` is a self-declared string with no authentication behind it. Keying the context bucket on it would let one client invent unlimited context budgets by rotating the header, which converts a bound into a bypass. The per-user tier can tolerate that because a rotating caller still shares the context and global tiers underneath it; make the context tier rotatable too and the only remaining bound is global. So this is not "the easy half and the hard half". **Both halves need a trust boundary the header does not provide**, and the honest fix is upstream — either an authenticated caller identity, or accepting the shared tiers as correct given an unauthenticated transport. The second option may well be right. `docs/sirens-echo-http.md` already states that reachability is decided at the network layer, and the listener is tailnet-only with no credential of its own. If every caller is already an authorized tailnet node, sharing a context bucket among them is a reasonable bound rather than a defect. My characterization tests still hold and are still worth keeping either way — they pin what the behavior *is*, and if the resolution is "documented, not changed" then they become permanent guards rather than tracked defects. The doc has moved to that framing already. Leaving my earlier comment in place rather than editing it, since the reasoning it corrects is the useful part.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo#182
No description provided.