feat(storage): route quarantined data to a dedicated root prefix (BLDX-1644) - #3664
Draft
hariharanatlan wants to merge 1 commit into
Draft
hariharanatlan wants to merge 1 commit into
hariharanatlan wants to merge 1 commit into
Conversation
Connectors have started downloading raw files straight from the source system into object storage. Raw source content carries warehouse hostnames, schema references, usernames, authoring filesystem paths and literal filter values, so it has a materially different security posture from the metadata artifacts the storage tiers were designed for. Add a `quarantined` boolean, orthogonal to StorageTier, that roots a tier's ordinary prefix under QUARANTINE_PREFIX (default `quarantine`, settable via ATLAN_QUARANTINE_PREFIX). Tier stays lifecycle-only, so quarantined data still exists at all three lifecycles and each tier gets its own default location beneath one root — a single access/retention policy can then cover every quarantined object. Prepending rather than inventing a flat layout keeps run-scoping, app-scoping and per-tier cleanup working unchanged. Opt-in: defaults to False everywhere, so every existing storage key is unchanged and no connector is relocated. Two correctness details: - PROTECTED_STORAGE_PREFIXES gains `quarantine/persistent-artifacts/`, which does not start with `persistent-artifacts/` and would otherwise be unprotected from cleanup. - cleanup_storage sweeps the quarantined run prefix alongside the ordinary one, so an explicit run cleanup does not leave sensitive bytes behind. UploadInput rejects `quarantined=True` combined with an explicit storage_path: the explicit key is used verbatim and bypasses tier resolution, so the pair would silently write a non-quarantined key. storage_subdir is the supported way to place files within the prefix. Quarantine is a location and a routing guarantee, not redaction — the restricted IAM, encryption and retention that make the root meaningfully secure are provisioned outside the SDK. Atlan's blob-storage proxy also allowlists writable top-level prefixes, so the quarantine root must be added there before quarantined writes can reach the upstream store. See ADR-0021.
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 81.1%) Detailed Coverage ReportThis message was truncated. Download full message |
Contributor
📦 Trivy Vulnerability Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailspackages/conformance/uv.lockuv.lock |
Contributor
📦 Trivy Secret Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailspackages/conformance/uv.lockuv.lock |
Contributor
☂️ Code Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
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
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 the storage standard agreed in the platform design discussion on raw-context asks (Slack thread) — BLDX-1644.
Why
Connectors have started downloading raw files straight from the source system into object storage so downstream apps can read the original artifact rather than the metadata extracted from it. Tableau (REQ-1635) and PowerBI (REQ-1636) do this today; more ECL use-cases are expected to need raw source context.
Raw source files are a different kind of data from anything the SDK previously stored. A connector's own artifacts are metadata it produced; a raw source file is content the customer authored, and it routinely carries warehouse hostnames, database and schema references, usernames, authoring filesystem paths, and literal filter values. Inspecting each file to decide how sensitive it is does not scale and fails open on the first shape nobody predicted.
The existing tiers were designed for apps consuming files internally. Without a standard, each connector picks its own prefix — that is already happening — and every such choice is a governance decision made by whoever wrote the upload call.
What
A
quarantinedboolean, orthogonal toStorageTier, that roots a tier's ordinary prefix underQUARANTINE_PREFIX(defaultquarantine, settable viaATLAN_QUARANTINE_PREFIX).quarantined=TrueTRANSIENTfile_refs/…quarantine/file_refs/…RETAINEDartifacts/apps/{app}/workflows/{wf}/{run}/…quarantine/artifacts/apps/{app}/workflows/{wf}/{run}/…PERSISTENTpersistent-artifacts/apps/{app}/…quarantine/persistent-artifacts/apps/{app}/…Following the thread's four conclusions:
Falseeverywhere; no existing key moves.StorageTierstays the single source of truth for path generation, so every caller inherits the routing.Two correctness details worth reviewing
PROTECTED_STORAGE_PREFIXESgainsquarantine/persistent-artifacts/. It does not start withpersistent-artifacts/, so without its own entry, cleanup would delete quarantined persistent data that the unquarantined tier protects.cleanup_storagesweeps the quarantined run prefix alongside the ordinary one.build_output_path()returns only the unquarantined prefix, so quarantinedRETAINEDdata would otherwise survive a cleanup its unquarantined equivalent clears — the wrong default for sensitive bytes.One deliberate restriction
UploadInputrejectsquarantined=Truecombined with an explicitstorage_path. An explicit key is used verbatim and bypasses tier resolution, so the pair reads as "quarantine this" while writing an ordinary key — the exact silent failure the flag exists to prevent.storage_subdiris the supported way to place files within the resolved prefix, and it is also the shape the first consumer needs (moving off a hand-picked verbatim key onto tier mode).Out of scope — named, not hidden
403(code1009); today it admitsartifacts/andpersistent-artifacts/(seeapplication_sdk/storage/preflight.py). Until that entry exists, quarantined writes succeed against a deployment-owned store but fail against the upstream store.ATLAN_QUARANTINE_PREFIXis env-settable so a deployment can align the root without an SDK release.This is the contract-now / platform-later split proposed in the thread: the contract lands fast and unblocks the two connector consumers, the infrastructure trails.
Known tradeoff
Opt-in is fail-open. A new raw-from-source write lands in an ordinary prefix if its author does not set the flag. Opt-in was chosen deliberately — default-on would relocate every existing connector's artifacts, a breaking change no consumer asked for. A conformance rule flagging un-quarantined source-download writes is the natural follow-up once adoption exists. Recorded under Consequences in the ADR rather than left implicit.
Testing
uv run pytest tests/unit→ 9823 passed, 0 failed, 9 skipped. This is the load-bearing assertion: every pre-existing test passes untouched, so no object-store key changed and the flag really is opt-in._file_ref_basequarantining as a pure prefix,RETAINEDstill requiring a run prefix, round-trip throughmodel_dump, payloads written before the field existed still deserialising, theUploadInputguard, the flag reaching the returned ref on all three upload branches (including the cross-pod deployment-store fallback), persisted/materialised refs, and all three cleanup behaviours.ruff check+ruff formatclean;pyright0 errors (11 pre-existing warnings on untouched lines).uv run poe regen-capabilitiesrun; manifest committed.uv run pre-commit run --all-filescould not complete locally — a hook's Go toolchain fails to parse its owngo.mod(invalid go version '1.25.0'). It fails identically on an untouched file, so it is a pre-existing local environment issue, not this change; the Python hooks were run directly instead.Adoption
A connector already on
App.uploadmoves from a verbatim key totier=+quarantined=True+storage_subdir=. Tableau is the first consumer once a release ships.Docs: ADR-0021,
docs/concepts/storage.md,docs/concepts/file-reference.md,docs/configuration.md.🤖 Generated with Claude Code