BE-714: Gate the actor-ID header behind a service credential - #9233
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## t/be-760-verify-kratos-sessions-in-the-graph-api #9233 +/- ##
====================================================================================
+ Coverage 59.84% 59.92% +0.08%
====================================================================================
Files 1425 1426 +1
Lines 139512 139853 +341
Branches 6570 6579 +9
====================================================================================
+ Hits 83484 83811 +327
- Misses 54958 54969 +11
- Partials 1070 1073 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will not alter performance
Comparing |
PR SummaryHigh Risk Overview Bootstrap routes ( The Graph server refuses to start without a non-empty Also masks sensitive env values in CLI Reviewed by Cursor Bugbot for commit 99c31d8. Bugbot is set up for automated code reviews on this repo. Configure here. |
65ebe47 to
3654b9c
Compare
There was a problem hiding this comment.
Pull request overview
Gates Graph actor delegation and bootstrap routes behind a shared service credential.
Changes:
- Adds composable Kratos and service-delegation authentication providers.
- Wires the credential through Graph clients, server configuration, and Compose.
- Adds delegation tests and masks sensitive CLI environment values.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.env |
Defines the local service secret. |
Cargo.toml |
Adds the constant-time comparison dependency. |
Cargo.lock |
Records generated dependency updates. |
apps/hash-graph/Cargo.toml |
Adds debug-derivation support. |
apps/hash-graph/src/subcommand/admin_server.rs |
Hides the Mailchimp key in help. |
apps/hash-graph/src/subcommand/server.rs |
Validates and masks the service secret. |
infra/compose/compose.yml |
Distributes the secret to Graph services. |
libs/@local/graph/api/src/rest/auth.rs |
Enforces authentication and bootstrap gating. |
libs/@local/graph/api/src/rest/mod.rs |
Wires the provider chain into REST. |
libs/@local/graph/authentication/Cargo.toml |
Adds subtle. |
libs/@local/graph/authentication/src/actor.rs |
Shares a test actor resolver. |
libs/@local/graph/authentication/src/delegation.rs |
Implements service delegation. |
libs/@local/graph/authentication/src/kratos.rs |
Adapts Kratos to provider chaining. |
libs/@local/graph/authentication/src/lib.rs |
Exports delegation support. |
libs/@local/graph/authentication/src/provider.rs |
Introduces terminal provider chaining. |
libs/@local/graph/authentication/src/request.rs |
Removes bare actor-header fallback. |
libs/@local/graph/migrations/src/bin/cli/subcommand/mod.rs |
Hides database passwords in help. |
libs/@local/graph/postgres-store/src/store/config.rs |
Hides database passwords in help. |
libs/@local/hash-backend-utils/src/create-graph-client.ts |
Sends the service credential by default. |
libs/@local/telemetry/src/traces/sentry.rs |
Hides Sentry DSN values in help. |
tests/graph/http/.httpyac.config.js |
Injects the secret into HTTP tests. |
tests/graph/http/test.sh |
Runs delegation integration tests. |
tests/graph/http/tests/hashql.http |
Preserves the no-credentials test case. |
tests/graph/http/tests/service-delegation.http |
Covers delegation rejection paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
tests/graph/http/.httpyac.config.js:11
- This suite-wide hook adds the service secret to every HTTP request, including requests to Kratos (
kratos-session.http:44,72,83) and the type fetcher (type-fetcher.http:9,27). That forwards a sensitive Graph credential to unrelated services. Scope the injection and<omit>handling to the Graph REST origin (127.0.0.1:4000) only.
api.hooks.onRequest.addHook("serviceSecret", (request) => {
request.headers = {
"X-HASH-Service-Secret":
process.env.HASH_GRAPH_SERVICE_SECRET ?? "hash-svc-local-dev-secret",
...request.headers,
};
libs/@local/graph/authentication/src/request.rs:131
- Making providers the only credential path invalidates the current OpenAPI source annotations: operations still advertise
X-Authenticated-User-Actor-Idalone (for exampleapi/src/rest/property_type.rs:141andprincipal.rs:206), while bootstrap operations advertise no service-secret header. Those documented requests now receive 401. Update the utoipa authentication/header contract for both delegated and bootstrap calls so generated clients expose the required credential.
/// The provider is the only credential path: a request whose credential is rejected fails, and a
/// request without a recognized credential fails as [`MissingCredentials`]. Chain providers as
/// pairs, nested for more than two, to accept several credential kinds.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (1)
infra/compose/compose.yml:623
- The Compose topology is documented as the starting point for self-hosted deployments (
README.md:182) and publishes Graph on all host interfaces (compose.yml:629-630). Falling back to this repository-known credential means a deployment that does not override the variable accepts a secret any client knows, allowing it to delegate as an arbitrary actor. Require an explicitly supplied production secret across Graph and every sender, while keeping the local-development value in a development-only configuration.
HASH_GRAPH_SERVICE_SECRET: "${HASH_GRAPH_SERVICE_SECRET:-hash-svc-local-dev-secret}"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (2)
libs/@local/graph/api/src/rest/auth.rs:100
- This newly requires a service secret on the two bootstrap endpoints, but their source OpenAPI annotations still advertise no credential/header or 401 response (
rest/permissions.rs:314-322andrest/principal.rs:169-180). Consumers generated from that contract will call these routes without the new header and receive 401. Update both source annotations to describe the service-secret requirement and unauthenticated response; likewise model the service-secret + actor-ID pair for the existing actor-header annotations rather than editing generated OpenAPI output.
let bootstrap = is_bootstrap_route(request.uri().path());
if bootstrap && !presents_service_secret(request.headers(), &service_secret) {
return rejection(&AuthenticationError::MissingServiceSecret).into_response();
libs/@local/graph/api/src/rest/auth.rs:101
- A bootstrap request with an
X-HASH-Service-Secretheader that is present but incorrect also enters this branch, so it is reported asMissingServiceSecret("request requires" a credential) rather thanInvalidServiceSecret. Because this returns beforeresolve_request_actor, it also bypasses the warn-level mismatch logging used for stale deployments. Distinguish an absent header from a mismatched value here, returnInvalidServiceSecretfor the latter, and emit the same warn-level signal.
if bootstrap && !presents_service_secret(request.headers(), &service_secret) {
return rejection(&AuthenticationError::MissingServiceSecret).into_response();
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |
🌟 What is the purpose of this PR?
Closes the last unauthenticated path into the Graph REST API: the
X-Authenticated-User-Actor-Idheader is only honored next to a service secret. Internal services (Node API, Temporal workers) authenticate themselves before naming the actor they act for, and the bootstrap routes require the same secret instead of being reachable by anyone on the network.🔗 Related links
🚫 Blocked by
🔍 What does this change?
AuthenticationProviderreturnsControlFlow<Authentication>:Continue(())means the request carries no credential the provider handles,Break(Verified | Rejected)is terminal. Chaining is the?operator, so a rejected credential structurally cannot fall through to another providerAuthentication::Verifiedcarries the authorization crate'sAuthenticatedActor— Kratos verifies to a typedId, service delegation passes the claimed actor through asUuiddelegation::ServiceDelegationProviderrecognizes the pair ofX-HASH-Service-Secretand the actor-ID header as one credential. The secret is compared in constant time. An actor-ID header without the secret is rejected, never ignoredresolve_request_actoris deleted; the new rejections log at warn level, since only internal services send these credentials and a mismatch is a deployment problem/policies/seed,/actors/machine/identifier/system/{identifier}) require the service secret and lose their unauthenticated allowlist entriesHASH_GRAPH_SERVICE_SECRETto start (parse-optional soserver --healthcheckworks without it), and an empty configured secret never matches — a set-but-empty variable cannot silently disable the credentialDebugoutput.httpyac.config.jshook, andservice-delegation.httppins the rejection paths end-to-endhide_env_valueson the five sensitive clap env args (Postgres password ×2, OpenAI key, Mailchimp key, Sentry DSN) —--helpno longer prints their valuesPre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
client_credentials(BE-728 (internal)).🐾 Next steps
client_credentialsfor service identity (BE-728) (internal)🛡 What tests cover this?
delegation.rsunit tests: the full header matrix — pair verifies, wrong secret, actor header without secret (the no-fallback case), secret without actor, malformed actor UUIDprovider.rschain tests: falls through onContinue, stops at the firstVerifiedorRejectedservice-delegation.httpend-to-end rejection paths with client-safe message asserts; the entire HTTP suite now runs through the delegation path via the injected secret❓ How to test this?
yarn start:graphorcargo run --bin hash-graph --all-features -- server)curl -i http://127.0.0.1:4000/actors/machine/identifier/system/h -H "X-HASH-Service-Secret: hash-svc-local-dev-secret"→ 200; without the header → 401X-Authenticated-User-Actor-Idbut without the secret → 401the actor-ID header requires the service credentialcd tests/graph/http && sh test.sh🐒 Demo
N/A — backend authentication path; see the httpyac suite output for the wire-level flow.