Skip to content

fix(catalog): vended credentials follow catalog grants - #760

Merged
viethqb merged 8 commits into
mainfrom
fix/vended-credentials-follow-grants
Oct 27, 2026
Merged

viethqb merged 8 commits into
mainfrom
fix/vended-credentials-follow-grants

Conversation

@viethqb

@viethqb viethqb commented Oct 27, 2026

Copy link
Copy Markdown
Member

Summary

A catalog client holding only a read grant on a table (or on its namespace) loaded the table and was handed STS credentials with Put and Delete on the table's prefix, with or without the X-Iceberg-Access-Delegation header. Measured on an own stack: the reader overwrote a data file, deleted a data file and deleted the metadata file, and the table could no longer be scanned by anyone.

Two causes, both fixed here:

  • Lakekeeper picks the credentials' scope from its authorizer (WriteData → read-write-delete, ReadData → read), and egret ran it on AllowAllAuthorizer.
  • The mount marked every request LakekeeperInternal, which skips the authorizer for every table action, so even a real authorizer would never have been asked.

Spec: docs/en/specs/2026-10-27-vended-credentials-follow-grants-design.md (decisions D1–D4, controller defaults C1–C14, per-method authorizer table, mutation, gate and live check recorded). Plan: docs/en/plans/2026-10-27-vended-credentials-follow-grants.md.

