From 757ee332b7d2bdf88fa6281bd99efab72f1b82c4 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Thu, 6 Aug 2026 09:06:28 +0000 Subject: [PATCH] fix(server): authorize batch and transaction operations under dev-mode Closes #237. dev-mode is documented as opening authorization, and it did so for every single-item operation. BatchGetItem, BatchWriteItem, TransactGetItems and TransactWriteItems were still evaluated against IAM and denied, because their per-table authorization branch returns early and sat above the dev-mode check. The seeded dev user holds only SelfServicePolicy, so all four failed with AccessDeniedException while PutItem, GetItem, Query, Scan, UpdateItem and DeleteItem succeeded. Rather than add a second dev-mode check inside that branch, the gate now precedes every authorization branch. The defect was one of ordering, so a fix that leaves the ordering hazard in place invites the next branch to reintroduce it. key_info is still computed before the gate so the engine layer keeps its cache reuse, and both check_authorization call sites now sit below the gate. There are only two such call sites in the tree, both in this function, so the gate is now structurally unbypassable. Production behaviour is unchanged: with dev_mode false the batch branch runs exactly as before. Verified by running the batch/transaction authorization suite against a production-mode postgres server: 10 passed, 0 failed. Reproduced and fixed with a negative control. Against an unmodified main build the new test gives 4 failed, 1 passed, with the AccessDeniedException from the report; with the fix, 5 passed. The root cause the report identifies is that no CI job builds dev-mode, so nothing ever issued a request against a dev-mode server and a feature-matrix compile check passed while the path was broken. This adds a run-integration-dev-mode job that starts a zero-config dev-mode server and runs the new tests, and gates the integration aggregator on it. The tests are keyed on EXTENDDB_TEST_DEV_MODE so they skip against the production-mode servers the other jobs start, where these operations are correctly denied. --- .github/workflows/integration.yml | 61 +++++++++- crates/server/src/request_helpers.rs | 62 ++++++----- tests/test_dev_mode_authorization.py | 161 +++++++++++++++++++++++++++ 3 files changed, 256 insertions(+), 28 deletions(-) create mode 100644 tests/test_dev_mode_authorization.py diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 7c2533e8..ac68cc65 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -126,6 +126,58 @@ jobs: EXTENDDB_ADMIN_PASSWORD: ${{ steps.init.outputs.admin_password }} run: devtools/run-tests --extenddb --pytest --comprehensive --parallel --filter "not import_export" + run-integration-dev-mode: + # dev-mode was shipped with no CI coverage at all, which is how the batch and + # transaction authorization regression reached main: the build compiled, so + # a feature-matrix check passed, while nothing ever issued a request against + # a dev-mode server. This job starts one and exercises the data plane. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: dtolnay/rust-toolchain@stable + + - uses: Swatinem/rust-cache@v2 + with: + cache-on-failure: true + + - name: Build release (dev mode, in-memory SQLite) + run: cargo build --release -p extenddb --no-default-features --features sqlite-memory,dev-mode + + - name: Start ExtendDB in dev mode + run: | + # No config file on purpose: dev mode falls back to built-in defaults, + # which is the zero-config path a user gets, and the one the profile + # documents as a DynamoDB Local replacement. + ./target/release/extenddb serve --foreground --port 18444 --config /nonexistent.toml & + for i in $(seq 1 30); do + if curl -s http://127.0.0.1:18444/health | grep -q healthy; then + echo "Server ready" + exit 0 + fi + sleep 1 + done + echo "Server failed to start" + exit 1 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install Python dependencies + run: pip install -r requirements.txt + + - name: Run dev-mode authorization tests + env: + EXTENDDB_TEST_ENDPOINT: http://127.0.0.1:18444 + EXTENDDB_TEST_DEV_MODE: "1" + AWS_DEFAULT_REGION: us-east-1 + # The seeded zero-config dev credential. AWS's documented example key, + # which grants nothing anywhere; it exists so SigV4 has a key to verify. + AWS_ACCESS_KEY_ID: AKIAIOSFODNN7EXAMPLE + AWS_SECRET_ACCESS_KEY: wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY + run: python3 -m pytest tests/test_dev_mode_authorization.py -v + run-rust-integration: runs-on: ubuntu-latest services: @@ -213,12 +265,19 @@ jobs: integration: runs-on: ubuntu-latest - needs: [run-integration, run-integration-sqlite, run-rust-integration] + needs: + [ + run-integration, + run-integration-sqlite, + run-integration-dev-mode, + run-rust-integration, + ] if: always() steps: - run: | if [ "${{ needs.run-integration.result }}" != "success" ] || \ [ "${{ needs.run-integration-sqlite.result }}" != "success" ] || \ + [ "${{ needs.run-integration-dev-mode.result }}" != "success" ] || \ [ "${{ needs.run-rust-integration.result }}" != "success" ]; then exit 1 fi diff --git a/crates/server/src/request_helpers.rs b/crates/server/src/request_helpers.rs index 733c9397..923b5e48 100755 --- a/crates/server/src/request_helpers.rs +++ b/crates/server/src/request_helpers.rs @@ -62,34 +62,7 @@ pub(crate) async fn authorize_request( operation: &str, account_id: &str, ) -> Result, DynamoDbError> { - // Batch and transaction operations address multiple tables in nested request - // structures with no top-level TableName. DynamoDB authorizes each of them - // per table, against that table's specific ARN, using the IAM action the - // operation maps to (BatchGetItem/BatchWriteItem are their own actions; - // TransactGetItems decomposes to GetItem; TransactWriteItems decomposes to - // the per-sub-op action). Authorizing the whole request against a single - // `table/*` wildcard — as the generic path below does — lets an explicit - // Deny on one table be bypassed. Evaluate every (action, table) pair and - // reject the entire request if any is denied (all-or-nothing), matching - // DynamoDB. Verified against the AWS IAM Service Authorization Reference. - if let Some(targets) = batch_transact_authz_targets(operation, input) { - for (action_op, table) in targets { - let resource_arn = build_resource_arn(&state.region, account_id, Some(&table)); - authorization::check_authorization( - state.authz_cache.as_ref(), - identity, - &action_op, - &resource_arn, - false, - extenddb_auth::policy::context::RequestParams::default(), - ) - .await?; - } - return Ok(None); - } - let table_name = extract_table_name(input); - let resource_arn = build_resource_arn(&state.region, account_id, table_name.as_deref()); // P118: Fetch table_key_info for item-level operations via the SWR cache. // The result is used for LeadingKeys extraction here AND returned to the @@ -112,10 +85,45 @@ pub(crate) async fn authorize_request( // verification already ran upstream, so the request is authenticated; only // the IAM policy decision is skipped. key_info is still returned so the // engine layer can reuse it. + // + // This check is deliberately positioned ahead of EVERY authorization branch + // below, not merely the generic one. It previously sat after the batch and + // transaction branch, which returns early, so those four operations were + // still evaluated against IAM and denied in a dev-mode build while single + // item operations passed. Any future operation-specific branch must be added + // below this point, or it will reintroduce that defect. if state.dev_mode { return Ok(key_info); } + // Batch and transaction operations address multiple tables in nested request + // structures with no top-level TableName. DynamoDB authorizes each of them + // per table, against that table's specific ARN, using the IAM action the + // operation maps to (BatchGetItem/BatchWriteItem are their own actions; + // TransactGetItems decomposes to GetItem; TransactWriteItems decomposes to + // the per-sub-op action). Authorizing the whole request against a single + // `table/*` wildcard, as the generic path below does, lets an explicit + // Deny on one table be bypassed. Evaluate every (action, table) pair and + // reject the entire request if any is denied (all-or-nothing), matching + // DynamoDB. Verified against the AWS IAM Service Authorization Reference. + if let Some(targets) = batch_transact_authz_targets(operation, input) { + for (action_op, table) in targets { + let resource_arn = build_resource_arn(&state.region, account_id, Some(&table)); + authorization::check_authorization( + state.authz_cache.as_ref(), + identity, + &action_op, + &resource_arn, + false, + extenddb_auth::policy::context::RequestParams::default(), + ) + .await?; + } + return Ok(None); + } + + let resource_arn = build_resource_arn(&state.region, account_id, table_name.as_deref()); + let pk_attr = key_info .as_ref() .map(|ki| ki.key_schema[0].attribute_name.clone()); diff --git a/tests/test_dev_mode_authorization.py b/tests/test_dev_mode_authorization.py new file mode 100644 index 00000000..354fffff --- /dev/null +++ b/tests/test_dev_mode_authorization.py @@ -0,0 +1,161 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""Developer-mode authorization tests. + +`dev-mode` opens the IAM policy decision for an authenticated caller: SigV4 is +still verified, but no `dynamodb:*` policy is required. The seeded `dev` user +holds only `SelfServicePolicy` (four `iam:*` actions on its own ARN), so any +operation that still reaches the IAM evaluator is denied. That makes an +`AccessDeniedException` here proof that an operation is bypassing the dev-mode +gate, not a policy misconfiguration. + +Batch and transaction operations regressed exactly this way: their per-table +authorization branch returned before the dev-mode check was reached, so +`BatchGetItem`, `BatchWriteItem`, `TransactGetItems` and `TransactWriteItems` +failed while every single-item operation succeeded. + +These require a server built and started in dev mode, so they are gated on +EXTENDDB_TEST_DEV_MODE and excluded from the backend-agnostic suite, which runs +against a production-mode server where these operations are correctly denied. +""" + +from __future__ import annotations + +import os + +import pytest +from botocore.exceptions import ClientError + +from conftest import wait_for_active + +pytestmark = pytest.mark.skipif( + os.environ.get("EXTENDDB_TEST_DEV_MODE", "").strip() != "1", + reason="requires a dev-mode server (set EXTENDDB_TEST_DEV_MODE=1)", +) + + +def _assert_not_denied(op_name: str, call): + """Run `call`, failing loudly if the operation was denied rather than served. + + Any other ClientError is re-raised: this test is about the authorization + decision, so a genuine request error should not be swallowed into a pass. + """ + try: + return call() + except ClientError as exc: + code = exc.response["Error"]["Code"] + if code in ("AccessDeniedException", "UnrecognizedClientException"): + pytest.fail( + f"{op_name} was denied in dev mode with {code}: " + f"{exc.response['Error'].get('Message', '')}" + ) + raise + + +@pytest.fixture() +def dev_table(dynamodb_client, unique_table_name): + dynamodb_client.create_table( + TableName=unique_table_name, + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "S"}], + BillingMode="PAY_PER_REQUEST", + ) + wait_for_active(dynamodb_client, unique_table_name) + yield unique_table_name + try: + dynamodb_client.delete_table(TableName=unique_table_name) + except ClientError: + pass + + +class TestDevModeOpensAuthorization: + """Every data-plane operation is served in dev mode, not just single-item ones.""" + + def test_single_item_operations_are_not_denied(self, dynamodb_client, dev_table): + # These already worked. They are kept so a regression that closes + # authorization altogether is distinguishable from one that affects only + # the batch and transaction branch. + _assert_not_denied( + "PutItem", + lambda: dynamodb_client.put_item( + TableName=dev_table, Item={"pk": {"S": "a"}, "v": {"N": "1"}} + ), + ) + _assert_not_denied( + "GetItem", + lambda: dynamodb_client.get_item(TableName=dev_table, Key={"pk": {"S": "a"}}), + ) + _assert_not_denied( + "UpdateItem", + lambda: dynamodb_client.update_item( + TableName=dev_table, + Key={"pk": {"S": "a"}}, + UpdateExpression="SET #v = :v", + ExpressionAttributeNames={"#v": "v"}, + ExpressionAttributeValues={":v": {"N": "2"}}, + ), + ) + _assert_not_denied( + "Query", + lambda: dynamodb_client.query( + TableName=dev_table, + KeyConditionExpression="pk = :p", + ExpressionAttributeValues={":p": {"S": "a"}}, + ), + ) + _assert_not_denied("Scan", lambda: dynamodb_client.scan(TableName=dev_table)) + _assert_not_denied( + "DeleteItem", + lambda: dynamodb_client.delete_item( + TableName=dev_table, Key={"pk": {"S": "a"}} + ), + ) + + def test_batch_write_item_is_not_denied(self, dynamodb_client, dev_table): + resp = _assert_not_denied( + "BatchWriteItem", + lambda: dynamodb_client.batch_write_item( + RequestItems={ + dev_table: [ + {"PutRequest": {"Item": {"pk": {"S": "b1"}}}}, + {"PutRequest": {"Item": {"pk": {"S": "b2"}}}}, + ] + } + ), + ) + assert not resp.get("UnprocessedItems", {}).get(dev_table) + + def test_batch_get_item_is_not_denied(self, dynamodb_client, dev_table): + dynamodb_client.put_item(TableName=dev_table, Item={"pk": {"S": "g1"}}) + resp = _assert_not_denied( + "BatchGetItem", + lambda: dynamodb_client.batch_get_item( + RequestItems={dev_table: {"Keys": [{"pk": {"S": "g1"}}]}} + ), + ) + assert len(resp["Responses"][dev_table]) == 1 + + def test_transact_write_items_is_not_denied(self, dynamodb_client, dev_table): + _assert_not_denied( + "TransactWriteItems", + lambda: dynamodb_client.transact_write_items( + TransactItems=[ + {"Put": {"TableName": dev_table, "Item": {"pk": {"S": "t1"}}}} + ] + ), + ) + got = dynamodb_client.get_item(TableName=dev_table, Key={"pk": {"S": "t1"}}) + assert "Item" in got + + def test_transact_get_items_is_not_denied(self, dynamodb_client, dev_table): + dynamodb_client.put_item(TableName=dev_table, Item={"pk": {"S": "tg1"}}) + resp = _assert_not_denied( + "TransactGetItems", + lambda: dynamodb_client.transact_get_items( + TransactItems=[ + {"Get": {"TableName": dev_table, "Key": {"pk": {"S": "tg1"}}}} + ] + ), + ) + assert resp["Responses"][0]["Item"]["pk"]["S"] == "tg1"