fix(server): authorize batch and transaction operations under dev-mode - #240
Merged
LeeroyHannigan merged 1 commit intoAug 6, 2026
Merged
Conversation
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.
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn and
yesyayen
as code owners
August 6, 2026 10:32
pdf-amzn
approved these changes
Aug 6, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
crates/server/src/request_helpers.rs- the dev-mode gate inauthorize_requestnow precedes every authorization branch instead of only the generic one. Batch and transaction operations address multiple tables in nested request structures, so they are authorized per table in a branch that returns early, and that branch sat above the gate.BatchGetItem,BatchWriteItem,TransactGetItemsandTransactWriteItemswere therefore still evaluated against IAM and denied.The obvious fix is a second
if state.dev_modeinside that branch. I did not do that. The defect is one of ordering, so a fix that leaves the ordering hazard in place invites the next operation-specific branch to reintroduce it.key_infois still computed before the gate so the engine layer keeps its cache reuse, and bothcheck_authorizationcall sites now sit below it. There are only two such call sites in the tree, both in this function, so the gate is now structurally unbypassable rather than merely correct today. The comment at the gate says so, for whoever adds the next branch.tests/test_dev_mode_authorization.py- new. Asserts that all four previously denied operations are served, with the single-item operations kept alongside them so a regression that closes authorization altogether is distinguishable from one that affects only the batch and transaction branch. The seededdevuser holds onlySelfServicePolicy, fouriam:*actions on its own ARN and nodynamodb:*, which is what makes anAccessDeniedExceptionhere proof that an operation bypassed the gate rather than evidence of a policy misconfiguration..github/workflows/integration.yml- newrun-integration-dev-modejob, and theintegrationaggregator now gates on it.Why
dev-modeis documented as opening authorization.crates/storage-sqlite/README.mdcalls the profile "a drop-in replacement for DynamoDB Local: plain HTTP on loopback, open authorization, and a seeded credential", and the startup banner says "authorization open (SigV4 still enforced)". That held for single-item operations and not for the four above, so anyone evaluating the profile as a DynamoDB Local replacement hitAccessDeniedExceptionon the first batch write.The reason this shipped is worth stating plainly, because it is the part that generalises. The only thing that sets
dev_mode = trueiscfg!(feature = "dev-mode")incmd_serve.rs. Across every workflow, CI runs exactly four cargo invocations:cargo build --release,cargo build --release -p extenddb --no-default-features --features sqlite,cargo test --workspace, andcargo clippy --all-targets. None of them enabledev-mode, so every server CI has ever started ran withdev_mode = falseand the broken branch was unreachable. Independently,git grepfinds nothing in thetree that sets
dev_mode: true, so no unit test covered the gate either. The feature combination did get compiled during review, which is exactly the trap: a compile check cannot catch an ordering bug.That is why this PR adds the CI job rather than only the code fix. Without it the next regression is invisible in the same way.
Closes #237
Testing done
The fix, with a negative control. Built the same dev-mode binary from unmodified
mainand from this branch(
--no-default-features --features sqlite-memory,dev-mode), started each with no config file so it uses the built-in defaults, and ran the new tests against both:mainThe failures on
maincarry the exact error from the report, for exampleAccessDeniedException: User: arn:aws:iam::...:user/dev is not authorized to perform: dynamodb:BatchWriteItem. The one test that passes on both is thesingle-item one, which is the control.
Production authorization is unchanged. With
dev_modefalse the batch branch runs exactly as before, but since this moves code in the authorization path I verified the direction that matters rather than asserting it. Stood up a production-mode server on the postgres backend, minted a SigV4 key pair the way CI does, and ran the batch and transaction authorization suite, which asserts these operations are denied per table when the caller's policy does not allowthem:
cd tests/rust && cargo test batch_transact_authz -- --test-threads=1gives 10 passed, 0 failed.Gates.
cargo test --workspace: 668 passed, 0 failed, 0 filtered out.cargo fmt --all -- --check: clean, zero diffs.cargo clippy --all-targets -- -D warnings: clean.cargo clippy --workspace --all-targets -- -W clippy::pedantic: 421 warnings,identical to the count on
main. The two inrequest_helpers.rsarelarge future with a size of 20176 bytes, present onmainat the same size,so this change adds none.
The new CI job has never run. It builds the dev-mode target, starts a zero-config dev-mode server and runs the new tests. It is gated on
EXTENDDB_TEST_DEV_MODEso the file skips against the production-mode serversthe other jobs start, where these operations are correctly denied. If anything in the job definition is wrong it will surface on this PR, which is the right place for it.
Checklist
cargo test --workspace) - 668 passed, 0 filtered outcargo fmt --check)cargo clippy -- -W clippy::pedantic) - no new warningsversus
main, 421 on both sidesthe documented behaviour was already "authorization open", and this makes
the implementation match it. The comment at the gate now records the
ordering constraint
Storagetrait, auth model, on-diskformat, 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, and since this sits in the authorization path that deserves a
sentence rather than a bare "n/a". The auth model itself is unchanged and already
documented: in a
dev-modebuild SigV4 is verified and the IAM policy decision isskipped for the authenticated caller. Production authorization is untouched, as
the 10-test suite above confirms. What changes is that the implementation now
matches the documented model for four operations where it did not.
Breaking changes
None for any production build.
dev_modeis only ever true in adev-modebuild, andcrates/bin/src/main.rsfails compilation ifdev-modeis paired with a production backend such aspostgres, so the flag cannot be reached in aproduction binary. I verified that guard still holds:
cargo check -p extenddb --no-default-features --features postgres,dev-modefails with "thedev-modefeature requires a dev/CI backend such assqlite".Worth stating explicitly for reviewers of an auth change: this widens what a
dev-modebuild permits. Four operations that were denied are now served. That is the documented intent of the profile, and the surface is bounded by three independent constraints, all pre-existing:dev-modeis a compile-time feature gated to the sqlite backends, the serve path binds loopback only, and SigV4 verification still runs, so an unauthenticated request is still rejected.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.