On main 827da4bb, after F-A (#758).

No migration. No API shape change. Behaviour change: a read grant now vends read-only storage credentials, and an engine attached with a read-only role can no longer write.

Changes (egret-catalog-engine, egret-bin)

  • EgretAuthorizer (authorizer.rs) replaces AllowAllAuthorizer. It answers ReadData and WriteData on a table from catalog.catalog_grants, by the guard's own rule. Every other question it answers as AllowAll did, because the guard has already decided that request (the spec lists each method and why).
  • One grant rule. grants_write, catalog_grant_covers_by and CatalogNameEq moved from the guard to egret_catalog_engine::grant_match, so the guard's decision and the vended scope cannot disagree. Names compare by Lakekeeper's collation in both (F-A D12).
  • The mount names the caller. CatalogCaller gains warehouse and role. For an allowed request the mount hands Lakekeeper a principal (egret-catalog~<client>) with a claim egret_catalog_client {warehouse, role, subject}, built from public Lakekeeper APIs. GET /config keeps the internal actor: it vends nothing, and with a principal Lakekeeper would register every catalog client as a Lakekeeper user.
  • Who else asks. egret's system principal (browse, recovery, warehouse admin, lineage sweep) keeps full access, as before. Any other principal gets no data access (fail closed). A data question Lakekeeper asks on behalf of another user (a DEFINER chain or a for-user check, neither reachable in egret today) is refused.
  • Comments that described the old allow-all behaviour (storage profile, signer, mount) were corrected.

Decisions

The user decided (2026-10-27): approach (a), an egret authorizer plus a real caller principal; read grants get read-only credentials; an engine with a read-only role becomes read-only (egret-provisioned Trino and StarRocks hold catalog manage and are unchanged); the credential lifetime is a separate follow-up. Controller defaults C1–C14 are in the spec, each with its reason.

Test plan

New tests in crates/egret-bin/src/catalog_vended_credentials_tests.rs run the real guard, the table hook and Lakekeeper's router (mount_catalog_engine) on a warehouse with egret's S3 profile over MockS3: an in-process S3 and STS that records the session policy of every key Lakekeeper mints, and enforces it as RustFS does. Lakekeeper's own storage validation of the warehouse runs against it.

  • Red first. On the F-A head, 6 of the first 11 tests failed exactly on the defect (RWD policy for a table reader, a reader with no header, a namespace reader, a catalog-read engine, /credentials, and a reader handed the writer's cached key). The 5 controls passed.
  • Pins.
    • Write, namespace-write and catalog-manage clients keep RWD.
    • egret's own browse load still mints RWD.
    • A config call registers no Lakekeeper user.
    • A grant spelled in another case vends by the catalog's collation.
    • Lakekeeper's credential cache is keyed by permission, so a reader after a writer gets its own read-only key.
    • Create and stage-create on another table's location or its parent, register of another table's metadata file, and register of a forged metadata file on another table's location all get 409. No key minted in the attempt reaches that table.
  • After review.
    • A mixed role (namespace read plus write on one table) gets RWD on that table and read-only on its sibling.
    • A view load is read-only, even for a namespace writer.
    • A grant-store error fails the decision.
    • A data question about another user is refused. This test was seen failing first.
    • Through Lakekeeper's router, an unknown principal and a client of another warehouse get no data access.
    • The config test now checks only its own token's principal.
  • Mutation. 15 mutants of the decision branches on the final code, all killed. This includes the warehouse re-check, which had survived before the review.

Full local gate on the pre-review code (4599c6c5; before the rebase this was 12ba90c1, with identical code), against the real test DB with no DB skips:

check result
cargo fmt --check, clippy over the workspace with -D warnings, just test-shipping-resolution pass
cargo test --workspace --no-fail-fast 8387 passed, 0 DB skips. 9 failed: 7 are sql_starrocks_live, which needs a live StarRocks, as on earlier PRs. 2 are timing bounds that failed at a load average of 20–40 caused by other sessions: a_rename_with_nothing_bound_moves_at_once passed 3/3 alone, and overflow_records_never_hold_more_connections_than_the_cap passed 6/7 alone
just test-ranger-gates pass
Frontend npm ci, gen:api (no diff), lint, tsc -b, build, all 10 test:bundle* pass
vitest 8487/8500 under load (all 13 failures were 5 s timeouts); every failing file passes when rerun alone. This PR changes no frontend file

After the review, on bd80dbe5 (the last code commit): cargo fmt --check pass, workspace clippy -D warnings pass, egret-bin catalog_ tests 58 passed, egret-catalog-engine 155 passed / 0 failed, no DB skips. The full workspace gate and the live check were not repeated: the only change to the vending decision refuses questions egret cannot reach today.

Live check

On an own stack (egret-f4creds), with RustFS, pyiceberg 0.12 and a Trino 483 engine provisioned by egret. Findings M1–M4 were repeated on the fixed code.

  • The table reader, with the header, without it, and through /credentials, now gets a session policy of s3:GetObject, s3:GetObjectVersion only.
    • PUT under its own table is 403.
    • Overwriting a data file, deleting a data file and deleting the metadata file are all 403.
    • It still reads: a pyiceberg scan returns 6 rows. A pyiceberg append fails with ACCESS_DENIED.
  • The namespace reader is read-only on every table in the namespace.
  • The table and namespace writers get RWD, and their pyiceberg appends succeed.
  • Nothing was damaged. Afterwards all tables scan, with the expected row counts.
  • Trino (catalog manage) ran CREATE, INSERT, UPDATE, DELETE, RENAME and DROP, and also INSERT into a pyiceberg-made table. Everything succeeded.
  • fix(catalog): classifications follow a renamed table or column #757: a classified column's tag moved with a Trino table rename.
  • egret's browse API still loads table details.

Review

An independent review found no HIGH. Each finding and how it was handled is in spec §11:

  1. Register onto another warehouse's location is possible when two warehouses share a bucket with differently written endpoints. This needs an admin misconfiguration and predates this branch. C8 is narrowed to one warehouse (measured and pinned), and the cross-warehouse case is recorded as a follow-up.
  2. The query count per load was wrong in C6. It is corrected to 4–6 egret queries (guard plus authorizer), on every load including client-managed ones.
  3. ActionOnTable.user was ignored. A data question about another user is now refused (red-first unit test).
  4. Test gaps were closed (see "After review" above).
  5. The rebased commit ids are explained in the spec.
  6. The behaviour to document is listed below.

Follow-ups

  • Credential lifetime (3600 s today; 900 s proposed): measure against long Trino queries first.
  • Callers with the same permission on the same table share one cached key.
  • Cross-warehouse register when two warehouses share a bucket (review 1): refuse overlapping storage connections, or refuse a register whose metadata location is outside the warehouse's own tables.
  • Read the grants once per request instead of in both the guard and the authorizer (review 2).
  • Docs (egret-data/docs, Platform): a read grant now vends read-only storage credentials, and an engine attached with a read-only role can no longer write. Closes docs KI-311.

Docs

  • egret-data/docs Platform section updated (KI-311: a read grant vends read-only credentials; an engine with a read-only role cannot write)

🤖 Generated with Claude Code

viethqb and others added 8 commits October 27, 2026 21:35
Move `grants_write`, `catalog_grant_covers_by` and `CatalogNameEq` from the
catalog guard in egret-bin to `egret_catalog_engine::grant_match`, so
lakekeeper's authorizer can decide by the same rule. `CatalogNameEq::load`
takes the requested names instead of the guard's needs. No behaviour change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A catalog client with only a read grant on a table (or its namespace)
loaded the table and was handed STS credentials with Put and Delete on
the table's prefix, with or without the delegation header, and could
overwrite and delete its data and metadata files. Lakekeeper picks the
credentials' scope from its authorizer's ReadData/WriteData answer, and
egret ran it on AllowAllAuthorizer behind a mount that marked every
request LakekeeperInternal, which skips the authorizer altogether.

EgretAuthorizer answers ReadData and WriteData on a table from
catalog.catalog_grants, by the guard's own rule (grant_match): a read
grant vends read-only credentials, a write or manage grant covering the
table read-write-delete. Every other question it answers as AllowAll did,
since the guard has already decided the request. egret's system
principal keeps full access; an unknown principal gets no data access.

The mount now hands lakekeeper a principal that names the catalog client
(warehouse, role, subject from the verified token) instead of the
internal actor, except on GET /config, which vends nothing and would
otherwise register every client as a lakekeeper user.

Tests drive the real guard and lakekeeper router over an in-process S3
and STS that records each session policy: read grants (table, namespace,
catalog), no header, /credentials, a reader after a writer (the STS
cache is keyed by permission), write and manage grants, egret's own
principal, and create/stage-create/register on another table's location.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A config call registers no lakekeeper user (the mount keeps the internal
actor there), and a grant spelled in another case still vends credentials
of its scope, as the guard's collation decides. Both were mutants of the
fix that no test killed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The design (an egret authorizer for lakekeeper, the caller principal the
mount hands it, the per-method table), the measurements before and after,
the red run, the mutation check, the local gate and the live check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lakekeeper passes a user in an authorization question only for a DEFINER
view chain or a for-user permission check. egret's grants belong to the
caller, so the authorizer now refuses ReadData/WriteData asked about
another user instead of answering with the caller's grants. Neither path
is reachable today (no trusted engines, no management API), so a unit
test pins the classification; another pins that a grant store error
fails the decision (C12).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…by subject

A role with namespace read and write on one table gets RWD on that table
and read-only on its sibling; a view load vends read-only credentials even
to a namespace writer (C9). The config test now looks only for its own
token's principal instead of counting every catalog client in the shared
database.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the request metadata set directly, /credentials on an in-memory
warehouse shows the data decision: a client of the warehouse may read,
a client whose token names a warehouse bound elsewhere may not (the
authorizer's own warehouse check, C5), and an unknown principal may not
(C4). The order-preserving merge of answers is now a function with a
unit test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Narrow C8 to one warehouse and record the cross-warehouse register as a
follow-up with its precondition, correct the per-load query count (C6),
explain the rebased commit ids, add the post-review tests and mutants,
and a section on each review finding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@viethqb
viethqb merged commit 2d2c3f3 into main Oct 27, 2026
6 checks passed
@viethqb
viethqb deleted the fix/vended-credentials-follow-grants branch October 27, 2026 21:51
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.

1 participant