feat(token-id-capture): add an optional bearer token to the read route

[ananthsub]

Adds an optional bearer token on the route that serves a rollout's captured training tokens.

Why

The read route exists so a trainer that cannot see the writer's filesystem can still read a rollout's records. It serves those raw tokens on the same server the harness calls to generate.

Inside a trusted cluster that is acceptable. It stops being acceptable once the harness runs in a sandbox whose only egress is that server, because the harness could then read its own training data, or another rollout's.

flowchart LR
    harness["Agent Harness<br/>(sandboxed, inside the Agent Server)"] -->|generate| server
    server["Model Server<br/>serves generation and the token read route"]
    harness -.->|"GET /ng-capture/tokens/&lt;id&gt;<br/>401 without the token"| server
    reader["Gym rollout collection<br/>or the training framework"] -->|"GET with bearer token"| server

Loading

What it does

env:
  nemo_gym:
    token_id_capture_read_token: ${oc.env:NG_TOKCAP_READ_TOKEN,}

Set, the route requires a matching bearer token and returns 401 otherwise. The comparison is constant-time, since that token is the only thing between an untrusted harness and every rollout's training data.

Unset, the route stays open and logs a warning once at startup.

On the default

Defaulting a security control to off is worth naming rather than glossing. The reason is that an existing deployment upgrading to this version would otherwise start getting 401s from a reader that has no way to learn the token. The warning is there so the gap is visible rather than silent.

There is a better end state: both the model server and the reader resolve the same configuration, so the token could be generated once during config resolution and reach both automatically, with an explicit value still overriding for runs where the configuration is not shared. That is not in this PR, and I would rather state the tradeoff than defend the current default as obviously right.

Set it before running a harness in a sandbox.

Tests

That the route returns 401 without a token when one is configured, that it serves the records with a correct token, and that it stays open when no token is set.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[ananthsub]

Closing: the read path this guards has been removed from the stack rather than secured.

The route served a rollout's records over HTTP for a reader that cannot see the writer's filesystem. Two problems surfaced while verifying the stack end to end:

• Nothing consumed it. No code path at any PR in the stack used HttpTokenReader or LocalTokenReader. The only non-test reference anywhere was a docstring mention.
• A remote reader could not be safe. is_incomplete is how a consumer learns that a rollout lost a model call; the route serves records and nothing about them, so an HTTP reader could only ever return False. It would train on an incomplete rollout without knowing, which is the exact failure the capture path exists to prevent.

So #2124 no longer registers the route and no longer ships either reader, LocalTokenReader is gone entirely (TokenCaptureStore already satisfies TokenSource, so it wrapped something that needed no wrapping), and token_id_capture.read_token is gone with them. The stack is eight PRs instead of nine.

When a non-colocated reader is actually needed it should come back as one PR: a route that serves records and the incomplete flag, a client that implements is_incomplete honestly, and the bearer token. Splitting the route into #2124 and its auth into this PR meant an unauthenticated endpoint existed for six PRs, which is the other reason not to restore it as-is.

Documented in #2341: "Reader and writer have to be on the same node today."