fix: map system buckets and keep durable artifacts stable under the tenant-rooted layout #1863 - #1889
Open
DmytroZaichenkoDev wants to merge 14 commits into
Open
DmytroZaichenkoDev wants to merge 14 commits into
DmytroZaichenkoDev wants to merge 14 commits into
Conversation
TenantLayoutTransform knew four bucket locations; the codebase builds descriptors against four more. Each threw under the tenant-rooted layout, and because these paths are composed off the request path the failures took out a subsystem rather than a request: deployment_cost_stats/ per-request cost accounting, on the /v1/deployments path background_jobs/ background job scanning response_mappings/ the response id index api_key_data/ per-request keys, so every application caller They now map to a reserved .system/ segment at the root, above any tenant, which is what preserves today's behaviour: the job scheduler scans its bucket whole and all four are keyed by globally unique ids. Whether this state should become tenant-scoped is a question for the phase that turns multi-tenancy on. The locations move to ResourceDescriptor so a layout can enumerate them, and their callers reference those constants - a second literal next to the caller is how these came to be missed. An unregistered location still throws rather than falling back silently. Separately, invitation ids encoded getAbsoluteFilePath(), which the seam made layout-dependent, so getInvitationResource could not parse them back and share-by-link returned 404. They now use getStableFilePath(), which is the legacy shape whichever layout is active - an id handed to a user has to keep resolving across a layout change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 2, 2026
The AAD lives on inside stored ciphertext, so binding it to the physical path means a storage-layout change silently voids every stored secret: config entities drop out of the merged config as invalid, external-service and background-job keys stop decrypting. Bind it to the stable path instead, which is byte-identical under the legacy layout. Proven with real AES-GCM: a secret encrypted under the legacy layout decrypts after the flip, and physical-path ciphertext does not. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nto fix/issue-1863-system-buckets
ApplicationService keys a public function app's deployment folder as a synthesized sub-bucket, public/deployments/<id>/, which the transform rejected — every deploy of a public app threw under the tenant-rooted layout. Public sub-buckets now keep their legacy suffix under the tenant root; dotted segments are rejected so the mapping stays reversible, since dotted names there are reserved for principal branches and type folders. The private and review variants already worked via multi-segment principal ids and are pinned by the new test alongside the public one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ames #1863 The tenant-rooted tree gives .org/.system/.users/.keys and dotted type folders one namespace, and full-path parsing depends on the reserved names staying unambiguous. No ResourceTypes group takes one today, but nothing kept a future one off them; now the transform rejects the collision and the every-type round-trip test fails at build time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…1863 Review findings on the seam, applied in place: - The Users/ and Keys/ prefixes lived twice — formatted by BucketBuilder, re-declared by the transform "because this module cannot depend on it". Both now reference ResourceDescriptor, the same cure the system-bucket literals got. - getStableFilePath re-inlined the composition rule the seam centralises; both paths now go through getStoragePrefix(layout), so the stable path is the legacy layout's composition by construction, not by comment. - Principal ids now reject dotted segments the way public scopes already did: ".." escapes the tree, and dotted names are reserved. - BackgroundJobService derives its AAD through one helper, mirroring SecretFieldProcessor; SYSTEM_LOCATIONS is a Set (membership, no order); stray blank lines dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… gaps #1863 Production: - Tenant ids are now validated by the same rule the transform applies to everything else: a separator would nest one tenant's root inside another's, a leading dot collides with reserved names. Checked at construction and on every composition. - The constant re-export aliases are gone — a pattern with no precedent here, two public names per value, and the naming had already drifted (_BUCKET_LOCATION vs _LOCATION). Callers reference ResourceDescriptor. - The transform's PLATFORM_LOCATION ("") no longer shadows ResourceDescriptor.PLATFORM_LOCATION ("platform/"): renamed to TENANT_TREE_ROOT. - One vocabulary for the type-group parameter across the interface and both layouts; javadoc trimmed to contracts; the rejection policy moved onto toTenantLocation, where it is enforced. Tests: - ApplicationDeploymentLayoutTest now composes locations through ApplicationService.deploymentFolderLocation instead of pinning literals that could drift from what production synthesizes. - Closed branch gaps: dotted rejections in the tenant-to-legacy direction, the Users// boundary, resolveByPath under the tenant layout, invalid tenant ids both at the transform and the constructor. - Layout flips moved to @beforeeach, the lone try/finally restore aligned with the @AfterEach pattern, dead marker indirection inlined, war-story comments trimmed to their invariants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DmytroZaichenkoDev
marked this pull request as ready for review
September 3, 2026 15:05
DmytroZaichenkoDev
requested review from
Oleksii-Klimov and
astsiapanay
as code owners
September 3, 2026 15:05
astsiapanay
reviewed
Sep 4, 2026
Agent noun per review on #1866; landed here because every commit above the seam edits this class and a bottom-of-stack rename would conflict through all of them for the same end state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
storage.layout.{tenantRooted,defaultTenant} per review on #1866 — the
layout is a storage concern. The block is stripped before the Storage
POJO decode because the codec rejects unknown properties.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per review: the name now says what the path is (the legacy layout's composition) rather than the property it has. The contract stays in the javadoc — durable artifacts derive from this path, never the physical one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per review: the four system-bucket constant pairs and SYSTEM_LOCATIONS move off ResourceDescriptor into an enum owning the bucket/location vocabulary; the transformer asks isSystemLocation instead of set membership, and callers name their bucket by entry. Also documents resolveTypeFolder's parameter, which the vocabulary rename had left unexplained. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nto fix/issue-1863-system-buckets
astsiapanay
approved these changes
Sep 7, 2026
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stacked on #1866.
TenantLayoutTransformhandled four bucket locations; the codebase builds descriptors against four more, and each threw under the tenant-rooted layout. These paths are composed off the request path, so the failures took out a subsystem rather than a request:deployment_cost_stats//v1/deploymentspathbackground_jobs/response_mappings/api_key_data/They now map to a reserved
.system/segment at the root, above any tenant — that is what preserves today's behaviour, since the job scheduler scans its bucket whole and all four are keyed by globally unique ids. Whether this state should become tenant-scoped belongs to the phase that turns multi-tenancy on.The locations move to
ResourceDescriptorso a layout can enumerate them, and their callers now reference those constants; a second literal next to the caller is how these came to be missed. An unregistered location still throws rather than falling back silently.Durable artifacts must not follow the physical path. The seam made
getAbsoluteFilePath()layout-dependent, which breaks anything derived from it and then stored:getInvitationResourcecould not parse a pre-flip id back and share-by-link returned 404.SecretFieldProcessor(every@EncryptedFieldon config-managed entities),UserExternalServiceService(user-authored client secrets),BackgroundJobService(the per-request key inside a job record). Ciphertext keeps the AAD it was sealed with; after a layout flip the AAD re-derives differently and decryption tag-mismatches. For config entities the failure is silent: the entity drops out of the merged config as invalid.All four now use
getStableFilePath(), which is byte-identical to the legacy physical path under any layout.SecretFieldProcessorLayoutTestproves it with real AES-GCM: a secret encrypted under the legacy layout decrypts after the flip, and ciphertext bound to the tenant-shaped physical path does not.Public sub-buckets.
ApplicationServicekeys a public function app's deployment folder as a synthesized sub-bucket,public/deployments/<id>/, which the transform rejected — every deploy of a public app threw under the tenant-rooted layout. Public sub-buckets now keep their legacy suffix under the tenant root; dotted segments are rejected so the mapping stays reversible, since dotted names there are reserved for principal branches and resource-type folders. The private (Users/<sub>/deployments/<id>/) and review (Users/<sub>/publications/<pub>/deployments/<id>/) variants already worked via multi-segment principal ids and are pinned byApplicationDeploymentLayoutTestalongside the public one.Reserved names are now enforced. The tenant-rooted tree gives
.org/.system/.users/.keysand dotted type folders one namespace, and full-path parsing depends on the reserved names staying unambiguous. NoResourceTypesgroup takes one today, but nothing kept a future one off them; the transform now rejects the collision, and the every-type round-trip test doubles as the build-time tripwire.Found by the P1 verification instruments (#1870) and a follow-up audit of physical-path consumers.
🤖 Generated with Claude Code