Skip to content

fix(catalog): authorize renames and commits by the tables they change - #758

Merged
viethqb merged 15 commits into
mainfrom
fix/catalog-rename-authorizes-source
Oct 27, 2026
Merged

viethqb merged 15 commits into
mainfrom
fix/catalog-rename-authorizes-source

Conversation

@viethqb

@viethqb viethqb commented Oct 27, 2026

Copy link
Copy Markdown
Member

Summary

egret's embedded Iceberg REST catalog authorizes each request against catalog.catalog_grants in a guard in front of Lakekeeper. Several write routes were authorized by their path alone, or as catalog-root operations, without reading the body that names the tables they change. As a result, a role with any write grant in a warehouse could:

  • rename a table it cannot read onto a name it holds a grant on, and then read it (measured: rename 204, then read 200);
  • commit to any table through transactions/commit (measured: a property set on a table the role could not write);
  • commit to a table other than the one in the path through a single-table commit, because of Lakekeeper's "Spark branch" identifier fallback (measured: 200);
  • reach a differently-cased table: grants were compared byte for byte, while Lakekeeper compares names case-insensitively, so a grant on sales.Secret reached sales.secret;
  • drop child namespaces it holds no grant on with a recursive namespace drop (measured: corp.fin.ledger deleted with a grant on corp only).

This PR makes the guard read those bodies and decide on the tables they actually change.

Spec: docs/en/specs/2026-10-27-catalog-rename-authorizes-source-design.md (D1–D14, related defects in §7, live check and gate recorded).

No migration. No API shape change.

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

  • Rename (tables/rename, views/rename). The source needs what a drop needs: a write or manage grant covering the source table. The destination needs what a create needs: a catalog grant, or a namespace grant on the destination namespace. A table grant on the destination name is not enough.
  • Multi-table commit (transactions/commit). Every table in table-changes needs what a single-table update needs. One uncovered table refuses the whole commit, which matches Lakekeeper's all-or-nothing commit.
  • Single-table commit (POST …/tables/{t}). The guard authorizes the table Lakekeeper will actually commit to: the body's identifier if present, otherwise the path's table (D11). Spark branch commits keep working. View commits load by path and are pinned by a test.
  • Name matching (D12). The guard compares names with Lakekeeper's own case-insensitive ICU collation, through Postgres, and only for pairs that are not already byte-equal. Path segments are decoded the way Lakekeeper decodes them. No Unicode normal form plus case fold reproduces that collation: NFKC plus casefold would fail open on ß/ss, and NFC misses width forms and invisible characters.
  • Recursive namespace drop (D13) needs a catalog-level write or manage grant.
  • Signer routes (D14). Every signer route is refused with 403 ("remote signing is not used by egret"). The storage profile also keeps remote signing off, and tests pin that, because this flag is the only protection Lakekeeper offers there.
  • Buffering.
    • The guard authenticates first and only then buffers the body of these routes (rename 1 MiB, commits 16 MiB).
    • It re-injects the exact bytes for the view hold, the fix(catalog): classifications follow a renamed table or column #757 table hook and Lakekeeper.
    • A declared Content-Length over the cap gets 413 without reading the body; a streamed body that crosses the cap also gets 413.
    • Every other request streams through unread.
  • Errors. Every refusal from the guard or the mount uses Iceberg's JSON error model ({"error":{"message","type","code"}}), so clients such as pyiceberg raise a typed error.
  • Messages. A refusal names the side that failed and echoes only names the caller sent, so it does not reveal whether a table exists.

Decisions

The user decided (2026-10-27) to fix F-A as its own PR, ahead of the KI-308/309 package, and to close the whole class of guard holes found on the way here. Controller defaults D1–D14 are in the spec, each with its reason.

Test plan

