Skip to content

fix: authorize batch and transaction operations per-table - #232

Merged
pdf-amzn merged 4 commits into
mainfrom
fix/batch-transact-authz
Jul 28, 2026
Merged

fix: authorize batch and transaction operations per-table#232
pdf-amzn merged 4 commits into
mainfrom
fix/batch-transact-authz

Conversation

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

What

Authorize batch and transaction operations against each table they touch, instead of a single wildcard resource.

  • BatchGetItem, BatchWriteItem, TransactGetItems and TransactWriteItems previously authorized once against the table/* wildcard, because table names were only extracted from a top-level TableName. These operations carry their
    tables in RequestItems / TransactItems, so a policy scoped to a specific table was not evaluated for them.
  • authorize_request now extracts every (action, table) pair for these four operations, authorizes each against its real per-table ARN, and denies the whole request if any single table is denied (all-or-nothing).
  • Actions follow the AWS IAM Service Authorization Reference: BatchGetItem/BatchWriteItem use their own action per table; TransactGetItems maps to GetItem per table; TransactWriteItems decomposes to PutItem/DeleteItem/UpdateItem/ConditionCheckItem per sub-item. Non-batch operations are unchanged.

Why

The ARN-addressed batch and transaction operations were the only data-plane paths not authorized per table, inconsistent with the single-item operations and with DynamoDB's IAM model. A policy statement scoped to one table had no effect on these operations. This brings them in line.

Closes #

Testing done

  • Rust integration tests (new, tests/rust/src/batch_transact_authz.rs): provision a user with an explicit Deny on one table via the management API and assert each of the four operations is denied all-or-nothing when it touches that table, with positive controls (a request touching only allowed tables succeeds). Guard tests confirm the action decomposition: a GetItem deny does not block BatchGetItem but does block TransactGetItems; a PutItem deny does not block
    BatchWriteItem but does block TransactWriteItems. 8/8 pass against a live ExtendDB server.
  • Full Rust integration suite: 399 passed, 0 failed (no regression on the shared authz path).
  • cargo fmt --all -- --check clean; cargo clippy --workspace --all-targets 0 warnings.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean -- CI gate cargo clippy --workspace --all-targets passes with 0 warnings
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed -- n/a (authorization correctness fix; no user-facing docs or config surface affected)
  • Breaking changes are noted below (if any) -- none
  • If this changes the wire protocol, Storage trait, auth model, on-disk format, or public CLI surface, an RFC has been accepted or is linked below. Otherwise, an ADR captures the decision (link below).

ADR / RFC: n/a -- no trait, wire, on-disk, or CLI change. This is a correctness
fix that enforces the existing authorization model on the batch/transact paths
(no new auth model); an ADR can be added if a reviewer wants the decision recorded.

Breaking changes

None. No public interface changes; behavior change is limited to correctly
denying batch/transaction requests that touch a table the caller is denied.


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

BatchGetItem, BatchWriteItem, TransactGetItems and TransactWriteItems
were authorized once against the table/* wildcard resource, because
extract_table_name only reads a top-level TableName. Batch/transact
bodies carry their tables in RequestItems / TransactItems, so an explicit
Deny scoped to a specific table was never evaluated and could be bypassed.

authorize_request now extracts every (action, table) pair for these four
operations and authorizes each against its real per-table ARN, denying the
whole request if any table is denied (all-or-nothing). Actions follow the
AWS IAM Service Authorization Reference: BatchGetItem/BatchWriteItem use
their own action per table; TransactGetItems maps to GetItem per table;
TransactWriteItems decomposes to PutItem/DeleteItem/UpdateItem/
ConditionCheckItem per sub-item. Non-batch operations are unchanged.

Adds Rust integration tests that provision a user with an explicit Deny on
one table via the management API and assert all-or-nothing denial across
all four operations, plus guards proving the action decomposition (a
GetItem deny does not block BatchGetItem but does block TransactGetItems).
The batch_transact_authz tests provision their own Deny-policy user via the
management API, so they need EXTENDDB_ADMIN_PASSWORD at test runtime. They
previously called .expect() on it and hard-panicked under a bare `cargo test`,
where only devtools/run-tests injects that variable.

Match the existing Python auth suite (test_abac.py, test_auth_integration.py),
which skips when EXTENDDB_ADMIN_PASSWORD is unset. A bare `cargo test` now
skips these cleanly; devtools/run-tests --extenddb --rust-integration still
sets the password and runs them for real, so CI coverage is unchanged.
("Put", "PutItem"),
("Delete", "DeleteItem"),
("Update", "UpdateItem"),
("ConditionCheck", "ConditionCheckItem"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ConditionCheck sub-op in TransactWriteItems maps to IAM action dynamodb:ConditionCheck, not dynamodb:ConditionCheckItem. The AWS IAM Service Authorization Reference lists ConditionCheckItem as the resource type required by the dynamodb:ConditionCheck action — not as the action name itself. The action name is dynamodb:ConditionCheck.

This bug is not caught by the Rust tests in tests/rust/src/batch_transact_authz.rs because those tests deny/allow dynamodb:*, which matches any action name. A policy written as Allow dynamodb:ConditionCheck on table/X would fail to authorize with the ConditionCheckItem action string.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ConditionCheck sub-op does authorize under dynamodb:ConditionCheckItem, which is what this code produces. Per the AWS DynamoDB developer guide, Using IAM with DynamoDB transactions:

Permissions for Put, Update, Delete, and Get actions are governed by the permissions used for the underlying PutItem, UpdateItem, DeleteItem, and GetItem operations. For the ConditionCheck action, you can use the dynamodb:ConditionCheckItem permission in IAM policies.

There is no dynamodb:ConditionCheck IAM action, and ConditionCheckItem is an action name, not a resource type (DynamoDB's resource types are table, index, stream, backup, global-table).

You're right, though, that the existing dynamodb:* tests can't pin the exact action string. I've added two guard tests that do (7dbf23b):

  • deny exactly dynamodb:ConditionCheckItem → the TransactWriteItems ConditionCheck is denied
  • deny the (non-existent) dynamodb:ConditionCheck string → the operation is not denied

Both pass live against a running server, so the mapping is now pinned to the correct action rather than relying on the wildcard. Let me know if that resolves it.

LeeroyHannigan and others added 2 commits July 28, 2026 17:00
The existing all-or-nothing tests deny/allow dynamodb:*, which matches any
action string, so they cannot pin the exact IAM action each sub-op authorizes
under. Add two guards for the TransactWriteItems ConditionCheck sub-op:

- deny exactly dynamodb:ConditionCheckItem -> the ConditionCheck is denied
- deny the non-existent dynamodb:ConditionCheck string -> NOT denied

Together these prove the sub-op authorizes as dynamodb:ConditionCheckItem, per
the AWS IAM Service Authorization Reference ("For the ConditionCheck action,
you can use the dynamodb:ConditionCheckItem permission in IAM policies").
@pdf-amzn
pdf-amzn added this pull request to the merge queue Jul 28, 2026
Merged via the queue into main with commit 330ca12 Jul 28, 2026
12 checks passed
LeeroyHannigan added a commit that referenced this pull request Aug 11, 2026
…tests stop skipping

`batch_transact_authz` has reported green in every CI run since it landed in #232 on
2026-07-28 while executing nothing. All ten of its tests call `skip_no_admin()`,
which returns early when `EXTENDDB_ADMIN_PASSWORD` is unset, and neither rust
integration job set it.

The suite is not at fault and neither is its hardcoded account. Its own comment says
to run it via `devtools/run-tests --extenddb --rust-integration`, and that path is
correct: `devtools/provision-test-credentials` creates account 123456789012 with an
IAM user, access key and full-access policy, then exports the credentials. The suite
targets exactly that account.

What was wrong is that both jobs invoked `cd tests/rust && cargo test` directly,
bypassing the harness, so neither the account nor the password ever existed. Each job
then hand-rolled its own IAM provisioning against whatever random account `init`
generated, which is enough for the suites that only need a data-plane caller and not
enough for one that needs a known account.

So both jobs now call the harness, and their bespoke provisioning steps are deleted
rather than kept alongside it. Confirmed locally against a clean deployment: ten of
ten authz tests execute and pass, and the run reports zero SKIP lines where it
previously printed ten.

The vector backfill tests go back to reading `EXTENDDB_ADMIN_PASSWORD`. They briefly
used a separate `EXTENDDB_TEST_MGMT_PASSWORD` on the theory that the standard name
would un-skip a broken suite and turn CI red. That theory was wrong: the suite passes
once the harness provisions its account, so the workaround is removed rather than
left in place. Their guard still hard-fails when the variable is absent and
`EXTENDDB_EXPECT_VECTORS=1`, and now names the harness in the message.

Verification, run exactly as the rewritten job does
(`devtools/run-tests --extenddb --rust-integration --release`): 451 passed, 0
filtered out, 0 SKIP. The single failure is `restore_active_completeness`, which
passes 1/1 in isolation, fails the same way under the previous bare `cargo test`
invocation, and is unrelated to this change.
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