Skip to content

libsql-server: derive path-routed Hrana auth from the path namespace - #66

Merged
bartzon merged 2 commits into
v0.9.30-shopify-patchesfrom
hrana-path-route-authorize-path-namespace
Nov 4, 2026
Merged

bartzon merged 2 commits into
v0.9.30-shopify-patchesfrom
hrana-path-route-authorize-path-namespace

Conversation

@bartzon

@bartzon bartzon commented Nov 2, 2026 •

Copy link
Copy Markdown

Why

The path-routed Hrana endpoint (/dev/:namespace/v:version/pipeline) resolved the namespace in two places that could disagree. This change makes the route derive authentication, the connection, and the request context from one namespace, and removes the duplicated auth code that allowed the drift (// todo dupe #auth).

Tracked in shop/issues-retail#34508.

What changed

  • db_factory.rs: MakeConnectionExtractorPath → PathNamespaceRequest, a single extractor producing both the connection maker and the RequestContext for path-routed requests.
  • http/user/mod.rs: handle_hrana_pipeline uses the new extractor; the shared "namespace → JWT keys → authenticate" logic lives in authenticate_for_namespace, used by the header-based Authenticated / RequestContext extractors as well.
  • extract.rs: uses the shared helper; header-routed endpoints behave as before.

Test

New standalone::auth::jwt_auth_path_route_authorizes_path_namespace covers the path route with a namespace-scoped token: permitted writes still succeed, non-permitted ones are rejected with 403.

Ran cargo test -p libsql-server --test tests -- standalone::auth in a Linux container (rust 1.98.1): 3/3 pass across 3 runs. standalone::attach::attach_auth* fail identically on the base branch and attach_no_auth is flaky there too; unrelated.

CI

The first run failed only on connection_core::test::test_many_concurrent (456/457 passed, 4/4 tries failed). That test is schedule-sensitive (see #65): adding a test anywhere moves it into a CPU-contended slot. Second commit carries #65's nextest override so it runs with the runner to itself.

@bartzon
bartzon requested a review from a team November 2, 2026 14:01
@bartzon bartzon self-assigned this Nov 2, 2026
The `/dev/:namespace/v:version/pipeline` route resolved its namespace in
two independent extractors. Replace them with `PathNamespaceRequest`, a
single extractor that derives authentication, the connection maker and the
request context from the path namespace, and factor the shared
"namespace -> JWT keys -> authenticate" logic into
`authenticate_for_namespace` so the header-based `Authenticated` and
`RequestContext` extractors use the same code.

Add a test covering the path route with a namespace-scoped token.
@bartzon
bartzon force-pushed the hrana-path-route-authorize-path-namespace branch from b9eab30 to f80b6c2 Compare November 2, 2026 14:13
@bartzon bartzon changed the title libsql-server: authorize path-routed Hrana requests against the path namespace libsql-server: derive path-routed Hrana auth from the path namespace Nov 2, 2026
`connection::connection_core::test::test_many_concurrent` runs 3 tasks x
1000 `BEGIN IMMEDIATE` transactions with a 100 ms TXN_TIMEOUT and a 100 ms
SQLite busy timeout under cfg(test). It fails with "database is locked" (or
hits the 60 s slow timeout) whenever other test binaries compete for the
runner's CPUs. nextest's schedule is deterministic for a given test set, so
adding a test anywhere in the workspace can move it into a contended slot
and make it fail on every retry; this PR's new test did exactly that
(456/457 passed, test_many_concurrent 4/4 tries failed).

Reserve all CPUs for it so nextest runs it with nothing else in flight.
Same change as the unmerged #65; no change to the test itself.
@tszymczyszyn-shopify

Copy link
Copy Markdown

LGTM 👍

I built sqld and ran it with --enable-namespaces --disable-default-namespace and a server-wide JWT key. Path and header routes behaved as expected, and the path route now works without x-namespace.

Suggestion (non-blocking): the new test registers the same JWT key for mine and other. It checks which namespace's permissions apply, but not which namespace's keys verify the token. A test with distinct keys would cover the other half of the contract. I ran this version locally against this PR, and it passes for v2 and v3:

  #[test]
  fn jwt_auth_path_route_uses_path_namespace_jwt_key() {
      let mut sim = turmoil::Builder::new()
          .simulation_duration(Duration::from_secs(1000))
          .build();

      sim.host("primary", make_standalone_server);

      sim.client("test", async {
          let client = Client::new();
          let (mine_enc, mine_key) = key_pair();
          let (other_enc, other_key) = key_pair();

          for (ns, key) in [("mine", mine_key), ("other", other_key)] {
              assert!(client
                  .post(
                      &format!("http://primary:9090/v1/namespaces/{ns}/create"),
                      serde_json::json!({ "jwt_key": key })
                  )
                  .await?
                  .status()
                  .is_success());
          }

          // Both tokens may write both namespaces, so only the signing key differs.
          let claims = serde_json::json!({ "p": { "rw": { "ns": ["mine", "other"] } } });
          let mine_bearer = format!("Bearer {}", encode(&claims, &mine_enc));
          let other_bearer = format!("Bearer {}", encode(&claims, &other_enc));

          for version in [2, 3] {
              let url = format!("http://primary:8080/dev/other/v{version}/pipeline");
              let body = serde_json::json!({
                  "requests": [
                      { "type": "execute", "stmt": { "sql": format!("create table t{version} (x)") } },
                      { "type": "close" }
                  ]
              });

              // Signed with the header namespace's key: rejected.
              let resp = client
                  .post_with_headers(
                      &url,
                      &[
                          (hyper::header::AUTHORIZATION, mine_bearer.as_str()),
                          (HeaderName::from_static("x-namespace"), "mine"),
                      ],
                      &body,
                  )
                  .await?;
              assert_eq!(resp.status(), hyper::StatusCode::UNAUTHORIZED);

              // Signed with the path namespace's key: accepted despite the header.
              let resp = client
                  .post_with_headers(
                      &url,
                      &[
                          (hyper::header::AUTHORIZATION, other_bearer.as_str()),
                          (HeaderName::from_static("x-namespace"), "mine"),
                      ],
                      &body,
                  )
                  .await?;
              assert_eq!(resp.status(), hyper::StatusCode::OK);
              assert_eq!(resp.json_value().await?["results"][0]["type"], "ok");
          }

          Ok(())
      });

      sim.run().unwrap();
  }

@bartzon
bartzon merged commit 6c09547 into v0.9.30-shopify-patches Nov 4, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants