feat(artifact-cas): verify uploaded content against the declared digest by javirln · Pull Request #3365 · chainloop-dev/chainloop

javirln · 2026-08-25T06:24:20Z

The Artifact CAS keys stored artifacts by their client-declared SHA256. This
change adds end-to-end content-integrity verification to the upload path so the
canonical key is only ever written from bytes CAS has confirmed hash to that
digest, across all object-store backends (S3, S3 access point, Azure Blob), in
addition to the OCI backend's existing layer-digest check.

The store now fails closed on upload: verification happens before anything is
handed to the backend.

What changed

The CAS service streams each upload to a temporary file on a local staging
volume, computes its SHA256 while receiving, and verifies it against the
declared digest. Only a verified file is handed to the backend; a mismatch is
rejected with InvalidArgument and nothing is written to the backend.
Consolidates the upload path into a single spill-then-upload flow shared by all
backends, replacing the per-backend streaming branch and the StreamingUploader
interface. CAS memory stays bounded because content lives on disk, and the
object-store SDKs read the file back in bounded parts.
Azure uploads use UploadFile; the S3 backend relies on the service-layer
verification rather than a per-object checksum precondition (a whole-object
SHA256 precondition cannot be expressed for multipart uploads).
Adds a configurable staging_dir plus a boot-time sweep that clears leftover
staging files from a previous crash.
Helm chart: mounts a dedicated staging emptyDir with a configurable path and
size limit (the container root filesystem is read-only).

Operational notes

The staging directory must be a per-pod writable volume (node-disk emptyDir —
not tmpfs, not /tmp). Size it for roughly (concurrent uploads) × max artifact
size per replica; a breach triggers pod eviction rather than a clean error.
Uploads no longer overlap the receive and backend-upload phases, so large
artifacts take longer end to end.

This contribution was produced with the assistance of Claude Code.

The Artifact CAS keys stored artifacts by their client-declared SHA256. This change adds end-to-end content-integrity verification to the upload path so the canonical key is only ever written from bytes CAS has confirmed hash to that digest, across all object-store backends (S3, S3 access point, Azure Blob), in addition to the OCI backend's existing layer-digest check. The CAS service streams each upload to a temporary file on a local staging volume, computes its SHA256 while receiving, and verifies it against the declared digest before handing the verified file to the backend. A mismatch is rejected with InvalidArgument and nothing is written to the backend. The upload path is consolidated into a single spill-then-upload flow shared by all backends, replacing the per-backend streaming branch and the StreamingUploader interface. A configurable staging directory (optional; defaults to the OS temp dir) and a boot-time sweep of leftover staging files are added, along with a dedicated, opt-out staging emptyDir in the Helm chart. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 5d0fc605-25b0-4bb3-900c-2c2a8fb4922f

chainloop-platform · 2026-08-25T06:24:29Z

AI Session Checks — ⚠️ 1 session(s) missing

Missing AI Coding Sessions

We detected commits in this PR that were AI-assisted, but the matching Chainloop Trace session(s) could not be found in Chainloop.

Please make sure the AI coding session evidence has been sent by the Chainloop CLI, or add the skip-ai-session label to this PR to bypass this check.

Learn more about Chainloop Trace.

Powered by Chainloop and Chainloop Trace

cubic-dev-ai

4 issues found across 25 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="deployment/chainloop/templates/cas/deployment.yaml">
<violation number="1" location="deployment/chainloop/templates/cas/deployment.yaml:133">
P2: When `cas.staging.mountPath` is set to `/tmp`, Kubernetes rejects the Pod because it duplicates the existing `jwt-public-key` mount at `/tmp`. Reject this value during Helm rendering instead of relying only on the values-file comment.</violation>
</file>
<file name="app/artifact-cas/cmd/main.go">
<violation number="1" location="app/artifact-cas/cmd/main.go:174">
P2: When the configured or fallback staging directory exists but is not writable, `MkdirAll` succeeds and startup continues, so every upload later fails in `os.CreateTemp`. Probe-create and remove a staging file during startup, then fail before serving traffic when that check fails.</violation>
</file>
<file name="deployment/chainloop/values.yaml">
<violation number="1" location="deployment/chainloop/values.yaml:1483">
P1: When `cas.staging.enabled` is set to `false`, the documented fallback cannot write uploads because this chart always mounts the JWT Secret at `/tmp` as read-only. Keep staging mandatory or provide and validate a writable fallback path before advertising this mode.</violation>
</file>
<file name="deployment/chainloop/README.md">
<violation number="1" location="deployment/chainloop/README.md:887">
P2: The staging emptyDir is used only for upload temp files, but this entry tells operators to budget concurrent downloads too. Remove downloads from the sizing formula so operators do not over-size the staging volume based on traffic that does not use it.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

cubic-dev-ai · 2026-08-25T06:35:19Z

+ ## unverified ever reaches the backend. The container root filesystem is
+ ## read-only, so a writable emptyDir is mounted here.
+ ##
+ ## Optional: when disabled, the CAS falls back to the OS temporary directory.


P1: When cas.staging.enabled is set to false, the documented fallback cannot write uploads because this chart always mounts the JWT Secret at /tmp as read-only. Keep staging mandatory or provide and validate a writable fallback path before advertising this mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At deployment/chainloop/values.yaml, line 1483: <comment>When `cas.staging.enabled` is set to `false`, the documented fallback cannot write uploads because this chart always mounts the JWT Secret at `/tmp` as read-only. Keep staging mandatory or provide and validate a writable fallback path before advertising this mode.</comment> <file context> @@ -1473,7 +1473,34 @@ cas: + ## unverified ever reaches the backend. The container root filesystem is + ## read-only, so a writable emptyDir is mounted here. + ## + ## Optional: when disabled, the CAS falls back to the OS temporary directory. + ## Keep it enabled while readOnlyRootFilesystem is true, since neither the root + ## filesystem nor the /tmp secret mount are writable in that case. </file context>

cubic-dev-ai · 2026-08-25T06:35:19Z

+ # Writable scratch volume for staging + verifying uploads before they
+ # reach the backend (the container root filesystem is read-only).
+ - name: staging
+ mountPath: {{ .Values.cas.staging.mountPath | quote }}


P2: When cas.staging.mountPath is set to /tmp, Kubernetes rejects the Pod because it duplicates the existing jwt-public-key mount at /tmp. Reject this value during Helm rendering instead of relying only on the values-file comment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At deployment/chainloop/templates/cas/deployment.yaml, line 133: <comment>When `cas.staging.mountPath` is set to `/tmp`, Kubernetes rejects the Pod because it duplicates the existing `jwt-public-key` mount at `/tmp`. Reject this value during Helm rendering instead of relying only on the values-file comment.</comment> <file context> @@ -126,6 +126,12 @@ spec: + # Writable scratch volume for staging + verifying uploads before they + # reach the backend (the container root filesystem is read-only). + - name: staging + mountPath: {{ .Values.cas.staging.mountPath | quote }} + {{- end }} {{- if eq "gcpSecretManager" .Values.secretsBackend.backend }} </file context>
Suggested change
mountPath: {{ .Values.cas.staging.mountPath | quote }}
mountPath: {{ if eq .Values.cas.staging.mountPath "/tmp" }}{{ fail "cas.staging.mountPath must not be /tmp" }}{{ end }}{{ .Values.cas.staging.mountPath | quote }}

this is a good mount, we might need another one, i.e /tmp-staging-fs

cubic-dev-ai · 2026-08-25T06:35:19Z

