Skip to content

test(acl): cover who may create a data set, and by which route - #110

Merged
olavgg merged 1 commit into
mainfrom
test/dataset-create-acl
Sep 2, 2026
Merged

test(acl): cover who may create a data set, and by which route#110
olavgg merged 1 commit into
mainfrom
test/dataset-create-acl

Conversation

@JosteinGj

Copy link
Copy Markdown
Contributor

The suite proved a per-data-set write grant cannot create a data set, and nothing else. That leaves three gaps.

  • No allow side. A rule that only ever denies is indistinguishable from one that denies everyone: the suite would stay green through a change that made data sets uncreatable, with the failures landing in unrelated fixtures where they read as fixture breakage.
  • Only the near-miss principal. MT_WRITEONLY holds a write grant, just a scoped one. The plain cases - read-only and no-grant - are what would catch a check keyed on "holds any write grant" rather than on the blanket one.
  • Only one of the two routes. /resources/create gates DATASET/POLICY nodes with its own check, so proving /datasets/create is guarded says nothing about it. That route is the one the typed node surface makes natural - resources.create(vec![Dataset::new(..)]).

Three Rust tests and one Python one. The Python side stays deliberately thin, per that file's own selection principle: it covers the /resources route because that is a different binding path (the typed node union), not because the backend rule needs re-proving.

Two things the writing found, both now pinned in place rather than in a comment somewhere:

  • The lowercase-label case is deliberate. A type-label is canonicalised before it is matched, so a gate comparing the raw string would let dataset through.
  • A policy create needs its type. Bean validation runs before the ACL gate, so a policy without one is a 400 naming nodes[0].type and never reaches the permission check - a test built on an invalid body would pass against a backend with no gate at all.

Rust: 17 ACL tests pass (was 14). Python: 10 (was 9).

The suite proved a per-data-set write grant cannot create a data set, and
nothing else. That leaves three gaps.

- **No allow side.** A rule that only ever denies is indistinguishable from
  one that denies everyone: the suite would stay green through a change that
  made data sets uncreatable, with the failures landing in unrelated fixtures
  where they read as fixture breakage.
- **Only the near-miss principal.** MT_WRITEONLY holds a write grant, just a
  scoped one. The plain cases - read-only and no-grant - are what would catch
  a check keyed on "holds any write grant" rather than on the blanket one.
- **Only one of the two routes.** `/resources/create` gates DATASET/POLICY
  nodes with its own check, so proving `/datasets/create` is guarded says
  nothing about it. That route is the one the typed node surface makes
  natural - `resources.create(vec![Dataset::new(..)])`.

Three Rust tests and one Python one. The Python side stays deliberately thin,
per that file's own selection principle: it covers the `/resources` route
because that is a different binding path (the typed node union), not because
the backend rule needs re-proving.

Two things the writing found, both now pinned in place rather than in a
comment somewhere:

- The lowercase-label case is deliberate. A type-label is canonicalised
  before it is matched, so a gate comparing the raw string would let
  `dataset` through.
- A policy create needs its `type`. Bean validation runs *before* the ACL
  gate, so a policy without one is a 400 naming `nodes[0].type` and never
  reaches the permission check - a test built on an invalid body would pass
  against a backend with no gate at all.

Rust: 17 ACL tests pass (was 14). Python: 10 (was 9).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JosteinGj
JosteinGj requested a review from olavgg September 2, 2026 10:53
@olavgg
olavgg merged commit 79173af into main Sep 2, 2026
17 of 18 checks passed
@olavgg
olavgg deleted the test/dataset-create-acl branch September 2, 2026 11:14
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