[jeffspahr]

Problem

The artifact security fix can reject an existing root even when the namespace's storage identity already isolates its objects. A layout such as s3://external-artifacts/model/customer-17-training/ should not require moving objects merely because it does not use KFP's namespace-prefix convention. Automatically selecting storage authorization previously recognized only controller-managed static credentials for minio://mlpipeline.

Behavior

KFP_SECURITY_ARTIFACT_AUTHORIZATION_MODE=auto selects a ready namespace receiver for either the verified managed store or an administrator-approved external S3 credential-file broker. The shared UI still authorizes the user for the namespace; the namespace storage identity decides which objects may be read. Storage denial never retries with shared UI credentials. No MLMD authority or audit exception is introduced. Managed auto retains ownership/prefix checks for the legacy artifacts/ root: its broad stock grant also covers private pre-SeaweedFS V1 outputs, so readiness cannot make that history shared.

For external storage, an installation administrator can configure one adoption-policies.json policy in kfp-artifact-storage-config. It selects candidate namespaces and fixes an exact storage destination and receiver template. A trusted GitOps provisioner, controller or external-service adapter publishes current namespace UID/principal bindings in a protected ConfigMap. Binding and template sources must be explicitly referenced, labelled for discovery, and located in the KFP installation namespace. Matching labels or tenant-local ConfigMaps cannot approve access. Existing and newly provisioned namespaces enroll automatically; approvals.json remains available for non-overlapping exceptions.

The policy derives the existing approval object and reuses receiver reconciliation and readiness checks. It preserves approved broker sidecars/volumes/ServiceAccount; credential values are excluded from the binding, while the approved workload revision is included. Each request reads the current complete broker-written session profile, so normal STS renewal does not restart the receiver. Missing/stale bindings, missing sources/templates, conflicting authorities and revoked policy remain pending and block auto with retryable HTTP 503 after reconciliation. Enrollment flags can distinguish existing/new namespaces using an explicit UTC cutoff. Invalid shared JSON, document structure or selectors temporarily fail closed across enabled namespaces. A valid nonmatching selector excludes unrelated invalid policy payloads. New controller-owned provisional records recover automatically after configuration correction or deletion only if no external approval was ever resolved; adopted, revoked and ambiguous earlier records stay fail-closed. Restoring valid external approval recovers adopted namespaces. Missing referenced sources block the matching integration.

This adapter supports brokers writing a named AWS shared-credentials file with access key, secret key and session token. The administrator trusts the provisioner’s namespace-to-principal mapping and storage grants; KFP does not infer isolation from an IRSA annotation or independently prove arbitrary external IAM policy. Enrollment configures artifact reads and receiver rollout; the provisioner still supplies identities, broker/ServiceAccount wiring and pipeline root/write permissions. Deployments owned by another controller are not taken over automatically. Existing roots with valid grants remain in place within the approved store. Switching a namespace from the bundled MinIO receiver to external S3 is a destination migration: old minio://mlpipeline/... run artifacts become unreadable through that receiver. Inventory and migrate both objects and persisted references before cutover, or postpone adoption; KFP does not copy objects, rewrite history or serve both destinations automatically. Other custom integrations retain prefix enforcement or reviewed explicit storage.

Managed rollout remains automatic: ingress policy first, receiver rollout/readiness, bound protocol requests and retryable pending states. No manual UI/controller ordering is required. Updated receivers also pin legacy requests to the approved external destination and credential source; native URI query overrides are rejected instead of silently ignored. Removing automatic approval blocks auto; revoking the underlying identity also requires action in the broker/store for old-UI or explicit-storage consumers. HTTP artifacts, archived logs and volume policies remain separate.

Operator guide includes the reusable adoption policy and explicit approval examples, file contract, readiness inspection, capacity and network-isolation requirements. The standard receiver adds one Pod per enabled namespace (10m CPU/70Mi requests; 100m CPU/500Mi limits), plus broker/mesh sidecars and rollout surge. Actual NetworkPolicy enforcement and additive-policy review remain necessary.

Review recovery

A missing readiness record preserves prefix/metadata authorization and the namespace proxy route; it never selects central UI credentials. Failed record lookup/validation remains HTTP 503 with actionable permission/configuration guidance. External reconciliation reports kubeflow-pipelines-ready. Managed recovery preserves existing, verified Namespace ownership for receiver, configuration, credential and execution-bootstrap children. It never claims orphaned or foreign resources; central record ownership stays outside the preservation path. Malformed launcher configuration keeps the prior prefix-authorized namespace-proxy behavior instead of an endless reconciliation retry; the receiver can still return an actionable configuration error. Driver-agent CA rotation is reconciled even during pending/revoked approval while populated service-account token Secrets and resource ownership are preserved.

The live fixture now exercises malformed policy correction and approval-config deletion against real artifact reads, requiring recovery with the same authorization Secret UID. It also checks intentional rejection of old bundled-store history after external cutover, recovery of adopted namespaces, and renewed cross-tenant object/list/SAR checks. Revocation and source/identity binding are unchanged.

Legacy roots and custom stores

The managed version-1 path now runs existing ownership checks for exact artifacts and its descendants after key normalization, including encoded paths, previews, raw downloads and directory routes. It retains the existing IAM grant for direct-workload compatibility; approved external version-2 storage and reviewed explicit storage still use their identity grants. Namespace-less legacy history can require object/reference migration, even for the owning user; untracked V1 objects cannot use the narrow 2.18 MLMD-backed audit exception.

Approved external buckets with dots now use the same validation predicate in configuration and serving. Namespace and volume resource-name validation remains strict. Live external acceptance uses a dotted bucket throughout historical access, rotation and isolation checks.

Stores without the controller's IAM verification API can remain pending despite pre-created Secrets. Unsupported-operation, transport and permission errors remain fail-closed; repeated-sync regressions prove credentials are preserved and no users/keys or policies are created. The guides require selecting key-prefix before rollout or reviewed explicit storage with genuinely restricted identities and a compatible receiver. Existing endpoint configuration and custom namespace provisioning remain necessary. This is controller error-path coverage, not live MinIO qualification.

Rollout errors and existing storage grants

An incompatible namespace receiver now returns a fresh plain-text 503 with X-Kfp-Artifact-Proxy-Error: receiver-upgrade-required. Its HTML, content encoding and other upstream headers are discarded; valid artifacts still stream. The message explains automatic rollout and where to investigate a persistent failure. Pending/transient states and receiver upgrades retain Retry-After: 5. Invalid or empty readiness records (including a missing data key) and Kubernetes 401/403 instead give corrective configuration/authentication/RBAC guidance without Retry-After; no Secret contents are exposed.

shared/ remains intentionally tenant-readable and tenant-writable. Retained administrator-added object-read grants can also make additional objects readable to namespace viewers, including users unable to launch Pods or read Secrets. ready verifies required grants and receiver wiring, not least privilege of every effective policy. Before upgrading, inventory actual viewers and effective inline/attached/group/bucket permissions; narrow unintended grants or select key-prefix before rollout if that audience expansion is not intended. Listing still depends on effective bucket policy. The guide and agent assessment require these checks; no grant is silently removed. Legacy managed artifacts/ remains ownership-checked.

flowchart LR
  subgraph Before
    A["Old receiver returns HTML"] --> B["503 still contains HTML"]
  end
  subgraph After
    C["Old receiver response"] --> D["Discard body and upstream headers"] --> E["503 with plain upgrade guidance"]
  end

Loading

Minimal reproducer, from frontend/server after installing its declared dependencies:

npm test -- integration-tests/artifact-storage-authorization.test.ts helpers/artifact-authorization.test.ts -t 'incompatible receiver|empty record payload|missing data key' --maxWorkers=1

With the new regression tests applied to the previous implementation, six assertions failed on each branch: fixed-length/chunked/gzip HTML leaked and empty/whitespace/missing-key records were misclassified. They pass with this fix. Additional tests check hostile upstream headers, incremental valid streaming and sanitized permanent/transient errors. The live fixture checks the real old receiver's exact plain error and a distinct read-only namespace-A viewer reading tenant-B-written shared data while being denied Pod creation, B's private data and access to B's namespace. A separate real namespace-A ListObjectsV2 must return exactly the seeded shared object. SeaweedFS 4.34 returns an empty directory marker for GET, so that response is not counted as UI listing evidence.

IPv6 endpoints and IAM recovery

Approved external IPv6 endpoints now pass the bare IPv6 host to Minio while retaining the canonical approved URL and authorization binding. Previously, http://[fd00::10]:8333 passed controller validation but failed Minio client construction, producing a misleading credential error. Real-client regressions cover HTTP/HTTPS IPv6, default/explicit ports, IPv4 and DNS destinations. Malformed JSON, invalid schema and whitespace in KFP_ARTIFACT_EXTERNAL_STORAGE_CONFIG now fail startup with a sanitized error naming that setting, without printing its contents.

The managed IAM failure policy remains fail-closed. The controller already requests reconciliation after 10 seconds when verification is pending; this is not a recovery-time guarantee. Expanded regressions start ready, inject failures in each of ListAccessKeys, GetUserPolicy and GetBucketPolicy, persist pending, then verify automatic readiness recovery with unchanged credentials and no new keys or policy writes. An old ready record does not establish current authorization; a pending record does not silently activate prefix/audit fallback. Per-request readiness reads and validation remain unchanged.

Minimal endpoint/config reproducer, from frontend/server with declared dependencies installed:

npm test -- helpers/external-artifact-storage.test.ts configs.test.ts -t 'constructs a real client|reports invalid external storage' --maxWorkers=1

The new tests produced five failures per branch before the fix (two real Minio IPv6 constructor failures and three startup-diagnostic failures), then passed after it.

flowchart LR
  subgraph Before
    A["Approved bracketed IPv6 URL"] --> B["Bracketed host sent to Minio"] --> C["InvalidEndpointError / artifact 503"]
  end
  subgraph After
    D["Same approved URL and binding"] --> E["Bare host passed only to Minio"] --> F["Client constructs successfully"]
  end

Loading

Full-cluster execution also exposed an overly broad fixture leak sentinel: a legitimate 403 message names the namespace followed by a colon. The assertion now checks the exact seeded artifact payload; a regression proves a 403 containing those bytes is still rejected. This changes the test, not runtime authorization.

Validation

Single DCO-signed commit 0d891201e rebased on a6bc4e783. Exact-head full-cluster acceptance passed 44 managed checks and 1 expected old-2.17.2-UI/V1-API skip, plus 25 external-storage checks. Source and runtime image identities agree; both suites have empty cleanup-error lists. Read-only viewer, exact shared-object listing and legacy-root checks pass before and after rotation. The real old receiver also produces the required plain-text upgrade error.

External acceptance also verifies approved dotted-bucket historical access, three real STS renewals, expired-session recovery, foreign-role and cross-tenant object/list/SAR denial, approval/source revocation and recovery, and malformed-policy correction/deletion without replacing the authorization Secret or losing managed child ownership. The old failed revisions remain failed evidence: their fixture confused namespace punctuation in a correct 403 with artifact bytes; the corrected exact-payload regression now passes in the real cluster.

Current-head hosted frontend CI passed 1,855 UI tests, 1,371 server tests, 25 managed and 15 external live tests, 57 fixture regressions, 5 production-bundle startup checks and 38 real STS sessions, with clean cleanup. Local server builds/lint, controller tests (268 master / 248 release), all 49 runtime-path hooks and current workflow-policy checks pass. Standalone master/release/guide and exact combined guide tree f850b5d591a425e004df220dfa0a193efb841d65 pass Sphinx with warnings treated as errors and zero warnings.

Stock Kind networking is used. The IPv6 regression uses the real client constructor; it does not claim live IPv6 transport qualification. IAM error recovery is controller error-path coverage, not live MinIO qualification. Candidate artifact acceptance is complete; required CI, maintainer approval and merge remain separate, as do final merged-release, populated upgrade, ingress login, external IAM and mesh/additive-policy qualification.

Merge #14883 on master, then #14884 on 2.18, then #14703. Tracking: #14421.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jeffspahr. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[github-actions]

Contributor Report for @jeffspahr

Metric	Value
Kubeflow org member	Yes
Issues opened in kubeflow/pipelines	360
Merged PRs in kubeflow/pipelines	300
GitHub account age	3470 days (created 2017-04-21)
PR comments in kubeflow/pipelines	1520

[Copilot]

🔵 Needs a closer look

The unresolved cross-namespace proxy-retargeting issue and changed authorization boundary require human security review.

1 open finding

• Bind authorization to the receiving namespace, not hostname · New

What changed in this PR

Adds opt-in storage-based artifact authorization so namespace-scoped credentials can serve existing artifact roots without migration.

Changes:

• Adds authorization-mode validation and namespace-bound proxy routing.
• Generates artifact-proxy ingress policies.
• Adds storage-isolation tests, CI coverage, and rollout documentation.

File	Description
test/​seaweedfs/​artifact_authorization_test.sh	Runs disposable storage verification.
test/​seaweedfs/​artifact_authorization_fixture.py	Provisions tenant identities and historical roots.
manifests/​kustomize/​third-party/​metacontroller/​base/​cluster-role.yaml	Grants NetworkPolicy management permissions.
manifests/​kustomize/​README.md	Documents storage-mode prerequisites.
manifests/​kustomize/​base/​installs/​multi-user/​pipelines-profile-controller/​test_sync.py	Tests ingress policies and readiness.
manifests/​kustomize/​base/​installs/​multi-user/​pipelines-profile-controller/​sync.py	Generates proxy ingress policies.
manifests/​kustomize/​base/​installs/​multi-user/​pipelines-profile-controller/​deployment.yaml	Supplies the installation namespace.
manifests/​kustomize/​base/​installs/​multi-user/​pipelines-profile-controller/​decorator-controller.yaml	Registers NetworkPolicy attachments.
frontend/​server/​integration-tests/​artifact-storage-authorization.test.ts	Tests authorization and routing safeguards.
frontend/​server/​integration-tests/​artifact-storage-authorization-live.test.ts	Tests real storage read/list isolation.
frontend/​server/​handlers/​artifacts.ts	Implements storage authorization and routing guards.
frontend/​server/​configs.ts	Adds validated authorization-mode configuration.
frontend/​server/​configs.test.ts	Tests mode defaults and prerequisites.
frontend/​server/​app.ts	Wires authorization mode into middleware.
frontend/​README.md	Explains artifact authorization choices.
docs/​operator-guides/​server-config.md	Documents the new setting.
docs/​operator-guides/​index.md	Links the rollout guide.
docs/​operator-guides/​artifact-authorization.md	Explains isolation, adoption, and rollback.
docs/​agents/​ci.md	Documents fixture scope and limitations.
CHANGELOG.md	Adds artifact compatibility guidance.
.github/​workflows/​frontend.yml	Runs the real-storage fixture.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jeffspahr]

Reran the generated artifact-proxy ingress policy on stock Kind networking: all 32 assertions passed.

Environment: Kind v0.33.0, Kubernetes v1.36.1, one Linux arm64 node, IPv4. The cluster used its default kindnet and kube-proxy; no CNI replacement or disableDefaultCNI. The kindnet log confirms the built-in kube-network-policies controller started and its policy engine became ready.

kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
nodes:
- role: control-plane

Tested the actual policy produced by the profile-controller sync handler at master PR #14883 commit 02c2cfa234c2d7ef511a14f98b45664d56941a68 and 2.18 PR #14884 commit 3461fa090be5f9c1e25e4e109127e3740f4b3c80. Both emitted byte-identical policy JSON (SHA-256 bbade1fb236d3728b835cb0ff1ad0e069fd52cd060c81dac68ebb928de27420c). The policy requires the expected UI pod label and installation namespace in the same ingress peer.

Each row was checked through both the destination Pod IP on port 3000 and its Service IP on port 80:

Source	Before policy	Policy applied, then repeated	Policy removed
Shared UI in the installation namespace	Allow	Allow	Allow
Unrelated pod in the installation namespace	Allow	Deny	Allow
Tenant A pod spoofing the UI label	Allow	Deny	Allow
Tenant B pod spoofing the UI label, in the destination namespace	Allow	Deny	Allow

8 successful baseline controls + 16 allow/deny assertions across two passes + 8 successful policy-removal controls = 32 passing assertions. Denials were actual request timeouts; generic kubectl/API/exec failures did not count as isolation. The destination pod retained its UID and had zero restarts during the policy checks. Connectivity returned for every client after policy removal. The disposable cluster was removed after evidence capture.

This validates the generated standard Kubernetes NetworkPolicy's ingress boundary on default Kind networking. It uses an HTTP responder with the proxy's labels and ports; it is not a full KFP deployment, real-user SAR/ingress authentication, Istio, or multi-node/dual-stack acceptance test. The separately reported real SeaweedFS storage authorization tests remain separate evidence. Installations must still verify their own CNI, mesh, and additive policies; final release-image acceptance remains outstanding.