Implement roadmap_aug10: authorization, storage parity, Iceberg conformance and the guardrails that catch them - #1
Merged
Conversation
Addresses the critical clusters of roadmap_aug10.md: the API-layer authorization bypasses (B0a-B0m), the Iceberg metadata conformance and commit-path defects (B11-B16o), and the middleware/reliability items. Authorization (pangolin_api) B0a POST /api/v1/tokens took no session at all: any authenticated principal could mint a Root JWT for any tenant. Now Root-only, or TenantAdmin within its own tenant and never above its rank. B0b Credential vending performed no authorization and hardcoded read+write for a table it never looked up. Now resolves the asset, requires Read, and vends write only when Write is held. B0c rename_table had no permission check. Now Write on the source and Create on the destination namespace, plus a 409 on collision. B0d update_namespace_properties discarded its session and never resolved the catalog. Now Write-scoped to the namespace. B0e create_view/get_view had no checks; a view's SQL is its whole definition. Now mirror create_table/load_table. B0f perform_maintenance ran destructive jobs against a hardcoded "default" catalog with no authz. Now uses the path catalog and requires Delete. B0g The Iceberg OAuth token endpoint checked `active` but not expiry, so an expired service user could renew indefinitely. B0h PANGOLIN_DEV_MODE waived the NO_AUTH public-bind guard - the two flags are routinely set together. The guard is unconditional. B0i PermissionScope::Tenant matched without comparing tenants, so a grant in tenant A satisfied resources in tenant B. B0j Logout revoked session.user_id, which no token carries as its jti, so tokens survived logout. UserSession now carries the jti; rotate_token revokes the rotated-out token too. B0k /api/v1/oauth/exchange was not public, making the OAuth login flow unreachable: the browser could never redeem its code. B0l OAuth linked accounts by unverified email. Identity is now (provider, subject); email linking needs a verified address and an operator domain allowlist. B0m expires_in_hours could panic token issuance before the checked arithmetic ran. Clamped, plus a CatchPanicLayer. B0o A malformed or absent jti skipped revocation entirely; the API-key branch also returned before the public-path check. Iceberg (pangolin_core, pangolin_api) B11-B14 default-spec-id (was current-partition-spec-id), last-partition-id, schema "type": "struct", and no more explicit nulls on optional fields. B13 metadata-log is appended on every commit and truncated to write.metadata.previous-versions-max. B15 A client sequence number can no longer jump the counter to i64::MAX and overflow the next commit. B16 A feature-branch commit no longer moves main. B16a One shared parse_namespace across all handlers: a nested namespace registered on create is now found on commit. B16b last-updated-ms advances on every commit, not just snapshots. B16c -1 resolves against what this commit added, not vec.last(); duplicate Add* ids are rejected. B16d/g Metadata files are written before registration and reclaimed on a lost CAS instead of orphaned. B16e create_table returned the table directory as metadata-location. B16f The hand-rolled schema parser dropped complex columns, forced every field optional and widened int to long. Deserialized now. B16h Namespace property removals were silently ignored. B16i pageToken/pageSize and next-page-token on list responses. B16j Iceberg handlers return the spec error envelope. B16k Federated forwarding on create/delete namespace and the tree. B16l The timeout layer sits outside the concurrency limiter, so queued requests have a deadline. B16m delete_warehouse deletes before invalidating, closing the window where a racing read re-cached deleted credentials. B16n PANGOLIN_SHUTDOWN_GRACE_SECS actually bounds the drain. Storage B17 One ns_key helper on the memory backend; multi-level namespaces were undeletable there. New CatalogStore::delete_file and replace_namespace_properties, implemented across all four backends. Also completes loadNamespaceMetadata and namespaceExists. cargo test --workspace: 57 targets green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses the storage-layer cluster of roadmap_aug10.md (B1-B7, B17-B30) plus improvement #1, the parity harness that makes the whole class of finding testable rather than reviewable. Tenant isolation and data integrity B1 Mongo's get_audit_event discarded the tenant_id parameter, so any tenant holding an audit UUID could read another tenant's record. B2 Mongo revocation was a silent no-op: the write went through serde as a string under `id`, the check queried a BSON UUID under `token_id`. Nothing could ever match, so revoked JWTs - including after logout - stayed valid. Cleanup had the mirror type mismatch and deleted nothing, so the collection grew without bound. B3 SQLite delete_branch referenced a column that does not exist and was non-transactional, so the branch was committed away and its assets orphaned while the caller saw an error. B4 Postgres decoded a TEXT[] as String; `Row::get` panics on decode failure, so any search with a hit panicked the request. SQLite's sibling silently returned raw JSON as the namespace. B5 Mongo dropped the compare-and-swap entirely, so two concurrent Iceberg commits both "succeeded" and one snapshot was lost. B6 The memory by-id asset index was keyed on catalog name alone, so tenant A deleting `sales` broke get_asset_by_id for tenant B's. B7 All three persistent backends stored the Debug spelling of AssetType and parsed only two variants, defaulting the other 15 to IcebergTable - a DeltaTable round-tripped as an Iceberg table. B26 Memory skipped the CAS whenever the expectation was None, which is the create-path assertion, not "no check". B30 Memory delete_tenant had no cascade: warehouses (with credentials), catalogs, assets and tokens all outlived the tenant. Parity and determinism B17 One ns_key helper on memory; create/get keyed on "." while delete/update keyed on 0x1F, so nested namespaces were undeletable. B18 SQLite's foreign_keys pragma is per connection but was set on one arbitrary pooled connection, making ON DELETE CASCADE fire nondeterministically. Now set through the connect options. B19 SQLite get_metadata_location ignored the branch and returned an arbitrary row, so a dev read could return main's pointer. B20 SQLite left the metadata_location column stale, freezing Asset.location at creation time after every commit. B21 SQLite delete_catalog cascaded before checking existence, so deleting a nonexistent catalog destroyed matching children first. B22 SQLite persisted Debug action names and parsed snake_case, then swallowed the mismatch, misattributing nearly every audited action. B23 Mongo audit filters compared Debug names to snake_case documents and always matched zero rows; listings were unsorted and unbounded. B24 Postgres list_catalogs omitted catalog_type/federated_config from the SELECT and hardcoded Local, so federated catalogs looked local. B25 Memory merge_branch reused asset ids (repointing the by-id index at the copy) and never advanced the target head. B27 ORDER BY on every paginated query; memory sorts before slicing. B28 One definition of search: LIKE metacharacters escaped, and one tag semantic (ALL-match, empty list means no filter) across all four. B29 Memory audit returned oldest-first and unbounded without a filter. Two defects the new parity suite found on its first run * SqliteStore had no inherent revoke_token/is_token_revoked, so the trait delegations in sqlite/main.rs called themselves. Revoking a token on SQLite recursed until the stack was exhausted and aborted the process. * The SQLite audit_logs table still declared the original (actor, resource, details) shape while the code inserted the full AuditLogEntry, so every audit write failed with "no such column" and the backend kept no audit trail at all. Schema version bumped to 2. New CatalogStore methods delete_file and replace_namespace_properties are implemented across all four backends. cargo test --workspace: 57 targets green. Clippy budget lowered 36 -> 34. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses B8-B10 and B43-B46 of roadmap_aug10.md, plus improvement #3. Deployment B8 `docker compose up` could not start the API: the quick-start compose file set no PANGOLIN_JWT_SECRET, and since 0.6.0 the server refuses to start without one. The result was a crash-looping container with no explanation. The `:?` form now fails `docker compose up` itself with a message telling you what to set. B9 Both compose files set PANGOLIN_STORE_TYPE. The server reads PANGOLIN_STORAGE_TYPE, so the variable did nothing - and anyone editing it to `postgres` silently stayed on the in-memory backend and lost their data on every restart. B10 docker-compose.release.yml pinned alexmerced/pangolin-api:0.2.0, four releases behind, and ran scripts/test_release_v0.2.0.py, which does not exist. The "release verification" file verified nothing. Now parameterised by PANGOLIN_VERSION and pointed at a real script. Docs B43 docs/environment-variables.md documented PANGOLIN_HOST, PANGOLIN_PORT and PANGOLIN_STORE_TYPE - none of which any code reads - and omitted 34 that it does. Rewritten from the source, and the second, separately drifted copy under getting-started/ is now a redirect. The fix is not the rewrite; it is scripts/check_env_var_docs.sh, which re-derives the set from the code and fails CI on either kind of drift. A `<!-- not-a-variable -->` marker lets the page still warn readers off names that do not exist. Hygiene B44 A live PyPI API token sat in plaintext in the repo-root .env. It was never tracked by git, but .env is read by docker compose - so a publish credential was being handed to containers with no use for it, and it was one .gitignore edit away from leaking. Removed, with a pointer to the keyring / CI-secret flow PUBLISHING.md already describes. ** The token must be rotated at pypi.org: it existed on disk in plaintext and has to be treated as exposed. ** B45 git rm --cached on ~260 KB of tracked debug output, deleted the two .bak store monoliths (~4k lines of divergent dead query copies that any grep of the store layer would hit), and added the ignore patterns that were missing when those files were first committed. B46 Pruned 18 unused runtime dependencies (all @smui/*, marked, smui-theme) from the UI - zero imports in src/, pure image bloat. Added the missing @vitest/coverage-v8 so `npm run test:coverage` runs, added a test:e2e script for the Playwright specs that had none, and pointed Playwright at 5173 (where vite dev actually serves) with webServer enabled, so the specs connect to something. CI (improvement #3) A `config drift` job validates every compose file, asserts the quick start still fails loudly without a signing secret, rejects any PANGOLIN_* name in a compose file that no code reads, and runs the env-var doc check. All three of B8/B9/B10 are the same failure - config nothing validates - so the job targets the class rather than the instances. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses B31-B37 of roadmap_aug10.md, plus improvement #10. Most of these are
the same defect: the UI and the server had drifted apart with nothing checking.
Base URL and transport
B31 Three different names for the API base URL coexisted - the client read
PUBLIC_API_URL, .env.example declared VITE_API_URL, the compose files
passed VITE_API_URL, and the login page read a fourth spelling off
import.meta.env. None agreed, so the client's fallback always won and
every deployed build called `http://localhost:8080` - the *visitor's*
machine. Standardized on PUBLIC_API_URL (SvelteKit's dynamic public env
requires the prefix), defaulting to same-origin in production.
B32 ~13 raw `fetch('/api/v1/...')` calls only resolved under the dev proxy.
With adapter-node they hit the SvelteKit server, which has no /api/v1
routes, and 404'd in production - and they skipped X-Pangolin-Tenant, so
a root user acted on the wrong tenant. All routed through apiClient.
That includes the NO_AUTH probe in the auth store, which always
concluded "auth enabled" because its relative URL 404'd.
B34 Nothing in src/ handled a 401, so an expired JWT left the user in a
permanently broken "authenticated" session. The client now reports 401
to a handler the root layout registers, which ends the session and
redirects. Logout also revokes the token server-side - it previously
only cleared localStorage, leaving a working credential behind.
Endpoints the UI called that did not exist (B33)
Added `DELETE /api/v1/branches/{name}?catalog=...`: the UI's branch-delete
control had always 404'd because only GET was registered.
Added `GET /api/v1/oauth/providers` (public): the login page hardcoded four
provider buttons because this 404'd, so buttons for unconfigured providers
went nowhere. Now only configured providers render.
Pointed the permissions client at `GET /api/v1/permissions?user=<uuid>`,
which exists, instead of `/users/{id}/permissions`, which never did - the
permissions page and EditPermissionsDialog showed an empty list forever.
Removed `initiateOAuth`, which called another route that does not exist.
Other
B35 Tag-filtered search was broken end to end: the UI sent repeated `tags`
params, and `serde_urlencoded` cannot deserialize repeated keys into a
Vec, so it 400'd. Now one comma-separated value on both sides.
B36 DataTable's only actions-slot outlet sat inside an
`{#if searchable && !serverSide}` guard; the catalogs page passes
neither, so its "New Catalog" control never rendered and there was no
way to create a catalog from that page. The page also refetched the
same unpaginated list and never set hasNextPage. Both fixed, plus the
HTML comments in attribute position that Svelte parsed as boolean props.
B37 The root layout used `tenantStore` with no import (a latent
ReferenceError), the handler was wired to no element, and the tenant
loader was commented out - so root users could not switch tenants and
X-Pangolin-Tenant was never set. Imported, re-enabled, and the selector
is back in the header.
B46 test plumbing, and what it was hiding
vitest.config.ts overrode `import.meta.env` wholesale, which broke
SvelteKit's virtual env module - so every suite importing the API client
failed to *load*. That is why client.test.ts's mocks could lack `.text()`
without anyone noticing. With the override gone and the mocks completed,
UI suites passing went 7 -> 11 and tests passing 37 -> 86.
40 tests still fail. They are pre-existing and outside this roadmap's scope:
Svelte 5 dropped `$on` on component instances, and a number of assertions
were written against an older UI. They were previously invisible because the
suites never loaded; making them visible is the point.
Improvement #10: deleted `src/lib/stores.ts`, `app.scss`, and the
unauthenticated `routes/api/docs/[...path]` file-read route (no callers, a
blacklist-based traversal guard), and stripped 27 console.log calls - one of
which logged a bearer-token prefix to the browser console.
cargo test --workspace: 57 targets green. `npm run build` succeeds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Addresses B38-B41, B_cli1-B_cli8 and B_sdk1-B_sdk5 of roadmap_aug10.md, plus
the `deny_unknown_fields` leg of improvement #0.
The systemic fix first
Every finding in this cluster has the same shape: a client sends a field the
server does not have, serde ignores it, and the request succeeds having
quietly done something other than what was asked. `deny_unknown_fields` on
all 34 server request structs turns that into a 422 that names the field.
It immediately caught five payloads in the project's own tests. The worst:
full_system_test.rs created a catalog with kebab-case `warehouse-name` and
`storage-location`, both dropped - so it asserted 201 for a catalog with no
warehouse and no storage location, and the warehouse existence check never
ran. That test now creates the warehouse it always claimed to reference.
Python SDK
B38 __version__ was hardcoded "0.1.0" against a 0.6.0 package; now read
from the installed distribution.
B39 No timeouts anywhere - a hung server blocked the caller forever. Every
request is now bounded, and the client reuses a Session.
B40 requires-python ">=3.8" was unsatisfiable with pydantic>=2 and
pyiceberg; bumped to >=3.9 with matching classifiers.
B41 Importing anything from pypangolin pulled in the whole Iceberg stack,
so `pytest tests/` could not even collect. pyiceberg is an extra and
catalog.py imports it lazily; tests now collect and pass (5/5).
B_sdk1 PermissionScope's kebab-case aliases never matched - the server emits
kebab-case *variants* with snake_case *fields*, which I verified against
its actual output rather than inferring. Every scope deserialized empty.
Role.permissions was typed List[Permission] where the server returns
PermissionGrant, so any role with grants raised ValidationError.
B_sdk2 delete(asset_id, key) deleted *all* metadata; request_access sent
`motivation` where the server reads `reason`; federated create dropped
uri/warehouse/credential; token generate sent three ignored fields so
every token was silently 24h; rebase omitted the required `name`.
B_sdk3 Four CLI commands raised TypeError or AttributeError on every call,
masked by blanket handlers that exited 0.
B_sdk4 profiles.yaml (holding the JWT) was written at the umask - now 0600
under a 0700 directory, with the mode set before the secret is written.
get-token no longer echoes the token into the terminal, and connection
assets no longer store the encryption key beside its own ciphertext.
B_sdk5 Added the console-script entry point the docs promised, py.typed,
pytest pythonpath, `raise ... from e`, and the ConfigDict migration.
Rust CLIs
B_cli1 create-catalog sent `warehouse`/`type`; the server takes
`warehouse_name`/`catalog_type`. Both were dropped, so the required
--warehouse flag was discarded, the warehouse existence check was
skipped, and the CLI printed success.
B_cli2 All six merge commands targeted /api/v1/merges/..., a prefix that has
never existed.
B_cli3 Four federated commands used /api/v1/catalogs/{name}/... instead of
/api/v1/federated-catalogs/...; create-federated-catalog made a *Local*
catalog; list-federated-catalogs filtered on a key that is not in the
response, so it always printed empty.
B_cli4 Both revocation commands 404'd (/api/v1/tokens/revoke*, real routes
are under /api/v1/auth/).
B_cli5 delete-user passed a username to a Path<Uuid> handler (400 every
time; it now resolves the id first); revoke-permission used a filter
DELETE that does not exist; request-access posted to a GET-only route;
three `.unwrap()`s on server JSON panicked the CLI.
B_cli6 update-user --username was silently dropped (the server cannot rename
a user); unhandled commands printed a note and exited 0.
B_cli7 ConfigManager::new().unwrap() panicked with no $HOME; the config file
holding the token was written 0644; reqwest had no timeout.
B_cli8 merge-branch sent source/target instead of source_branch/target_branch;
request-access was a no-op returning success; search was a placeholder
telling users a working feature did not exist; get-token sent a null
tenant; generate-code printed the live JWT into copy-paste output.
Also closed a metadata-delete authorization gap found on the way: the handler's
entire authorization was the comment `// Check permission logic`, so any tenant
member could delete any asset's description, tags and discoverable flag.
Every CLI command now exits non-zero on failure (21 sites). That is what made
this cluster survivable: the errors were printed and the exit code was 0, so no
script and no CI job could tell a working command from a broken one.
cargo test --workspace: 57 targets green. pytest: 5 passed. Clippy at budget.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Completes roadmap improvements #0, #1 and #2, and fixes B42 plus one more handler with no authorization check. The permission matrix (improvement #0) Every finding in the B0a-B0m cluster was invisible to CI. The code compiled, was formatted, was lint-clean, and the tests passed - because nothing asserted *who is allowed to call what*. A handler that forgets `check_permission` looks identical, to every check that existed, to one that calls it. `POST /api/v1/tokens` minting a Root JWT for any authenticated caller survived a full security release that way. `authz_matrix_tests` drives each sensitive route as Root, the owning tenant's admin, an ungranted tenant user, and a foreign tenant's admin, asserting the expected 200/403 for each. It covers token minting and rank limits (B0a), the token-lifetime panic (B0m), view endpoints (B0e), maintenance (B0f), credential vending (B0b), cross-tenant catalog access (B0i), logout actually revoking (B0j), the OAuth exchange endpoint staying public (B0k), unknown request fields being refused, and anonymous callers being turned away. Verified by reintroducing B0a behind a one-line edit: the matrix fails with `200 where 403 expected`. A test that cannot fail is not a guardrail. Deliberately end-to-end through the real router - a unit test of `check_permission` proves the function works, not that the handler calls it, and "the handler does not call it" is the whole bug class. CI (improvements #1, #2, #3) * `guardrails` runs the permission matrix and the cross-backend parity suite as their own job, so a failure reads as "authorization or backend parity regressed" rather than as an anonymous test failure. * `sdk` runs pytest and ruff, asserts the package imports *without* the Iceberg extra (B41), and asserts `__version__` matches package metadata (B38). * `ui` builds, and greps for the two patterns a type checker cannot express: a raw `fetch('/api/v1/...')` reappearing (B32) and any resurrection of `VITE_API_URL` (B31). * `config-drift` (previous commit) validates the compose files and the environment-variable reference. Two more authorization gaps, same class as B0c-B0f * `rebase_branch`: the entire authorization was `// TODO: Granular permissions? For now assume Write on Catalog`. Any tenant member could rebase any branch, which rewrites its contents. * `delete_business_metadata`: the authorization was the comment `// Check permission logic`. Any tenant member could delete any asset's description, tags and `discoverable` flag - the last of which governs who can see the asset at all. B42: `list_catalogs` paginated in the store and filtered by permission afterwards, so a TenantUser got variable-size pages *including empty pages while more authorized data existed* - and a client that stops on an empty page, which is the normal idiom, silently missed data. Filtering now happens before the page is cut. Also brought ruff to zero on the SDK (unused imports, discarded bindings), with E701 turned off explicitly and with a reason rather than by omission. cargo test --workspace: 57 targets green. pytest: 5 passed. ruff: clean. `npm run build`: succeeds. Clippy at its 34 budget, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every finding in roadmap_aug10.md, grouped by what an operator needs to act on rather than by the audit's own numbering - with the five findings exploitable by any authenticated principal called out first, and the PyPI token flagged for rotation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nly a live database could reveal Four items requested after the roadmap work: a real SQLite migration, running the suites against live Postgres and MongoDB, a security advisory for 0.6.0, and the version bump with migration notes. 1. The SQLite audit_logs fix did not reach existing databases The previous commit corrected `sqlite_schema.sql` and bumped SQLITE_SCHEMA_VERSION. That is not a migration: the schema file is written entirely with `CREATE TABLE IF NOT EXISTS`, which does nothing when the table already exists. Fresh installs got the corrected columns; every upgraded database kept the broken ones, now with a version number claiming otherwise - worse than no version at all. There is now a real v1 -> v2 step, keyed off `PRAGMA table_info` rather than the recorded version (some databases predate the version table, and a version only says what a previous run claimed). The old table is preserved as `audit_logs_pre_v2` rather than dropped or force-fitted: its shape has no `resource_type`, which is NOT NULL and parses as an enum, and a fabricated entry in an audit log is worse than an absent one. `tests/sqlite_migration_tests.rs` builds a genuine pre-v2 database and asserts the upgrade works. Verified to fail without the migration - three of its four cases, exactly the upgrade paths. 2. Running against live Postgres and MongoDB The parity suite was written against memory and SQLite, the two backends CI could run without a service container. Pointed at a live Postgres and MongoDB for the first time, it failed on both. None are regressions. * **Postgres: asset search was broken outright.** No migration ever created `business_metadata`, while `search_assets` joins it - every search failed with `relation "business_metadata" does not exist`, a hard SQL error rather than an empty result. The three CRUD methods were unimplemented, so the trait's "not supported" default answered them. Added both. * **Mongo: role assignments were unreadable.** `bson::to_document` writes a `Uuid` as a string while the deserializer expects Binary, so `assign_role` wrote documents `get_user_roles` could never match and that could not be deserialized at all. Every role-derived permission silently vanished: a user holding an admin role was authorized as though they held none. The same asymmetry caused B1 and B2 in two other collections; one helper now covers all of them. * **Mongo: `get_metadata_location` had no fallback** to the asset's own `location`, unlike the other three backends, so the read path and the commit CAS disagreed about what "current" meant. * **Mongo: the "no transaction support" fallback was unreachable.** `start_transaction` is a local call in the Rust driver and cannot fail for want of a replica set; the error arrives on the first operation inside the transaction and was propagated rather than caught. `delete_catalog` failed outright on standalone mongod instead of degrading as its comment promised. Also made `test_postgres_access_requests` re-runnable - it used a fixed username and only passed against a freshly created database. CI now runs the parity suite against Postgres, MongoDB and MinIO service containers, and **fails if any backend was skipped**. A skipped backend passing silently is precisely how two of them went untested through a security release. 3. Security advisory SECURITY.md now carries the 0.7.0 advisory: ten issues exploitable by any authenticated principal, the two availability defects, the audit-trail gaps, and the data-loss class - with the rotation steps each one implies. 0.6.x is marked unsupported. 4. Version and migration notes `scripts/bump_version.sh` sets one version across all five artifacts *and* the inter-crate path requirements (missing those breaks the build outright, as this commit found out), with a `--check` mode wired into CI. Everything is at 0.7.0. `docs/upgrading/0.6-to-0.7.md` covers the breaking changes: unknown request fields now rejected, branch commits no longer moving main, tokens without a jti, OAuth email linking, the renamed compose and UI variables, and the four changed SDK signatures - each with what it was silently doing before. Verified with live Postgres, MongoDB and MinIO: 58 test targets, exit 0, zero failures. Clippy at its 34 budget, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
✅ Deploy Preview for lustrous-pithivier-968b22 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…osed Everything verified on MongoDB so far ran against a *standalone*, which supports neither transactions nor retryable writes - so only the degraded branches ran. That is the same gap that produced the last round of findings, one layer in: the fallback in `delete_catalog` was broken for as long as the code existed precisely because nothing had ever run the branch it falls back *from*. A single-node replica set is enough to tell the two apart - a set of one has transactions and retryable writes - so there is now `docker-compose.mongo-rs.yml` for local use (on port 27018, so it can run alongside the standalone) and a `mongo-replica-set` CI job. The job asserts a primary was elected *and* probes that a transaction actually commits, because a job that silently re-tests the standalone paths is worse than no job. Running it found another defect, in the fallback this time: `delete_catalog_unsafe` deleted every matching tag, branch, asset and namespace and only then discovered the catalog did not exist - returning "not found" to a caller with every reason to believe nothing had happened. That is B21, fixed for SQLite during the roadmap work; the same shape survived here because this branch only runs without transactions and had never executed. The parity test for B21 also needed sharpening. It named a catalog with no children at all, which passes whatever the ordering is - it constrained nothing. It now creates an orphaned child (a namespace under a catalog name with no catalog row) and asserts it survives a failed delete. Verified to fail against the unfixed Mongo path. That change surfaced a genuine backend asymmetry worth recording: Postgres is the only backend where the orphaned state is *unreachable*, because it carries foreign keys from namespaces to catalogs and from assets to namespaces. The test asserts the ordering where orphans are possible and says so explicitly where they are not, rather than skipping quietly - "Postgres refused to create the fixture" and "Postgres passed the assertion" are very different facts. All four backends green. Clippy at budget, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The string/Binary mismatch had been fixed four times, in four collections,
each time as its own bug: audit events, token revocation, role assignments,
permission records. Fixing them one at a time was treating symptoms. This
audits all 21 collections at once, with a round-trip test per entity rather
than per feature, and finds four more.
There are three ways this codebase converts a Uuid to BSON and they all
differ. `to_bson_uuid` gives Binary with the generic subtype; `doc! { "k": id }`
gives Binary with the UUID subtype; `bson::to_document` gives a string. Reads
disagree too: a typed Collection<T> demands binary, `bson::from_bson` demands a
string. A write and a read chosen independently agree only by luck, and the
failure is silent — the filter matches nothing and the caller gets an empty
result indistinguishable from an empty collection.
What that was costing:
* Every service-user method was a no-op. Writes let Mongo generate an ObjectId
while the by-id methods filtered on `_id`; the listing and the API-key lookup
used snake_case names for a kebab-case struct. API-key authentication could
never resolve a service user on MongoDB. It fails closed, so this was an
outage rather than a bypass.
* Business metadata could be written but never read: only asset-id was
rewritten as Binary, so the first of the other three UUID fields aborted the
read. Writing metadata made it permanently unreadable.
* Listing active tokens failed outright — timestamps written as BSON DateTime,
read by a chrono deserializer that only accepts RFC3339. The created_at arm
swallowed the same error and substituted now().
* A branch with a head commit could not be read. Only branches that had never
been committed to worked, which is why every existing test passed.
`from_bson_uuid` now accepts all three encodings so records already written by
any of them still load; writes go through `to_bson_uuid` alone. The new suite
covers every collection and runs against both MongoDB topologies in CI.
Two test-environment defects surfaced while verifying this. The db-test compose
file had no object store at all, so the compliance tests fell through to the EC2
metadata endpoint and failed with a credentials error naming nothing relevant —
CI had MinIO and the documented local workflow did not. And the image CI pulled,
bitnami/minio:latest, has been withdrawn from Docker Hub: that job could not have
been passing. Both now use minio/minio with explicit bucket creation.
Verified against live MongoDB in both topologies and live PostgreSQL: full
pangolin_store suite green, clippy at budget.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>The UI job in CI built the app and never ran its tests. The suite had drifted to 40 failures across 14 files, and most of them could not have passed: the global test setup replaces the API and store modules with stubs, so the unit tests *for those modules* were asserting against the stub rather than the code. Running them turned up real defects underneath. * A root user could not create another root user - the role option's value was `Root` and the server's `UserRole` is kebab-case. The same PascalCase leftovers showed a tenant admin an Edit control for root users, and left the role badge colours keyed on values the API never returns. * A warehouse created in the UI showed no bucket in the UI: the list page read `s3.bucket`, the create form writes `bucket`. Both are now accepted on read, as the server already does. That table also rendered Type twice, in place of the Region column its own template had a branch for. * `production` is not a branch type. The API knows `ingest` and `experimental`; the UI typed it as `'experimental' | 'production'` and keyed the green badge on `production`, so every branch rendered identically and `ingest` - the type carrying a distinct permission - had no representation at all. * A branch with no recorded parent was displayed as branching from `main`. * A local catalog could be created with neither a warehouse nor a storage location, leaving it nowhere to write its tables. * The role select on the user edit page had no accessible name, and its fallback value matched none of its options. * `logout()` could throw and skip the caller's redirect, stranding the user on a page they were no longer authenticated for. `DataTable` moves from `createEventDispatcher` to callback props, with its six consumers. That is the Svelte 5 idiom and it is what makes the component testable: `component.$on(...)` was removed in Svelte 5, so the row-click test had been sitting there as a stub asserting nothing. CI now runs `npm test` and carries a svelte-check budget on the same ratchet as the clippy one. svelte-check errors fell 166 -> 150, mostly by giving `StorageConfig` the index signature the server's free-form HashMap always implied. The app runs in Svelte 5 legacy mode - none of its 90 components use runes. That is supported and works; converting them is separate work and is not done here. 129 tests pass, build clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…comments
`gh pr checks` on the last push: five jobs red. Everything in the previous two
commits was verified locally, which turns out not to be the same thing.
The important one: the workflow set `RUSTFLAGS: -D warnings` at the top level.
`clippy` and `test` override it with `RUSTFLAGS: ""`; the two jobs added for the
audit - `guardrails` and `mongo-replica-set` - did not, so they failed at
*compile* on the tree's pre-existing dead-code warnings. The permission matrix,
the cross-backend parity suite and every MongoDB suite they were added to run
had never once executed. A guard that cannot run is worse than no guard,
because it reads as protection. The top-level setting is removed rather than
overridden in two more places: it gated nothing, since every job that compiles
Rust opted out of it, and `clippy-ratchet` is the actual gate on warnings.
The rest:
* `config drift` failed on its own documentation. Its `grep -v 'not read'` was
meant to skip explanatory comments and matched neither of the two that exist
("was renamed", "which nothing reads"). Both guards now strip comment lines -
the same fix already applied to the UI guards in this file.
* That guard was also not recursing, so it had never seen the eight
`deployment_assets/**/docker-compose.yml` files still setting
`PANGOLIN_STORE_TYPE`, which the server does not read. B9 fixed the two
compose files at the repo root and never reached these. They work anyway -
each sets `DATABASE_URL` and the server infers the backend from its scheme -
so this was dead, misleading config rather than an outage. Renamed to
`PANGOLIN_STORAGE_TYPE`; all four values in use are accepted by `build_store`.
* `python sdk` failed on 316 ruff findings. `pyproject.toml` says the ruleset is
"pinned here rather than left to whatever version the runner installs" - but
the job ran `pip install ruff`, which took 0.16 and its wider default rules.
Pinned to the version the package was actually clean against.
`PANGOLIN_API_URL` is exempted from the drift guard with the reason recorded:
it is set on the `tests` service in docker-compose.release.yml, and nothing
reads it because the script that service runs, scripts/integration_test.py,
starts its own server with `cargo run` - which cannot work in a
python:3.11-slim container with only ./scripts mounted. B10 replaced a missing
script with one that cannot run there. That path is still broken and is not
fixed here.
Both guards and the compose files verified locally; ruff and pytest clean at
the pinned version.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>`cargo audit` reports 26 vulnerabilities against the current lockfile, which is the one CI job still red. Several are not cosmetic for this service: two high-severity certificate-validation bypasses in `aws-lc-sys`, name-constraint and CRL-parsing defects in `rustls-webpki`. Pangolin vends cloud credentials and talks to S3, Azure and GCS over TLS, so certificate validation is on its critical path. This is the subset that needs no policy decision: targeted `cargo update -p` for each advisory with a compatible fix, leaving `rust-version = "1.92"` alone. 26 -> 17. Cleared: bytes 1.11.0 -> 1.12.1, crossbeam-epoch 0.9.18 -> 0.9.20, rustls-webpki 0.103.8 -> 0.103.13, hickory-proto, quinn-proto, time. A blanket `cargo update` reaches 8 rather than 17, but pulls AWS SDK crates requiring rustc 1.94.1 against a declared MSRV of 1.92. Raising the toolchain floor breaks every consumer still on 1.92, which is a decision about this project's users rather than a side effect of a dependency bump, so it is not taken here. What remains needs one of two deliberate choices, still open: * an MSRV bump to 1.94, which unblocks the `aws-lc-sys` cluster and the AWS SDK chain; * an `.cargo/audit.toml` ignore list with per-advisory justification. Some entries need this route regardless - `rsa`'s Marvin timing attack (RUSTSEC-2023-0071) has no fixed version at all, and `quick-xml` is held back by `azure_core` 0.20 and `object_store` 0.11. Verified: 55 test targets, all ok, zero failures, against live PostgreSQL, MongoDB and MinIO. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… never built Takes both halves of the dependency plan - bump what a bump fixes, justify in writing what nothing can - and turns up something larger on the way. ## The cloud-credential features have never compiled `cargo check -p pangolin_api --features cloud-credentials` fails. So do `aws-sts`, `azure-oauth` and `gcp-oauth` individually. This reproduces at the original `aws-sdk-sts = "=1.50.0"` pin, so it is not fallout from this change: they have never built, at any version. `cargo build` and `cargo test` run with default features and no job ever passed `--features`, so nothing noticed. For a catalog whose job includes vending scoped, time-limited cloud credentials, that is the feature set. The errors are the kind that only surface when a `cfg` block is never type-checked: five parameters bound as `_duration` / `_resource_path` / `_permissions` to silence unused warnings in the default build and then referenced without the underscore inside the feature block, across three signers; `anyhow!` used in gcp_signer.rs with only `anyhow::Result` imported; and `creds.expiration()` handed to `chrono::DateTime::parse_from_rfc3339`, though it returns an `aws_smithy_types::DateTime` and never was text. The operational consequence, verified in the code rather than assumed: with the feature off the `#[cfg]` block is absent, so `generate_credentials` falls through to the static-credential branch and vends the long-lived warehouse key with `expires_at: None`. Any deployment that set `use_sts` and a `role_arn` believing it received a scoped session token did not. Recorded in SECURITY.md. The `=1.50.0` pin carried no comment, protected nothing, and blocked `aws-config` from the release that drops the second TLS stack. Relaxed to 1.109. ## Advisories: 26 -> 0 Upgrades cleared the ones that matter here: the `aws-lc-sys` cluster (two high-severity certificate-validation bypasses, a PKCS7 signature-validation bypass), `rustls-webpki` name-constraint and CRL-parsing defects, plus quinn-proto, hickory-proto, bytes, crossbeam-epoch and time. Those sit on the path this service uses to reach S3, Azure Blob Storage and GCS. Reaching them needed the MSRV at 1.94, raised from 1.92 deliberately. The remaining eight are exceptions in .cargo/audit.toml, each with the reason it cannot be upgraded and what the exposure actually is. They are specific advisory IDs, so a new advisory - including a new one against these same crates - still fails the job. `rsa` is the clearest case: never compiled at all, since `sqlx-mysql` is an optional dependency this workspace does not enable, and `cargo audit` scans Cargo.lock rather than the build graph. ## Two new guards `features` builds each optional feature, and the store's azure/gcp backends - the check whose absence let the above rot. `msrv` reads `rust-version` out of the manifest and builds with exactly that toolchain. Every other job runs `stable`, so the declared floor had never been tested; it was a promise to consumers that nothing kept. Dockerfile, README, CONTRIBUTING and the deployment guide are all moved to 1.94 with it. Clippy rose 34 -> 37 on the newer toolchain; cleared rather than ratcheted up, now 31, budget lowered to match. Verified: 59 test targets, 370 tests, zero failures, against live PostgreSQL, MongoDB and MinIO. All four optional features compile. fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The advisory exceptions landed in `pangolin/.cargo/audit.toml` and the job kept failing on the very advisories they cover. `rustsec/audit-check` invokes `cargo audit --file pangolin/Cargo.lock` from the repository root, and `cargo audit` reads `.cargo/audit.toml` relative to the current directory only - it does not walk up the tree. Reproduced locally: `cargo audit` from `pangolin/` reports zero, the identical lockfile audited from the root reports eight. Duplicating the config at the repository root would have turned CI green while leaving a developer running `cargo audit` in `pangolin/` - where the workspace actually is - looking at a different answer from the one CI computes. That is the same shape as the defects this branch exists to fix, so the job now runs the same command in the same directory a developer would, and there is one copy of the file. Everything else on the previous push passed, including the two jobs added with it: `minimum supported rust version`, and `optional features build` across all four features. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Everything needed before publishing 0.7.0. Each item here is a reason the previous four releases did not actually ship what they claimed. **The tag workflow has never succeeded.** `build-macos-intel` targets `macos-13`, and GitHub retired that image in December 2025. Both v0.6.0 runs sat queued for the full 24-hour limit waiting for a runner that no longer exists, while linux, macos-arm and windows finished in 8-15 minutes; `create-release` was skipped because it `needs:` all four. v0.4.0, v0.5.0 and v0.5.1 were cancelled the same way. That is why the repository has tags but no releases and no published binaries. Moved to `macos-15-intel`, GitHub's documented replacement for x86_64, and bumped the release action off an unmaintained v1. **The Docker build script was pinned to 0.3.0** while the project shipped 0.4.0, 0.5.0 and 0.5.1 - so running it would have overwritten the 0.3.0 tags and published nothing under the current version. It now reads the version from the workspace manifest, so it cannot drift again, and refuses to overwrite a tag that is already published unless ALLOW_OVERWRITE=1. `latest` and the version tag are pushed together. **Release verification never verified anything.** The `tests` service in docker-compose.release.yml pointed first at a script that did not exist (B10), then at `integration_test.py` - which builds and starts its own server with `cargo run`, and so cannot run in the `python:3.11-slim` container it is given, with only ./scripts mounted and no Rust toolchain. Replaced with `release_smoke_test.py`, which talks to the running container over HTTP: waits on `/health/ready` (which probes the store, unlike `/health`), asserts `/health/live` and that `/metrics` exports at least one series, checks the Iceberg `/v1/config` shape, and - in no-auth mode - round-trips a catalog. The compose default moves 0.6.0 -> 0.7.0, and the service now mirrors the server's own PANGOLIN_NO_AUTH instead of a TEST_MODE nothing read. **The advisory named the wrong affected range.** SECURITY.md said "0.6.0 is affected"; the published image `alexmerced/pangolin-api` was last pushed at 0.5.1 and no 0.6.0 image ever existed, so most deployments are on 0.5.1 - which carries everything in the advisory plus everything 0.6.0 fixed. Corrected to `< 0.7.0`, with a note telling readers to check what they are actually running. CHANGELOG's Unreleased section becomes 0.7.0, dated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The release smoke test's first real run, against the built 0.7.0 image, found this. It would have shipped. `shutdown_grace` is meant to bound the *drain* - how long shutdown may spend finishing in-flight requests after SIGTERM. B16n implemented that as: tokio::time::timeout(shutdown_grace, serve) `serve` is the whole server, not the drain, so this bounded the lifetime of the process. With the default 25-second grace, the server started, served happily, and then exited 0 with "drain did not finish within the shutdown grace period" having received no signal at all. Every container would have crash-looped, and `PANGOLIN_SHUTDOWN_GRACE_SECS` was a countdown to death rather than a limit on draining. All 18 CI jobs passed with this present. So did the full workspace suite, twice, against live databases. Nothing runs the binary for longer than 25 seconds. The deadline now lives in `shutdown_signal`, armed only once a signal has actually been seen, as a detached task so returning still lets axum begin its drain. Verified directly: with a 5-second grace the server stayed up through 20 seconds of polling at health=200, then exited 2 seconds after SIGTERM with "shutdown complete". The `docker` job now runs the built image with a 5-second grace, waits 20 seconds, fails if it is no longer serving, and then confirms it still stops promptly. That is the check whose absence let this through. Two further defects in the release harness, both found the same way: * It inherited the developer's `.env`. Compose auto-loads it, so a local PANGOLIN_ROOT_USER / PANGOLIN_ROOT_PASSWORD fed the verification - and since that password is a placeholder here, the server's own config guard refused to start, so the harness failed for a reason unrelated to the artifact. Its inputs are now RELEASE_* names that cannot collide with a deployment's. * It shared a Compose project with the development database stack, both deriving `pangolin` from the directory name. `down -v` in the release harness deleted the dev stack's MinIO volume - which is exactly how the Postgres compliance test started failing with NoSuchBucket midway through this work. A maintainer verifying a release while a dev stack was up would have destroyed their local volumes. It now declares `name: pangolin-release`, and its MinIO no longer publishes host ports it never used. Verified: 59 test targets, 370 tests, zero failures against live PostgreSQL, MongoDB and MinIO. Release gate green end to end: all 10 checks, including an authenticated tenant round trip and the assertion that the same write without credentials is refused. clippy 31 at budget, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tion `config drift` failed on the previous push: its "every compose file must be valid" step exports PANGOLIN_JWT_SECRET, but docker-compose.release.yml now takes RELEASE_JWT_SECRET and RELEASE_ROOT_PASSWORD - required via `:?`, so `docker compose config` cannot render the file without them. Renaming those inputs so a developer's .env could not feed the release harness broke the one job that renders every compose file. Also corrects a justification that my own fix made false. The drift guard exempts PANGOLIN_API_URL, and the comment explained that as "nothing reads it, because the script that service runs cannot run in that container". That was true when written and is not now: scripts/release_smoke_test.py reads it. The exemption is still needed - the guard greps the Rust server, and this is a Python script - but for a different reason, and a stale reason in a guard is how the guard stops meaning anything. All four steps of the job verified locally against the actual commands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The login endpoint had no rate limit of any kind and was brute-forceable. Global concurrency and body limits and a request timeout existed; none of them made the thousandth password guess cost more than the first. Bcrypt slowed each attempt, which raises the price of a broad campaign but does nothing against a targeted guess at one weak password - and makes the endpoint an efficient way to burn the server's CPU. Two keys, because either alone has a blind spot: per source address bounds one attacker working through many accounts, per account bounds many addresses working on one, which is what credential stuffing looks like and what a per-address limit cannot see. Both are checked before any password verification, so a refusal costs a cache lookup rather than a bcrypt round. A successful login clears the account counter. `X-Forwarded-For` is honoured only when the operator sets PANGOLIN_TRUST_FORWARDED_FOR. Trusting it unconditionally would let a caller set a fresh value per request and bypass the per-address half - which is worse than no limit, because it still reads as protection. The limit is off in `RouterOptions::default()` and on in `app_with_config`. The in-process tests build with the former and drive login far harder than any real client; the production default belongs with the production config, where PANGOLIN_AUTH_RATE_LIMIT defaults to 10 a minute. `main` now serves via `into_make_service_with_connect_info`. Without it the `ConnectInfo` extractor finds nothing and every attempt shares one bucket - the limiter would appear to work while being trivially defeated. 14 tests. The 8 unit tests cover the counter; the 6 integration tests drive the real router, because a limiter that exists and is never consulted is the failure mode that let this endpoint stay unprotected while `moka` was already in the dependency tree for caching. Two of them isolate each half: many addresses against one account can only be refused by the account key, one address against many accounts only by the address key. Known limitation, documented in README and SECURITY rather than buried: the counters are in-process, so the limit is per replica. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A warehouse holds the credentials Pangolin uses to reach a customer's object storage. They were plaintext JSON in the catalog database, so anything able to read one row of `warehouses` - a backup, a read replica, a snapshot, an analyst with SELECT - held every tenant's cloud keys. AES-256-GCM with a fresh 96-bit nonce per value, stored as `enc:v1:<base64>`. `ring` was already in the tree via jsonwebtoken, object_store and rustls, so this declares a dependency that was already being compiled. An allowlist of credential keys rather than encrypting the whole map: bucket, region, endpoint and account name are not secrets, the object-store factory compares and concatenates them, and sealing them would break storage access for no gain. Both the dotted and undotted spellings are covered, because the UI writes `secret_access_key` while parts of the server read `s3.secret-access-key` and missing one means a credential stays in clear with no error anywhere. Sealed on create *and* update in all three persistent backends. Sealing only on create would protect the first credential and leak every rotation, which is worse than not doing it, because the table would look encrypted. The memory backend is excluded deliberately - it loses everything on restart, so it has no "at rest" - and the test file says so rather than quietly skipping it. Three limits, documented rather than implied away: * Off unless PANGOLIN_ENCRYPTION_KEY is set. Requiring it would break every existing deployment on upgrade. The server warns loudly at startup instead, because a security control that silently does nothing is the exact failure mode this audit keeps turning up. * Reads tolerate plaintext, so a database written before this still loads. Those rows stay unsealed until rewritten; docs/operations/encryption.md explains how to find and force that. * It protects a stolen database, not a compromised host: the key is in the server's environment. The wrong key fails loudly - GCM authenticates - with an error naming PANGOLIN_ENCRYPTION_KEY rather than returning rubbish. 17 tests. The 12 unit tests use explicit keys rather than the environment, after the first draft raced: they mutated a process-global and one test's `unset` decided another's behaviour, which passed under --test-threads=1 and failed otherwise. The 5 integration tests read the raw stored bytes through their own database connection rather than through the store, because asking the store to read back its own writes would pass just as happily if `seal` were never called. Verified: 61 test targets, 401 tests, zero failures against live PostgreSQL, MongoDB and MinIO. clippy 31 at budget, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ing success (A-24) Two defects. The second is worse than the one the roadmap named. The branch row and its copied assets were written by independent statements, so a failure between them left a branch that existed holding an arbitrary subset of its assets, with no rollback and no repair tool. And the copy error was logged and discarded - `Err(e) => tracing::error!(...)` - after which the handler returned 200. The caller was told the branch was ready when it was empty. The explicit-asset path did the same more quietly: `if let Ok(_)` around each create, and `continue` past any name it could not parse, so naming five assets and getting none was a success. `create_branch_with_assets` does both in one transaction on PostgreSQL and SQLite. Postgres copies with a single `INSERT ... SELECT`, so there is no window where some rows exist and others do not, and a large branch need not fit in memory. A malformed asset name now fails the whole operation. MongoDB has no atomic version and says so rather than pretending: the trait default is an error, the API takes the sequential fallback deliberately, logs that a partial failure will leave the branch incomplete, and returns a 500 naming the branch instead of a 200. A backend that cannot do something should be visible, not silently degraded - the unreachable transaction fallback in `delete_catalog` was the same shape of problem. The rollback test was checked for being load-bearing rather than assumed: committing the branch row before the failure point makes it fail with "the branch row survived a failed create", and it passes again once restored. A happy-path-only test would have passed against the old code. Verified: 62 test targets, 407 tests, zero failures against live PostgreSQL, MongoDB and MinIO. clippy 30, below the budget of 31. fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… backends have MongoDB had two indexes - commits(parent_id) and active_tokens(user_id) - created at startup with their errors discarded by `.ok()`. Everything else was a collection scan: every catalog lookup, every asset resolution on the Iceberg commit path, and worst, the role and permission reads that run on *every* authenticated request. The full set is now created at startup, derived from the filters this backend actually issues rather than from what seemed likely. Each entry carries the reason it exists, so a later reader can tell which are safe to drop. The part that matters most is not performance. Several are unique, which is the constraint the SQL backends express as a primary key. MongoDB previously accepted two catalogs of the same name in one tenant and returned an arbitrary one on lookup - a correctness difference from the other three backends, not a tuning difference. Catalogs, warehouses, branches, tags and one business-metadata record per asset are now enforced, and there is a test that creates a duplicate catalog and requires the second create to fail. Failures are reported rather than swallowed. A unique index cannot be built over a collection that already holds duplicates, and that deserves an error naming the collection and stating plainly that the constraint is not in force. Startup continues: refusing to boot over a missing index would turn a performance problem into an outage. One unit test asserts the kebab-case collections are spelled as stored. An index on `user_id` where the field is `user-id` indexes nothing and fails silently - the same spelling trap that made every role assignment unreadable and cost eight separate bugs to find. Verified: 63 test targets, 415 tests, zero failures against live PostgreSQL, MongoDB and MinIO - checked untruncated, after an earlier run's output was cut short by a `head -30` and only appeared to be clean. clippy 30 at budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the roadmaps ## Iceberg REST (A-5) `listViews`, `viewExists`, `dropView` and `registerTable` had no route, so a client got a routing 404 - indistinguishable from a catalog answering "no such view". `SHOW VIEWS` returned nothing, and a view created through the Iceberg API could not be removed through it. `dropView` refuses to delete a table addressed as a view: both are assets, and without the kind check view-drop rights would remove tables. `registerTable` refuses a metadata location it cannot read, because registering a missing one leaves a table whose every later `loadTable` fails far from the cause. `loadNamespaceMetadata` and `namespaceExists`, also listed under A-5, turned out to have been wired during the 0.7.0 work. `commitTransaction` stays absent, deliberately. The spec promises multi-table atomicity; the store commits per table with compare-and-swap and has no cross-table transaction. Routing it and committing one at a time would be worse than leaving it out, because an engine that sees the endpoint relies on the guarantee. A test pins the decision and tells whoever removes it what to check. ## Multi-replica The token cleanup job was never started. It was defined, the module was declared, and nothing called it - so `revoked_tokens` grew for the life of every deployment, and the revocation check reads that table on every authenticated request. It runs now and staggers its start, so replicas from one rolling deploy do not sweep in lockstep. Jitter rather than leader election: the sweep is a `DELETE ... WHERE expires_at < now` and is safe to run concurrently, so a lock table and lease would be complexity spent serialising something that does not need it. The remaining constraints - OAuth needs session affinity, rate limiting is per replica, a rotated credential can be served by a peer for the cache TTL - are documented rather than implied away. ## Backup and recovery (C-16) A drill that dumps, destroys the schema, restores, and verifies: a canary row, matching row counts, and a populated `_sqlx_migrations` without which the next startup re-runs every migration and fails. Run for real against PostgreSQL 15 with 1,345 rows: 7s backup, 53s restore. Writing it found three things. `pg_dump` refuses to dump a newer server *after* you believe you have a backup, so the version check is up front. PANGOLIN_ENCRYPTION_KEY is not in the dump, so a team that backs up religiously and never records the key restores a catalog full of unreadable credentials. And my own script had a SIGPIPE bug - `| head -1` under `set -o pipefail` - that raced on the host and failed every time in the container. ## Performance A load harness, and a correction to it. The first version used `urllib.request.urlopen` per request and reported ~33ms and 504 req/s against a server whose own histogram said 29 microseconds: it was measuring Python. With one keep-alive connection per worker the same server reports 5086 req/s. I came close to publishing the first set, which would have understated the catalog by about 1000x. It now prints the server's own means alongside its own and says that a large gap means the generator is the bottleneck. Measured: /health/ready 0.018ms server-side, /v1/config 0.023ms, an authenticated catalog list 0.060ms - so authorization roughly triples server-side cost and is still 60 microseconds. ## Roadmap reconciliation AUDIT_EXECUTION_PLAN.md and roadmap_aug10.md are historical records that were being read as to-do lists. Both carry a status header; the readiness scorecard is updated in place with the original ratings kept beside it so the direction of travel is visible; every recommended improvement and feature carries its real state, including the ones still not done. STATUS.md is the single reconciled view, and says the uncomfortable part plainly: 0.7.0 and 0.8.0 are unpublished, so anything running Pangolin today is on 0.5.1 from December and predates every security fix in this effort. Verified: 64 test targets, 425 tests, zero failures against live PostgreSQL, MongoDB and MinIO. clippy 30 at budget, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
What the OAuth flow did before this was authorization, not authentication: it exchanged a code for an access token, called the provider's userinfo endpoint, and believed the response. That is sufficient only if the access token could not have come from anywhere else, and establishing that is what OIDC is for. Now, for every provider that supports it: * PKCE (S256). A code intercepted from a referrer header, a proxy log or shell history on a shared machine cannot be redeemed without the verifier. The verifier is held server-side, deliberately not in `state`: `state` travels through the browser in the same URL as the code, so putting it there would hand it to exactly the attacker PKCE exists to stop. * `id_token` signature validation against the provider's JWKS. * `aud` must contain our client_id. Without it a token minted for a different application at the same provider logs its holder in here. * `iss`, against a discovery document whose own `issuer` is checked to match where it was fetched - otherwise `iss` validation is circular, because an attacker supplying the document also supplies the token. * `exp` with 60s leeway for clock skew. * `nonce`, so an id_token seen in one flow cannot be replayed into another. * Asymmetric algorithms only. `alg` is attacker-controlled, and accepting HS256 would let anyone holding the provider's public key forge a token, since for HMAC that is also the verification key. Discovery and JWKS cache for an hour. An unknown `kid` - what rotation looks like - forces one refetch, rate-limited to once a minute per provider. Without the refetch a rotation breaks every login until the cache expires; without the limit a stream of junk kids is a denial-of-service against the provider and against our own latency. GitHub is not an OIDC provider and is not treated as one: no id_token, no JWKS, so its logins still use userinfo and the code says so rather than reporting validation it did not perform. PANGOLIN_OIDC_REQUIRE refuses any provider that cannot be validated; off by default, because enabling it would break a working GitHub deployment on upgrade. Tested against a wiremock provider serving a real discovery document and JWKS, with tokens signed by a real 2048-bit RSA key. Nothing is mocked at the crypto layer, because the properties under test are the crypto. The first check that those tests were load-bearing was inconclusive: removing `set_audience` did not fail the confused-deputy test, because jsonwebtoken also rejects a token carrying an `aud` when none is configured, so the token was refused for an unrelated reason. The validation errors now name which check fired - better for an operator reading a log, and enough for the test to tell the difference. Re-verified: disabling audience validation fails that test, and weakening the nonce comparison fails the replay test. `check_env_var_docs.sh` caught all five new settings missing from the environment-variable reference, which is what it was added for. Documented; 42 variables now match the code. Verified: 65 test targets, 448 tests, zero failures against live PostgreSQL, MongoDB and MinIO. clippy 30 at budget, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Implements every finding in
roadmap_aug10.md— the full-repo audit of2026-08-10 — plus the improvements it recommended, and the follow-up work that
audit could not have found.
This is a security release. 0.6.0 carries a privilege escalation that any
authenticated principal can exploit. See
SECURITY.md for the
advisory and the upgrade guide
for the compatibility detail.
Verification
cargo test --workspacecargo clippycargo fmt --all --checkpytest/ruff(SDK)npm run build(UI)The security cluster
Ten issues exploitable with nothing but a valid credential — including the
lowest-privilege
tenant-useror any service-user API key:POST /api/v1/tokenstook no session at all and mapped abody-supplied
roles: ["Root"]into signed claims. Any authenticated callercould mint a
Roottoken for any tenant;check_permissionshort-circuitsfor
Root, so that token bypasses every subsequent check in the system.table, and hardcoded
read+write. Any tenant member obtained cloud storagecredentials for the entire warehouse.
session.user_id; revocation is keyed byjti, whichno token carries as its
user_id. Logout did nothing.client_credentialsfor afresh JWT, indefinitely.
PANGOLIN_DEV_MODEwaived theNO_AUTHpublic-bind guard — and thetwo flags are routinely set together.
rename_table,update_namespace_properties, the view endpointsand
perform_maintenancehad no authorization check at all.Plus B0i (cross-tenant grants), B0m (a remote panic in token issuance) and B0o
(unrevocable tokens).
What the guardrails found that review did not
Two suites were added because the entire authorization cluster was invisible to
CI — the code compiled, was formatted, was lint-clean, and the tests passed.
The permission matrix drives each sensitive route as Root / tenant admin /
ungranted tenant user / foreign tenant admin. Verified by reintroducing B0a
behind a one-line edit: it fails with
200 where 403 expected.The cross-backend parity suite runs the same assertions against all four
stores. On its first run it found:
SqliteStorehad no inherentrevoke_token, so the trait delegation calleditself — revoking a token on SQLite aborted the process.
audit_logstable declared different columns than the codeinserted, so that backend kept no audit trail at all.
Pointed at a live PostgreSQL and MongoDB for the first time, it found four more:
business_metadatawhilesearch_assetsjoins it.the deserializer expects Binary. Every role-derived permission silently
vanished: a user holding an admin role was authorized as though they held
none.
get_metadata_locationhad no fallback, so the read path and thecommit CAS disagreed about "current".
delete_catalogfailed outright on any standalonemongod.Also in here
default-spec-id,last-partition-id,"type": "struct",metadata-log; nested namespaces that registered underone key and were looked up under another (every commit to one 404'd); a
client able to jump the sequence counter to
i64::MAX; feature-branchcommits moving
main;create_tablesilently dropping every complex-typedcolumn.
revocation no-op, an orphaning branch delete, a search that panicked, a lost
CAS, and all three persistent backends rewriting 15 of 17 asset types to
IcebergTable.agreeing, so deployed builds called the visitor's localhost), ~13 raw
fetches that 404'd outside the dev proxy, three endpoints that did not exist,
no 401 handling, and a dead tenant switcher.
wrong fields, or nothing at all. All of it survived because every command
swallowed its error and exited 0.
docker compose upcould not start the API (B8), bothcompose files set a variable nothing reads (B9), and the release compose file
pinned an image four versions old running a script that does not exist (B10).
New CI
guardrails(authz matrix + parity against live Postgres/Mongo/MinIO, failingif a backend was skipped),
sdk,ui, andconfig-drift— which validatesthe compose files, the env-var reference, and that all five artifacts carry one
version.
Reviewing this
The commits are one per cluster and each message explains what was broken and
why it mattered. Reading them in order is the intended path.
Two things to weigh in on:
deny_unknown_fieldson request bodies, the branchsemantics change, and four SDK signatures. All are documented in the upgrade
guide with what each was silently doing before — but they are behaviour
changes and worth your judgement.
.envhas been removed and must be rotated. It wasnever tracked by Git but sat in plaintext and was passed into containers by
docker compose.🤖 Generated with Claude Code