Full local gate on 167cf529 (the last commit, a381fc1b, is docs only), 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
just test-ranger-gates pass
cargo test --workspace --no-fail-fast pass, 0 DB skips, except: sql_starrocks_live (7 tests) needs a live StarRocks, as on earlier PRs; and one known table_hook_db timing test (the 5 s plugin wait) failed under a load average of 20–36 and passed 3/3 alone
Frontend gen:api no diff
Lint, tsc -b, build, all 10 test:bundle* pass
vitest 8496/8500 under load; the 4 failing files pass when rerun alone. This PR changes no frontend file

New tests are in crates/egret-bin/src/catalog_guard_body_tests.rs, run through mount_catalog_engine: the real guard, the view hold and the table hook, in front of Lakekeeper's real router.

  • Red first: every refusal test was seen failing on the code before its fix.
  • Mutation: the new branches were mutation-checked, 20 mutants in total, all killed.
  • Agreement test: the_guard_decides_names_the_way_lakekeeper_resolves_them. The guard allows exactly when Lakekeeper's router finds the table.
  • Pins:
    • a Trino-shaped commit body passes;
    • a Spark branch commit is authorized by the table it commits to;
    • an unauthenticated body is not read: 0 bytes are pulled from the stream.

Live check

Ran twice on an own stack, not the shared demo stack, with a Trino 483 engine provisioned by egret, curl and pyiceberg 0.12. The second run was on the final code.

  • Trino keeps working.

    • CREATE, INSERT, UPDATE, DELETE, ADD/RENAME COLUMN, table and view rename, CTAS, DROP TABLE and DROP VIEW all succeeded.
    • Trino sends only per-table commits, with no identifier in the body; it never sends transactions/commit.
    • A commit took 84–164 ms, about the same as before.
  • A restricted catalog client:

    • a rename of an unreadable table gets 403, and the table is unchanged;
    • a rename into a name it holds only a table grant on gets 403;
    • a multi-table commit touching an unwritable table gets 403, and nothing is committed;
    • a single-table commit whose body names another table gets 403;
    • with a grant on sales.Secret, secret, SECRET and se%63ret pass, and an ungranted table gets 403;
    • a recursive drop with a namespace grant gets 403, and nothing is dropped;
    • the signer gets 403;
    • malformed bodies get 400;
    • authorized renames and commits succeed.

    pyiceberg raises a clean ForbiddenError.

  • Unauthenticated oversized request: 401 in 3–7 ms. An authenticated one over the cap gets 413 in 5–12 ms, without reading the body.

  • fix(catalog): classifications follow a renamed table or column #757: a classified table and a classified column renamed through Trino still move their classification.

Follow-ups (not in this PR)

  • F4, HIGH, its own package next: a read-only grant can obtain vended S3 credentials that write and delete. Lakekeeper picks the credential scope from its authorizer, and in egret that is AllowAllAuthorizer.
  • Low-severity guard gaps:
    • creating a namespace and listing namespaces are allowed for any write grant;
    • no body-read timeout: a client that declares a body and sends nothing holds the connection (memory stays capped).
  • /config advertises its uri without the port behind the edge on a non-default port, which breaks pyiceberg there (likely X-Forwarded-Host; not confirmed).
  • Then: the KI-308/309 package (grants, rules and catalog grants follow a table rename). Its spec and plan are written.

Documentation

Platform docs (egret-data/docs) follow in a separate docs PR: what a rename, a commit and a recursive drop require, and the case-insensitive grant matching.

Checklist

  • Linked to an issue
  • Tests added or updated (or not applicable)
  • Docs updated in egret-data/docs for any user-visible feature, UI, API or config change (separate PR after merge)
  • Commit messages follow Conventional Commits
  • No secrets, tokens or real data added

🤖 Generated with Claude Code

viethqb and others added 15 commits October 27, 2026 09:24
The catalog guard authorizes POST /{prefix}/tables/rename and
/views/rename as catalog-root writes without reading the body. The spec
sets the model: the source needs what a drop needs, the destination what
a create in its namespace needs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Any role with any write or manage grant in a warehouse could rename any
table or view, including one it could not read, onto a name it held a
grant on, and then read it. The guard never read the rename body.

The mount now hands the guard the body of POST /{prefix}/tables/rename
and /views/rename and puts the same bytes back for the view hold, the
table hook and lakekeeper. The guard requires write on the source (as a
drop does) and namespace-level write on the destination (as a create
does), answers 403 naming the side that failed, and 400 for a body that
is not a rename.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
POST /{prefix}/transactions/commit names its tables only in the body, so
the guard authorized it as a catalog-root write: any role with any write
grant could commit to any table. The mount now hands the guard the
commit body (16 MiB, the table hook's own cap) and puts it back. The
guard requires, for every table-changes identifier, what a single-table
update requires, refuses the whole commit with a 403 naming the first
table not covered, and answers 400 for a body it cannot read.

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

POST /{prefix}/v1/aws/s3/sign passes the guard as a catalog-root write,
and lakekeeper's own table check runs on AllowAllAuthorizer. What stops a
role with any write grant from getting a signature for any table's files
is that egret creates every S3 warehouse with remote signing off, which
lakekeeper refuses before any table lookup. The profile is extracted into
s3_storage_profile with that reason documented, a unit test fails if
signing is turned on, and a test through the real mount pins the 403.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The table hook and view hold stubs decided by path suffix, which drifts
from the mount. They now use GuardedBody::of. A duplicate-key rename body
is pinned to rename nothing, whichever copy is unauthorized.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mount read a rename or multi-table commit body (up to 16 MiB) before
the token was verified, so an unauthenticated caller could make it
buffer. CatalogGuard gains a required authenticate step that the mount
runs first. A declared Content-Length over the cap is refused unread with
413, and a streamed body that crosses the cap is a 413 too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lakekeeper commits a single-table commit to the body's identifier when
it has one, including its Spark-branch fallback, so a namespace grant on
corp.fin could commit to the table corp.fin. The mount now hands the
guard this body too (after authN, 16 MiB), and the guard requires write
on the table the commit actually reaches. View commits stay bound to
their path in lakekeeper; that is pinned by a test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lakekeeper resolves namespace and table names under its ICU
case_insensitive collation; the guard compared bytes, so whether a grant
matched depended on how the caller spelled the name. The guard now asks
Postgres to compare the request's names with the grants' names under
that same collation, on every route, and decodes path segments as
lakekeeper does (percent-decoding, + as a space).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DELETE /namespaces/{ns}?recursive=true deletes every child namespace
and table, but the guard required write on {ns} only, so a grant on corp
could delete corp.fin. A recursive drop now needs a catalog-level write
or manage grant; the query is decoded as lakekeeper decodes it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Remote signing is off on egret's warehouses and unused by its clients,
and lakekeeper's table check behind the signer allows everyone. The
guard now refuses /aws/s3/sign, /{prefix}/v1/aws/s3/sign and the
table-specific signer routes outright, whatever the grants. The profile
pins stay; the lakekeeper refusal is now pinned against its own router.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lakekeeper vends read-write-delete storage credentials whenever its
authorizer allows WriteData, and egret runs it on AllowAllAuthorizer, so
a read-only catalog grant receives credentials that can write and delete
the table's files. Recorded with the evidence for a separate package.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The guard and the mount answered 401/403/400/413 as plain text, which
Iceberg clients report as an unexpected payload. Every refusal is now
{"error": {"message", "type", "code"}}, as lakekeeper answers, with the
same messages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Trino writes only through single-table commits, whose bodies the guard
now reads. A commit in Iceberg Java's UpdateTableRequest shape, with an
identifier equal to the path, commits for single- and multi-level
namespaces, a name with a space, and a grant in another case. Also
records O1 (the /config uri behind a non-default edge port).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Trino 483's commit bodies carry no identifier; one captured in the live
check (an ADD COLUMN, uuid replaced) commits with a write grant and is
refused with a read grant. The spec records both live checks and O3, the
missing body-read timeout on guarded routes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@viethqb
viethqb merged commit 1e0a3cf into main Oct 27, 2026
6 checks passed
@viethqb
viethqb deleted the fix/catalog-rename-authorizes-source branch October 27, 2026 09:50
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