Skip to content

PartiQL (#411): measure the order of three refusals against authorization #888

Open

Description

@LeeroyHannigan

Describe the bug

Follow-up to #411. ExtendDB at 0e4b4a8 refuses three classes of statement before it authorizes the caller:

  • A call in a SET value: UPDATE "<t>" SET a = size(m) WHERE pk = 'h' AND sk = 's', refused with The function is not allowed in an update expression; function: size.
  • An empty bag: INSERT INTO "<t>" VALUE {'pk': 'a', 'sk': 'b', 'v': <<>>}, refused with Empty bags are not supported.
  • A parameter refused for its content: UPDATE "<t>" SET v = ? WHERE pk = 'h' AND sk = 's' with Parameters [{"SS": []}], refused with One or more parameter values were invalid: An string set may not be empty.

A caller with no PartiQL permission on the table gets the ValidationException. Whether the service answers that caller with the ValidationException or with AccessDeniedException is not measured. The branch's tests run with a caller that may run every statement.

Where the refusals sit at 0e4b4a8:

  • ExecuteStatement (execute_statement, crates/engine/src/partiql/mod.rs): check_request_parameter_types, the request checks, check_parameters (the content of the parameters), the parse and check_statement run before Gate::new builds the authorization gate (crates/engine/src/partiql/authorize/gate.rs). check_statement runs check_condition_values and check_written_values (crates/core/src/partiql/values.rs), which refuse the call and the empty bag. A refusal from any of these returns at once.
  • BatchExecuteStatement (batch::run, crates/engine/src/partiql/batch.rs): the same checks give the statement a parse_err entry, ValidationError with the text and no TableName, which the batch pushes without a gate. The other statements run. A statement that passed these checks gets a gate: its translation errors (Translation::Untranslated, Translation::Refused) and its run go through Gate::settle.
  • ExecuteTransaction (parse_member, crates/engine/src/partiql/transaction.rs): check_parameters and check_statement run in the parse of each member, before authorize_members. The content refusal fails the request without a position. The call and the empty bag fail it with the Validation failed in TransactStatements[i]: prefix.

gate.rs places authorization among the checks. The gate answers a refusal at once when the service makes that check before authorization. It holds every other refusal until the authorization of the statement, so a caller with no permission gets AccessDeniedException (Gate::settle). made_before_authorization lists the checks that a measurement of 2026-11-03 placed before authorization:

  • fewer parameters than placeholders
  • SELECT COUNT(*)
  • a strongly consistent read of a global secondary index
  • a path into a key attribute
  • overlapping disjuncts, and more than 50 disjuncts
  • a NextToken that is invalid or another statement's
  • a DELETE with a RETURNING clause other than ALL OLD *
  • a batch read without the full primary key

Its comment states that the checks of a request, the parse of a statement and the checks that need no catalog come before any gate. The three refusals of this issue are in that last group, and no measurement covers them.

Before the commits of this branch that refuse them in check_statement, ExecuteStatement refused a call in a SET value and an empty bag later. The classic request that write::run sends after gate.pass() (crates/engine/src/partiql/write/update.rs and insert.rs) refused them, so a caller with no permission got AccessDeniedException. In a batch they failed the whole request. This comes from a reading of the code at the branch's merge base. The measurements of the branch did not cover it.

To Reproduce

  1. Start ExtendDB at 0e4b4a8 with the SQLite backend.
  2. Create a table with pk S hash key and sk S range key.
  3. From a principal that has no PartiQL permission on the table, send each of the three statements above. Send each in ExecuteStatement, in a BatchExecuteStatement between two valid INSERT statements, and as a member of an ExecuteTransaction.
  4. ExtendDB answers the ValidationException of the statement in each operation. In the batch it is the statement's entry, and the two inserts run.

Expected behavior

The measurement in Proposed change decides it. If the service answers AccessDeniedException, that is the expected answer in each operation. If it answers the ValidationException, the order at 0e4b4a8 matches the service.

Actual behavior

The ValidationException of the statement from every caller, in the three operations, before authorization.

Environment

Proposed change

Measure before any code change. Send each of the three statements from a principal that has no PartiQL permission on the table, and record the answer:

  • in ExecuteStatement
  • in a BatchExecuteStatement with a valid INSERT before and after it
  • as the only member of an ExecuteTransaction, and as the second member after a valid UPDATE

The answer to compare is AccessDeniedException or the ValidationException of the statement. Add a control statement with no fault from the same principal, to show that it gets AccessDeniedException.

Then, for each class:

  • If the service answers the ValidationException, the order at 0e4b4a8 stands. Record the measurement in gate.rs, next to the checks listed as made before authorization.
  • If the service answers AccessDeniedException, move the check behind the gate. In ExecuteStatement, run the written-value checks and check_parameters after Gate::new and pass their error to Gate::settle, which answers AccessDeniedException for an error that made_before_authorization does not list. In the batch, give such a statement a gate, as a Translation::Untranslated error has one, instead of a parse_err entry. In the transaction, run the check after authorize_members.

The place of these checks among the others (the table name rule, a projected call, the ordering operand types, overlapping paths) stays as check_statement orders them. This issue covers their place against authorization only.

Additional context

#886 covers the order of the first refusal among two faults in one statement. The change proposed there moves the call and empty bag checks into the parser's walk. The walk also runs before the gate, so this issue applies to it unchanged.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions