Skip to content

fix(uploads): default an unregistered upload column by plane shape, not scope name; pin the database-scope upload surface - #1767

Merged
pyramation merged 1 commit into
mainfrom
feat/db-scope-upload-surface
Aug 19, 2026
Merged

fix(uploads): default an unregistered upload column by plane shape, not scope name; pin the database-scope upload surface#1767
pyramation merged 1 commit into
mainfrom
feat/db-scope-upload-surface

Conversation

@pyramation

@pyramation pyramation commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Track T6 of constructive-io/constructive-planning#1782 — the upload half of tenant static/SSG sites. Two of the three diagnosed defects do not reproduce on current main; this PR pins them with tests and fixes the one surviving instance of the pattern behind defect 2.

Defect 1 (db-scope upload mutations never generated) — does not reproduce. Pairing is registry/tag/FK based, not name based, since #1764: pairStoragePlane reads the @storageBuckets/@storageFiles tags and the files→buckets FK relation (graphile/graphile-storage-registry/src/pairing.ts), and graphile-presigned-url-plugin/src/plugin.ts:27 builds a surface per discovered plane via discoverStoragePlanes + uploadSurfaceNames. No regex, no prefix assumption. Now pinned at the GraphQL layer, not just the unit layer (below).

Defect 2 (WHERE scope = 'app') — the provisioner query is already fixed; one hard-coded 'app' survived. ALL_STORAGE_MODULES_QUERY (storage-module-cache.ts:41-74) selects every module for the database with no scope filter, and graphile-bucket-provisioner-plugin resolves modules by bucket-table identity, so explicit provisionBucket was fine. The remaining one was the managed-upload fallback for an unregistered upload column:

-  : allConfigs.find((c) => c.scope === 'app')          // 'database' plane → STORAGE_MODULE_NOT_FOUND
+  : globalConfigs[0]                                  // globalConfigs = allConfigs.filter(c => c.entityTableId === null)

Selection is now by shape — a global plane is one with no entity key — which is what a database-wide plane is whether it registered as database, platform, or app. No hiding: two global planes and no registry row throws STORAGE_MODULE_AMBIGUOUS naming both scopes, and zero still throws STORAGE_MODULE_NOT_FOUND.

Defect 3 (buckets.auth_sel_pub USING (is_public = true)) — reproduces; fixed in the generator in constructive-io/constructive-db#3269 (separate repo, separate PR). Verified there that public object serving does not depend on that policy: the static gateway resolves through resolve_route(), a STABLE SECURITY DEFINER resolver, and reads physical_name from the catalog buckets projection, never the tenant buckets table.

Tests

graphql/server-test/__tests__/db-scope-upload.integration.test.ts builds one schema over both an app-scope plane (app_buckets/app_files) and a new database-scope fixture whose tables are the unprefixed buckets/files, and asserts uploadAppFile and uploadFile both exist — a name-based pairing rule fails this. It then proves the db-scope path end to end against real MinIO: presigned PUT → 200, physical_name recorded on the tenant's own bucket row and equal to the bucket in the presigned URL, files row on the tenant plane only.

Unit coverage for the fallback: an unregistered column on a scope='database' plane resolves (passing 'database' to resolve_default_bucket), and two global planes refuse.

Verification

  • graphile/graphile-presigned-url-plugin: 71/71 pass
  • graphql/server-test/__tests__/upload.integration.test.ts: 45/45 (unchanged baseline)
  • graphql/server-test/__tests__/db-scope-upload.integration.test.ts: 6/6, real MinIO

Link to Devin session: https://app.devin.ai/sessions/d86629cb5b2947c7b6ab15546906ce80
Requested by: @pyramation

…ot scope name

Pin the database-scope upload surface end to end: an unprefixed buckets/files
plane emits uploadFile alongside the prefixed uploadAppFile, and a presigned PUT
lands in a real physical bucket recorded on the tenant's own row.
@pyramation pyramation self-assigned this Aug 19, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@pyramation
pyramation merged commit 82f1623 into main Aug 19, 2026
20 checks passed
@pyramation
pyramation deleted the feat/db-scope-upload-surface branch August 19, 2026 05:12
@devin-ai-integration

Copy link
Copy Markdown
Contributor

Runtime verification: the database-scope tenant upload path, proved end to end (local Postgres 18 + MinIO)

A database-scope tenant obtains a presigned URL, uploads for real, gets a real per-tenant physical bucket, and cannot see another tenant's buckets.

db-scope upload surface — 6/6, real PUT, physical_name recorded

db-scope upload suite 6/6

One schema over both an app-scope plane (app_buckets/app_files) and the unprefixed db-scope plane exposes both uploadAppFile and uploadFile; uploadFile returns a presigned PUT URL (key === contentHash, deduplicated === false); the PUT to MinIO is accepted; "tess-storage-public".buckets.physical_name is recorded and equals the bucket segment of the presigned URL; the files row lands only on the tenant plane.

lazy provisioning log

The physical bucket is really created (adversarial: bucket deleted first)

Tenant bucket names are deterministic, so "the bucket exists" alone is weak evidence. The bucket was deleted from MinIO, confirmed gone, and the suite re-run:

bucket deleted
bucket and object re-provisioned

It was lazily re-provisioned and the 24-byte object (key = content hash) landed inside it — in the tenant bucket, not the deployment-default test-bucket.

Regression — upload.integration green

51/51

51/51 across 2 suites (45 existing app-scope + 6 new; the jest pattern matches both files), exit 0.

Cross-tenant isolation, validated by re-introducing the removed policy (constructive-io/constructive-db#3269)

metaschema RLS 4/4
adversarial control 4/4

An owner-org member reads both buckets with their physical_names; a member of an unrelated org sees 0 rows, including WHERE is_public. Control: re-adding CREATE POLICY auth_sel_pub ... USING (is_public = true) makes the same outsider query leak assets and physical_name = 'tenant-a-assets-9f21'; dropping it restores blindness. Storage suites also green serially: storage-security 42/42, entity-storage-security 35/35.

Recording of the run

Advisory: the teardown log Error disposing PostGraphile[...]: PostGraphile instance has been released appears in every run (also on main) and never changes the exit code, so it cannot fail CI on its own. It would only matter if a CI step greps logs for Error, or if these suites stopped force-exiting, in which case the leaked handle could hang a job rather than fail it.

Written by Devin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant