Commit Graph
6 Commits
Author SHA1 Message Date
Chris LuandDevin ee315527cb vector: address remaining s3vectors review findings (#12115)
* vector: address remaining s3vectors review findings

Catalog and permission model:
- Vector buckets are declared Lance table buckets end to end: create sets
  format=lance, get/list/delete reject or skip ICEBERG buckets, and the
  format lookup runs on an internal trusted catalog so callers need no
  GetTableBucket grant.
- The implicit "vector" index namespace is created through the same
  internal manager, so a CreateTable grant alone authorizes CreateIndex;
  typed S3TablesError codes replace string matching on err.Error().
- CreateIndex pre-checks existence after the authorize, tags the record
  with a create nonce, and rolls back on engine failure under
  context.WithoutCancel with a versionToken CAS, so a retry never deletes
  a table it did not create.
- DeleteIndex drops the catalog record directly; DropIndex is
  unimplemented on the shared VectorService.
- DeleteVectorBucket removes the empty internal namespace on
  BucketNotEmpty and retries once.
- PutVectors/DeleteVectors require the update grant, and engine gRPC
  errors map to typed API errors instead of generic 500s.

API correctness:
- CreateVectorBucket/CreateIndex return their resource ARNs.
- ARNs reject empty bucket segments; vectorBucketArn must be an ARN.
- ListIndexes returns summaries built only from catalog fields.
- ListVectors honors maxResults/nextToken inside a segment via
  "seg:<pageStart>:<nextIndex>" resume tokens and validates
  segmentCount/segmentIndex.
- QueryVectors requires positive topK; PutVectors rejects empty keys.
- CreateVectorBucket rejects a supplied encryptionConfiguration rather
  than silently dropping it.
- Metadata filters emit Lance's real JSONB UDFs: json_get_string/
  json_get_float/json_get_bool picked by operand type, json_exists with a
  bracketed JSONPath, and per-member accessors for $in/$nin.

Engine:
- weed-vector-server stores the metadata column as Lance JSONB
  (lance.json extension) so the json_* UDFs apply; reads accept legacy
  Utf8 JSON columns.

Vectors port TLS:
- The TLS config is built before binding listeners, loads -cacert.file
  into ClientCAs when client-cert verification is on, fails startup on an
  empty/invalid CA file, and closes the gRPC conn on every failure path.

* s3vectors: authorize index create before writing the implicit namespace

A denied caller could still leave a persistent namespace behind because
ensureIndexNamespace wrote through the trusted catalog before the
caller's create grant ran. AuthorizeCreateTable now tolerates the
not-yet-declared implicit namespace (bucket policy alone decides) and
runs before any write; real table creates keep requiring the namespace.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3vectors: keep engine error details out of the API response

Unavailable now maps to 503 ServiceUnavailableException and unclassified
codes return a generic message; the real gRPC error stays in the log.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3vectors: exact-type metadata filters + NULL-safe $nin

The jsonb extractors convert numeric-looking strings through
json_get_float and 'true'/'false' through json_get_bool, so a stored '3'
could satisfy a filter for the number 3. Numeric and boolean comparisons
now also require the stored value not be a string, and each $in/$nin
group coalesces NULL so NOT cannot invert NULL into NULL.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3: validate the vectors TLS key pair before binding

The reloading certificate provider reads the files asynchronously, so a
missing or mismatched pair would only surface as failed handshakes on an
announced HTTPS port. Load it once up front.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: make a token-checked table delete atomic

The version token was read in one filer call and enforced in another, so
a delete+recreate slipping between them let the delete erase the
replacement. When a token is supplied the name path now carries the
observed mtime as an if_not_modified_after precondition (evaluated under
the path lock), and a decoupled delete strips the catalog marker under a
compare of the captured attributes before touching the data path.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3vectors: compare filter operands against the stored JSON type

jsonb's strict converters make json_get_string return text for numbers
and booleans too, so a get_string IS NULL guard is never true for scalar
metadata — numeric and boolean comparisons silently matched nothing.
Numbers and booleans now compare through TRY_CAST of the serialized
json_extract text, which yields NULL on any other stored type; string
comparisons stay on json_get_string behind a CASE guard so the extractor
only runs on rows storing a JSON string.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: allocate implicit namespace metadata before use

A first table create in an undeclared namespace took the
allowMissingNamespace branch with a nil record and panicked on the
owner assignment.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: read the delete target's metadata and snapshot in one lookup

Two separate lookups let a delete+recreate in between validate the
caller token against the old record while the conditional delete aimed
at the replacement.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: assert the captured record before a conditional table delete

An xattr update keeps Mtime, so the IfNotModifiedAfterNs guard alone
could not see a metadata write that had already rotated the version
token, and a recreate preserving the mtime could slip the mtime check.
The delete now first proves the entry still carries the captured
attributes under the filer's path lock, then keeps the mtime
precondition.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3vectors: match only JSON boolean literals in metadata filters

* filer,s3tables: delete versioned tables under one attribute precondition

* s3vectors: compare serialized JSON for string filter operands

json_get_string converts numeric-looking strings and booleans, so it
cannot serve exact-typed string comparisons even behind a guard. String
operands now compare the serialized json_extract text directly, which is
exact-typed and never errors; a Rust test pins the semantics against a
mixed-type dataset.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3vectors: compare boolean filter operands as serialized text

Arrow's boolean cast accepts '1' and '0', so a TRY_CAST column let a
stored number satisfy a boolean filter. Booleans now compare the
serialized json_extract text like strings do — a JSON boolean only ever
serializes as 'true' or 'false'.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: claim a catalog name's markers in one compare-checked write

A create wrote its markers attribute-by-attribute after checking the
name was free. A delete's restore landing in the check-to-write gap put
the old markers back first, and the create then overwrote only the ones
it sets — leaving the old tags, policy, and maintenance settings on the
new table. CreateTable, RegisterTable, and CreateView now write all
their markers in a single update that refuses to run while any catalog
marker is present, so the second writer loses with a conflict instead
of inheriting them.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: test the attribute-preconditioned table delete and name claim

Adds a BeforeDelete hook plus ExpectedExtended evaluation to the
in-memory filer so tests can land a competing write in a handler's
check-to-delete window, and covers two losing races: a token delete
whose record changed since the read, and a create colliding with a
delete's marker restore.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: reserve a catalog record under one compare-checked claim

claimCatalogRecord writes a deleting marker onto the record while it
still holds exactly the caller's captured attributes, so a delete or
rename can hold the record through a multi-step operation and a losing
writer backs out before any data or destination entry is written. The
claim key joins the markers a name-claim asserts absent, so the name
cannot be reused while the reservation stands.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: claim the record before purging or publishing past it

A token delete of a decoupled table purged the data first and checked
the marker afterwards, so a rename that finished in between left the
moved record pointing at purged data while the delete reported a
conflict. The delete now claims the record under the captured
attributes before any data is removed and releases the claim if the
purge fails, keeping the record intact for a retry.

The rename claims the source before publishing the destination, so a
losing rename backs out while no destination entry exists instead of
leaving one behind pointing at data a delete went on to remove.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: test the record claim across delete and rename races

A rename completing in the token delete's check-to-claim window turns
the delete into a conflict with the data intact, and a delete's claim
on the source turns the rename into a conflict before the destination
entry is published.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-10-26 20:49:44 -07:00
Chris Lu 8d2b417c69 iceberg, lance: vend read-only credentials to callers that may only read (#11859)
* s3tables: share the UpdateTable authorization gate

Extract the validation, load, and permission check from handleUpdateTable
into authorizeUpdateTable so the table an update was authorized against is
the same snapshot the mutation writes to. Manager gains
AuthorizeUpdateTable, mirroring AuthorizeCreateTable, so callers can ask the
question without updating anything.

* s3tables: share the UpdateView authorization gate

Extract the lookup, policy load, and permission check from handleUpdateView
into authorizeUpdateView, mirroring authorizeUpdateTable, and expose it as
Manager.AuthorizeUpdateView for callers that need the answer without the
update.

* s3api: scope vended table credentials to read-only on request

VendTableCredentials takes a readOnly flag and tablePrefixSessionPolicy
emits only s3:GetObject for the table prefix when it is set, keeping the
conditioned ListBucket and unconditioned GetBucketLocation statements that
reads still need.

* iceberg: vend read-only credentials when the caller may not write

LoadTable and friends vend storage credentials only for the authority the
caller holds: the vend now checks UpdateTable/UpdateView permission and a
caller that may only read the catalog entry gets a read-only session. Paths
that just passed a write gate (stage-create, view commit) keep the full
session.

* lance: vend read-only credentials when the caller may not write

storage_options vends the same read-only session as the Iceberg catalog
when the caller lacks UpdateTable on the table.

* lance: keep write credentials for callers authorized to declare

A Lance commit is a direct S3 write that never revisits the catalog, so a
caller that just passed CreateTable still gets a writable session; the
UpdateTable gate applies to describe, the read path.

* iceberg, lance: log a failed write check instead of only reading it as denied
2026-10-20 07:16:27 -07:00
Chris Lu 28862c866e Authorize an Iceberg table create before it writes (#10991)
* s3tables: share one CreateTable authorization gate

CreateTable and RegisterTable each carried their own copy of the name
validation, policy load and permission check. Fold them into
authorizeCreateTable, and expose it on the Manager for callers that write
into a table bucket before the table itself is registered.

Claude-Session: https://claude.ai/code/session_01QiJkka1T2NAWDWq4JQ8Vuy

* iceberg: authorize a table create before it writes

Stage-create returns before the S3Tables registration that authorizes a
create, and the plain create writes its metadata file before reaching it,
so a caller who may not create the table could still leave a staged
template, a marker and a v1.metadata.json in the target bucket - and get
vended credentials for a location of their choosing. Run the CreateTable
gate as soon as the table is known to be absent.

Claude-Session: https://claude.ai/code/session_01QiJkka1T2NAWDWq4JQ8Vuy

* iceberg: authorize a create-on-commit the same way

A commit against a table that does not exist creates it, writing the
metadata file first and only then reaching the registration that checks
the caller may create it. Denied callers saw a 500 for what is a 403.

Claude-Session: https://claude.ai/code/session_01QiJkka1T2NAWDWq4JQ8Vuy

* iceberg: pin that identity actions reach the create gate

The manager request is built from the caller's own context, so an identity
whose actions carry the permission still passes. Worth a test: a fresh
context here would silently deny every such caller.

Claude-Session: https://claude.ai/code/session_01QiJkka1T2NAWDWq4JQ8Vuy
2026-08-27 16:23:51 -07:00
Chris Lu b13463880c s3tables: scope management authorization to the caller's identity (#9961)
* s3tables: resolve account-less identities to a distinct principal

Static identities with no account block default to the shared admin
account, so getAccountID returned "admin" for every such user and the
permission checks treated them all as the admin principal. Only keep the
admin account when the identity actually carries an admin action;
otherwise fall back to the unique identity name.

* s3tables: limit the open-by-default fallback to anonymous access

The legacy permission path allowed any request that no policy explicitly
denied whenever default-allow was on, which is the zero-config default.
That let an authenticated identity without table permissions reach table
resources owned by others. Restrict the fallback to requests with no
identity or the anonymous identity; authenticated callers must pass an
explicit action or policy check. Zero-config and anonymous access are
unchanged.

* s3tables: drop the no-op ListTableBuckets account gate

The top-level check passed the principal as its own owner, so it always
allowed. Per-bucket filtering in the loop is the real authority; remove
the dead gate and the now-unused locals.

* s3tables: derive the Iceberg catalog's default-allow from auth state

The Iceberg catalog reuses the S3 Tables Manager, which hardcoded
default-allow on. Authenticated callers were enforced only because the
identity struct happens to propagate into the handler; if it were ever
dropped, a secured catalog would fall open. Mirror the S3 port and set
the Manager's default-allow from the authenticator, so an authenticated
caller is enforced regardless. Shell and admin keep their own trusted
Manager. Regression test covers the struct, name-only, and admin paths.

* s3tables: drop redundant ACTION_ADMIN string conversion

ACTION_ADMIN is an untyped string constant, so the conversion is a no-op.

* s3tables: enforce name-only authenticated callers, add trusted bypass

defaultAllowFor treated a request with no identity object as anonymous,
but the Manager path forwards only the identity name (not the struct).
A name-only authenticated caller could therefore be misclassified as
anonymous and allowed under the open default. Treat a server-set identity
name as authenticated too, and add an explicit trusted flag for the local
shell/admin tooling that legitimately bypasses authorization.

* s3tables: trim verbose comments
2026-06-14 13:55:36 -07:00
Chris Lu 0d8588e3ae S3: Implement IAM defaults and STS signing key fallback (#8348)
* S3: Implement IAM defaults and STS signing key fallback logic

* S3: Refactor startup order to init SSE-S3 key manager before IAM

* S3: Derive STS signing key from KEK using HKDF for security isolation

* S3: Document STS signing key fallback in security.toml

* fix(s3api): refine anonymous access logic and secure-by-default behavior

- Initialize anonymous identity by default in `NewIdentityAccessManagement` to prevent nil pointer exceptions.
- Ensure `ReplaceS3ApiConfiguration` preserves the anonymous identity if not present in the new configuration.
- Update `NewIdentityAccessManagement` signature to accept `filerClient`.
- In legacy mode (no policy engine), anonymous defaults to Deny (no actions), preserving secure-by-default behavior.
- Use specific `LookupAnonymous` method instead of generic map lookup.
- Update tests to accommodate signature changes and verify improved anonymous handling.

* feat(s3api): make IAM configuration optional

- Start S3 API server without a configuration file if `EnableIam` option is set.
- Default to `Allow` effect for policy engine when no configuration is provided (Zero-Config mode).
- Handle empty configuration path gracefully in `loadIAMManagerFromConfig`.
- Add integration test `iam_optional_test.go` to verify empty config behavior.

* fix(iamapi): fix signature mismatch in NewIdentityAccessManagementWithStore

* fix(iamapi): properly initialize FilerClient instead of passing nil

* fix(iamapi): properly initialize filer client for IAM management

- Instead of passing `nil`, construct a `wdclient.FilerClient` using the provided `Filers` addresses.
- Ensure `NewIdentityAccessManagementWithStore` receives a valid `filerClient` to avoid potential nil pointer dereferences or limited functionality.

* clean: remove dead code in s3api_server.go

* refactor(s3api): improve IAM initialization, safety and anonymous access security

* fix(s3api): ensure IAM config loads from filer after client init

* fix(s3): resolve test failures in integration, CORS, and tagging tests

- Fix CORS tests by providing explicit anonymous permissions config
- Fix S3 integration tests by setting admin credentials in init
- Align tagging test credentials in CI with IAM defaults
- Added goroutine to retry IAM config load in iamapi server

* fix(s3): allow anonymous access to health targets and S3 Tables when identities are present

* fix(ci): use /healthz for Caddy health check in awscli tests

* iam, s3api: expose DefaultAllow from IAM and Policy Engine

This allows checking the global "Open by Default" configuration from
other components like S3 Tables.

* s3api/s3tables: support DefaultAllow in permission logic and handler

Updated CheckPermissionWithContext to respect the DefaultAllow flag
in PolicyContext. This enables "Open by Default" behavior for
unauthenticated access in zero-config environments. Added a targeted
unit test to verify the logic.

* s3api/s3tables: propagate DefaultAllow through handlers

Propagated the DefaultAllow flag to individual handlers for
namespaces, buckets, tables, policies, and tagging. This ensures
consistent "Open by Default" behavior across all S3 Tables API
endpoints.

* s3api: wire up DefaultAllow for S3 Tables API initialization

Updated registerS3TablesRoutes to query the global IAM configuration
and set the DefaultAllow flag on the S3 Tables API server. This
completes the end-to-end propagation required for anonymous access in
zero-config environments. Added a SetDefaultAllow method to
S3TablesApiServer to facilitate this.

* s3api: fix tests by adding DefaultAllow to mock IAM integrations

The IAMIntegration interface was updated to include DefaultAllow(),
breaking several mock implementations in tests. This commit fixes
the build errors by adding the missing method to the mocks.

* env

* ensure ports

* env

* env

* fix default allow

* add one more test using non-anonymous user

* debug

* add more debug

* less logs
2026-02-16 13:59:13 -08:00
Chris Lu 79722bcf30 Add s3tables shell and admin UI (#8172)
* Add shared s3tables manager

* Add s3tables shell commands

* Add s3tables admin API

* Add s3tables admin UI

* Fix admin s3tables namespace create

* Rename table buckets menu

* Centralize s3tables tag validation

* Reuse s3tables manager in admin

* Extract s3tables list limit

* Add s3tables bucket ARN helper

* Remove write middleware from s3tables APIs

* Fix bucket link and policy hint

* Fix table tag parsing and nav link

* Disable namespace table link on invalid ARN

* Improve s3tables error decode

* Return flag parse errors for s3tables tag

* Accept query params for namespace create

* Bind namespace create form data

* Read s3tables JS data from DOM

* s3tables: allow empty region ARN

* shell: pass s3tables account id

* shell: require account for table buckets

* shell: use bucket name for namespaces

* shell: use bucket name for tables

* shell: use bucket name for tags

* admin: add table buckets links in file browser

* s3api: reuse s3tables tag validation

* admin: harden s3tables UI handlers

* fix admin list table buckets

* allow admin s3tables access

* validate s3tables bucket tags

* log s3tables bucket metadata errors

* rollback table bucket on owner failure

* show s3tables bucket owner

* add s3tables iam conditions

* Add s3tables user permissions UI

* Authorize s3tables using identity actions

* Add s3tables permissions to user modal

* Disambiguate bucket scope in user permissions

* Block table bucket names that match S3 buckets

* Pretty-print IAM identity JSON

* Include tags in s3tables permission context

* admin: refactor S3 Tables inline JavaScript into a separate file

* s3tables: extend IAM policy condition operators support

* shell: use LookupEntry wrapper for s3tables bucket conflict check

* admin: handle buildBucketPermissions validation in create/update flows
2026-01-30 22:57:05 -08:00