rossoctl / rossoctl/serverless-harness

P5: session identity isolation — per-request subject, no ambient credential

Open
#239 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
TypeScript
Stars
1
Forks
7
Avg merge
12h 59m
Merged PRs (30d)
71

Description

Split out of #220. This is the session identity isolation slice: per-request subject inflow, removal of the ambient credential fallbacks, and a startup scrub.

Source of truth is the P5 spec and ADR-0032, not #220's severity table. #220's table was written from reading the code rather than tracing it and is wrong on four of five rows; the corrected tracing is in the spec's §2.1.

Why this is the prerequisite, not the density win

#220 framed the blocker as five process-global mutables. Only one is real, and it is not a concurrency bug: ANTHROPIC_API_KEY is seeded write-once-if-absent at harness/src/run-turn.ts:310-312, so session A's token sticks process-wide and sessions B..N authenticate as A. That is an identity leak, not mutual corruption — and the actual missing prerequisite is that no per-session credential enters the harness at all.

Three ordered steps (spec §3.2 — the order is forced by §2.3)

  1. buildConfig()buildConfig(req) in packages/knative-server/src/server.ts:66-73 (call sites :111, :174, :411, :415).
    • Subject arrives on a dedicated X-SH-Subject header — not Authorization (reserved for caller auth; ADR-0011 lock-down implies it is coming) and not the request body (the body is logged and persisted to Redis).
    • Placeholder derived by a pure, non-secret mapping from mounted config.
    • Missing subject → 401 with no upstream request made.
  2. Delete the ambient reads in harness/src/run-turn.ts::applyModelGateway: the process.env.ANTHROPIC_API_KEY seed (:310-312) and the || process.env.ANTHROPIC_AUTH_TOKEN fallback (:306). Signature unchanged — only what it trusts. Safe only after step 1.
  3. Startup scrub at the server entrypoint only: set ANTHROPIC_API_KEY to the §3.3 sentinel (e.g. sh-unused-see-authorization-header), and delete ANTHROPIC_OAUTH_TOKEN and ANTHROPIC_AUTH_TOKEN.
    • Not in run-turn, not in leaf-job — both paths share applyModelGateway, and scrubbing there would break leaf mode.
    • The sentinel is required, not cosmetic: pi resolves the request key by provider name, so with ANTHROPIC_API_KEY simply absent, _getRequiredRequestAuth throws No API key found for "anthropic" and gateway mode breaks outright. Step 2 must not land without step 3.

Not in scope

  • Any pi-fork change. Pi's credential path is already per-session (AgentSession._getRequiredRequestAuth). Threading a SessionContext through pi was considered and rejected — four refactors that would add fork divergence and fix nothing.
  • harness/src/cli.ts:9-14 and packages/knative-server/src/leaf-job.ts:15-18 stay ambient on purpose.
  • Caller authentication — that is ADR-0011's lock-down work, not new scope here. X-SH-Subject says whose work this is, not who may ask.
  • Per-tenant sandbox or data isolation.

Placeholder isolation is in scope at the same strictness as key isolation (spec §3.4)

Under lock-down the harness holds no secret, so a leaked placeholder looks cosmetic. It is not. A placeholder is an identity assertion. If session B inherits tenant A's placeholder, the injector faithfully swaps in A's real credential — so A's budget is spent on B's work, and the injector's audit trail records the request as legitimately A's. The one mechanism that would otherwise catch the error instead certifies it.

The four inert globals get reachability guards

stdoutTakeoverState, sessionResourceCleanups, fileMutationQueues, commandResultCache (spec §4). #220 called these blockers; they are inert in server mode. Each guard exists to make that stay true, not to fix anything.

cwd is the one item that can change the verdict. It is process-wide, unlisted by #220, and expected inert under the FS-free two-tier model (P1) — but not proven. A failure here upgrades scope rather than being papered over.

Tests

Use spec §5's six rows as written. Three of them look over-specified until you know why, so the rationale is recorded here:

  • Precedence. The Sentinel identity row asserts ANTHROPIC_OAUTH_TOKEN and ANTHROPIC_AUTH_TOKEN are absent, not just that ANTHROPIC_API_KEY holds the sentinel. pi-fork packages/ai/src/env-api-keys.ts:96-99 returns ["ANTHROPIC_OAUTH_TOKEN", "ANTHROPIC_API_KEY"]OAuth outranks the API key — so pinning one variable leaves the assertion green while a real credential wins.
  • The leaf survives the deletions, for two independent reasons (spec §3.2 step 2). This is the dual-path check most likely to be re-derived from scratch: applyModelGateway is shared with runLeaf, and deploy/knative/leaf-scaledjob.yaml:65-72 mounts ANTHROPIC_API_KEY from llm-credentials independently.
  • Interleaving must occur at await boundaries, not sequentially. A sequential two-session test passes while the bug remains, which is worse than no test.
  • The ambient-absence test must set the env var deliberately and assert failure anyway.

Known gap, out of scope: deploy/knative/service.yaml:45-49 and :56-60 still mount the real api-key and auth-token from llm-credentials. Spec §3.6 scopes the lock-down row to in-process precisely because closing this "cannot be done by editing service.yaml" — it needs an injector in the harness's egress path (ADR-0011 · Z2). Tracked separately.

Interactions the MU1 control-plane spec records against this work

#238 (multi-user control plane, MU1) is a separate track that composes with this one. Its §3.6 names three things a P5 implementation should know, so they do not get discovered late:

  1. The Bearer payload is one-or-the-other. P5's target is an inert placeholder rewritten by an injector; MU1's interim mode puts the real control-plane-resolved token in the same field. A P5 implementation that unconditionally sets the placeholder would silently overwrite MU1's token. Hence TurnConfig should carry a tagged credential ({mode: 'placeholder'|'direct', value}), not a bare string.
  2. Direct mode diverges from §5's in-process invariant. Scope the lock-down assertion to the environment (which MU1 never writes) rather than the whole process, or gate it on direct mode being disabled — otherwise the assertion contradicts an accepted ADR-0033 cost.
  3. Inbound X-SH-Subject becomes conditional. Do not pin "inbound X-SH-Subject is always honoured": on a token-bearing path the subject is token.sub and an inbound header is ignored. Operator and leaf paths keep the header behaviour.

Naming to confirm before implementation

The X-SH-Subject header and the placeholder-config env SH_SUBJECT_PLACEHOLDERS. Both were pinned by the spec as proposals awaiting confirmation.

Mechanics (spec §8)

  • Worktree order: git submodule update --init --recursive, then cd pi-fork && npm ci && npm run build, then root pnpm install.
  • Tests need the sh-test-redis container on :6379. ~10 ECONNREFUSED across 4 files means it is stopped, not a regression.
  • make lint skips untracked files — stage new files first.
  • git commit -s (DCO is a required check).

Relationships

  • Split out of #220, alongside #240 (deployment model) and #241 (injector dependency).
  • Blocks #240 — multiplexing sessions into shared pods is only safe once a turn's identity comes from the request.
  • Blocks #241 / rossoctl/cortex#905 — the injector cannot key on a subject the harness never sends.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with docs/specs/2026-09-06-p5-session-isolation-design.md and docs/adrs/0032-per-request-subject-no-ambient-credential.md, then trace buildConfig in packages/knative-server/src/server.ts and applyModelGateway in harness/src/run-turn.ts. Use the spec's six test rows and the setup steps in §8; done means per-request subjects and placeholders are isolated, ambient credentials are scrubbed safely, leaf mode survives, and the reachability guards hold.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, authentication, backend, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.