+ _ = logger.Log(log.LevelWarn, "msg", "staging_dir not configured, falling back to OS temp dir (dev only)", "dir", dir)
+	}
+
+	if err := os.MkdirAll(dir, 0o700); err != nil {


P2: When the configured or fallback staging directory exists but is not writable, MkdirAll succeeds and startup continues, so every upload later fails in os.CreateTemp. Probe-create and remove a staging file during startup, then fail before serving traffic when that check fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/artifact-cas/cmd/main.go, line 174: <comment>When the configured or fallback staging directory exists but is not writable, `MkdirAll` succeeds and startup continues, so every upload later fails in `os.CreateTemp`. Probe-create and remove a staging file during startup, then fail before serving traffic when that check fails.</comment> <file context> @@ -152,6 +159,31 @@ func newProtoValidator() (protovalidate.Validator, error) { + _ = logger.Log(log.LevelWarn, "msg", "staging_dir not configured, falling back to OS temp dir (dev only)", "dir", dir) + } + + if err := os.MkdirAll(dir, 0o700); err != nil { + return err + } </file context>

cubic-dev-ai · 2026-08-25T06:35:19Z

 | `cas.containerSecurityContext.seccompProfile.type` | Set seccomp profile in cas container | `RuntimeDefault` |
+| `cas.staging.enabled` | Mount a dedicated emptyDir for upload staging and point staging_dir at it. Required when readOnlyRootFilesystem is true. | `true` |
+| `cas.staging.mountPath` | Directory where uploads are staged and verified. Must not be /tmp (used by the jwt-public-key secret mount). | `/staging` |
+| `cas.staging.sizeLimit` | Size limit for the staging emptyDir. Budget roughly (concurrent uploads + downloads) × max artifact size per replica. NOTE: a breach triggers kubelet POD EVICTION, not a clean error, so keep this generous and rely on per-request size caps. | `10Gi` |


P2: The staging emptyDir is used only for upload temp files, but this entry tells operators to budget concurrent downloads too. Remove downloads from the sizing formula so operators do not over-size the staging volume based on traffic that does not use it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At deployment/chainloop/README.md, line 887: <comment>The staging emptyDir is used only for upload temp files, but this entry tells operators to budget concurrent downloads too. Remove downloads from the sizing formula so operators do not over-size the staging volume based on traffic that does not use it.</comment> <file context> @@ -882,6 +882,9 @@ Once done, you can access with [two predefined users](https://github.com/chainlo | `cas.containerSecurityContext.seccompProfile.type` | Set seccomp profile in cas container | `RuntimeDefault` | +| `cas.staging.enabled` | Mount a dedicated emptyDir for upload staging and point staging_dir at it. Required when readOnlyRootFilesystem is true. | `true` | +| `cas.staging.mountPath` | Directory where uploads are staged and verified. Must not be /tmp (used by the jwt-public-key secret mount). | `/staging` | +| `cas.staging.sizeLimit` | Size limit for the staging emptyDir. Budget roughly (concurrent uploads + downloads) × max artifact size per replica. NOTE: a breach triggers kubelet POD EVICTION, not a clean error, so keep this generous and rely on per-request size caps. | `10Gi` | | `cas.automountServiceAccountToken` | Mount Service Account token in cas pods | `false` | | `cas.hostAliases` | cas pods host aliases | `[]` | </file context>
Suggested change
|`cas.staging.sizeLimit`| Size limit for the staging emptyDir. Budget roughly (concurrent uploads + downloads) × max artifact size per replica. NOTE: a breach triggers kubelet POD EVICTION, not a clean error, so keep this generous and rely on per-request size caps. |`10Gi`|
|`cas.staging.sizeLimit`| Size limit for the staging emptyDir. Budget roughly (concurrent uploads) × max artifact size per replica. NOTE: a breach triggers kubelet POD EVICTION, not a clean error, so keep this generous and rely on per-request size caps. |`10Gi`|

migmartri

In general it looks good to me, see my comments, some thoughts though

I'd make this mode to be the only mode, not configurable, if it's going to be tmpDir that's ok
What happened to the streaming capabilities you added in the other PR, why is it no longer needed? can we confirm that during the end to end input->stagingFS->blobFS we never load the whole file in memory?

migmartri · 2026-08-26T10:00:11Z

@@ -0,0 +1,61 @@
+//


this should probably not be in service package

migmartri · 2026-08-26T10:00:34Z

 {{- end }}
 data:
 server.yaml: |
+ {{- if .Values.cas.staging.enabled }}


what should this be configurable? I'd say this is mandatory and the only mechanism

migmartri · 2026-08-26T10:03:25Z

+	// IMPORTANT: hand the *os.File to Upload unwrapped. Wrapping it (io.TeeReader,
+	// io.LimitReader, a progress reader) hides io.ReaderAt/io.Seeker and silently
+	// forces the object-store SDK back onto in-memory multipart buffering.
+	if err := storageBackend.Upload(ctx, f, req.resource); err != nil {


does this still keep the whole file outside of memory?

migmartri · 2026-08-26T10:04:17Z

+// via io.ReaderAt/io.Seeker rather than buffering it in memory.
+//
+// It returns the number of bytes committed. A digest mismatch is returned as a
+// *digestMismatchError; a backend Upload failure as a *backendUploadError; spill


this comment is little bit useless it would be more useful if it explains what it does

migmartri · 2026-08-26T10:06:43Z

+	dir := bc.GetStagingDir()
+	if dir == "" {
+ dir = os.TempDir()
+ _ = logger.Log(log.LevelWarn, "msg", "staging_dir not configured, falling back to OS temp dir (dev only)", "dir", dir)


fail here do not fallback

migmartri · 2026-08-26T10:07:38Z

+ return err
+	}
+
+	if _, err := service.SweepStagingDir(dir, servicelogger.ScopedHelper(logger, "staging")); err != nil {


staging is overloaded, what about staging directory

migmartri · 2026-08-26T10:08:02Z

+ return err
+	}
+
+	if _, err := service.SweepStagingDir(dir, servicelogger.ScopedHelper(logger, "staging")); err != nil {


why are you sweeping on boot that might cause problems booting, I'd keep startup process as limited as possible

migmartri · 2026-08-26T10:11:03Z

-// streaming reader. The AWS SDK's manager.Uploader consumes the reader in
-// bounded-size parts (multipart upload), so CAS never needs to buffer the whole
-// artifact in memory.
-func (b *Backend) SupportsStreaming() bool { return true }


what changed that now this has been removed?

migmartri · 2026-08-26T10:12:50Z

+ {{- if .Values.cas.staging.enabled }}
+ # Writable scratch volume for staging + verifying uploads before they
+ # reach the backend (the container root filesystem is read-only).
+ - name: staging


can we just make it /tmp?

migmartri · 2026-08-26T10:13:14Z

+ # Node-disk (NOT tmpfs/RAM) scratch space for staging + verifying uploads.
+ - name: staging
+ emptyDir:
+ sizeLimit: {{ .Values.cas.staging.sizeLimit }}


why configure this? what happens if it goes over?

cubic-dev-aiBot reviewed Aug 25, 2026
View reviewed changes

migmartri reviewed Aug 26, 2026
View reviewed changes

	mountPath: {{ .Values.cas.staging.mountPath \| quote }}
	mountPath: {{ if eq .Values.cas.staging.mountPath "/tmp" }}{{ fail "cas.staging.mountPath must not be /tmp" }}{{ end }}{{ .Values.cas.staging.mountPath \| quote }}

	\|`cas.staging.sizeLimit`\| Size limit for the staging emptyDir. Budget roughly (concurrent uploads + downloads) × max artifact size per replica. NOTE: a breach triggers kubelet POD EVICTION, not a clean error, so keep this generous and rely on per-request size caps. \|`10Gi`\|
	\|`cas.staging.sizeLimit`\| Size limit for the staging emptyDir. Budget roughly (concurrent uploads) × max artifact size per replica. NOTE: a breach triggers kubelet POD EVICTION, not a clean error, so keep this generous and rely on per-request size caps. \|`10Gi`\|

Uh oh!

Conversation

javirln commented Aug 25, 2026• edited by cubic-dev-ai BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Operational notes

Uh oh!

chainloop-platformBot commented Aug 25, 2026

AI Session Checks — ⚠️ 1 session(s) missing

Missing AI Coding Sessions

Uh oh!

cubic-dev-aiBot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-aiBotAug 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-aiBotAug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-aiBotAug 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-aiBotAug 25, 2026

Choose a reason for hiding this comment

Uh oh!

migmartri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

javirln commented Aug 25, 2026•
edited by cubic-dev-ai Bot
Loading