Skip to content

Put the image digest inside the attested compose identity - #43

Merged
2xburnt merged 4 commits into
mainfrom
work/2xburnt/phala-digest-render-20260910T201919Z
Sep 10, 2026
Merged

2xburnt merged 4 commits into
mainfrom
work/2xburnt/phala-digest-render-20260910T201919Z

Conversation

@2xburnt

@2xburnt 2xburnt commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Part of ENG-1968

dstack hashes the compose text it is handed, and that hash is the only part of the attested identity that says anything about the application — the verifier's platform measurement covers the base dstack OS image and nothing more.

Delivering the image as a CVM environment variable left that hash binding the topology and not the bytes. Anyone able to repoint the commit-addressed GHCR tag — a compromised Actions token, registry-side compromise, an insider — could have a CVM run different code on its next pull while every attested value stayed unchanged and every allowlist check still passed. Artifacts would then carry a valid signature over attacker-controlled output.

What changed

The image is substituted into the compose file before the deploy, by digest rather than by the commit tag, since a tag can be repointed after the fact. The digest comes from the push itself (docker/build-push-action already reports it; the step just had no id). It no longer travels in the environment file — two sources for one fact is how they drift apart.

image.composeVariable changes meaning: a render-time placeholder, not a runtime environment variable. The policy schema is untouched, so no consumer policy has to change.

A compose file may spell the placeholder ${VAR} or ${VAR:?message}. envsubst silently ignores the second form, which is why this substitutes explicitly rather than shelling out — an unexpanded placeholder reaching dstack would be measured as a literal and pull nothing. A compose file referencing neither form fails the deploy, as does one where the variable name survives substitution.

The intended consequence

The compose hash now changes on every deploy, so relying-party allowlists are re-pinned per deploy. That is what pinning means. The previous stability came from measuring nothing that varied.

Verification

pnpm run check — Prettier, 85 tests, actionlint — passes.

The substitution logic was exercised directly against four cases before wiring it in: the ${VAR:?message} form and the bare ${VAR} form both render to …@sha256:…; a compose file with no placeholder exits 1 with does not reference ${APP_IMAGE}; a push reporting a malformed digest exits 1 before anything deploys.

A new test asserts the digest comes from steps.build.outputs.digest, that the reference is built as repository@digest against a sha256:[0-9a-f]{64} guard, that both failure paths are present, that the deploy consumes steps.compose.outputs.file rather than the policy path, and that the image is gone from the environment-file writer.

Follow-on

Needs a v1.6.0 release, then burnt-labs/satya-tee-attest#6 re-pins and drops the section of its deployment README explaining what the attestation no longer covers. This also resolves ENG-1640, which asked for digest pinning in that compose file and looked incompatible with the central flow.

dstack hashes the compose text it is handed, and that hash is the only
part of the attested identity that says anything about the application —
the platform measurement covers the base dstack OS image and nothing
more. Delivering the image as a CVM environment variable left that hash
binding the topology and not the bytes: anyone able to repoint the GHCR
tag could have a CVM run different code under an unchanged attested
identity, and every allowlist check would still pass.

The image is now substituted into the compose file before the deploy,
by digest rather than by the commit tag, since a tag can be repointed
after the fact. The digest comes from the push itself. It no longer
travels in the environment file: two sources for one fact is how they
drift apart.

A compose file may spell the placeholder ${VAR} or ${VAR:?message}. One
that references neither fails the deploy, as does one where the variable
name survives substitution — an unexpanded placeholder reaching dstack
would be measured as a literal and pull nothing.

The intended consequence is that the compose hash now changes on every
deploy, so relying-party allowlists are re-pinned per deploy. That is
what pinning means; the previous stability came from measuring nothing
that varied.
Policy-tool refs name the commit the change landed in. The flow callers
move next, to this commit, since a uses: has to name a revision where
the called workflow is itself correctly pinned.
Same reason as v1.5.0: a caller pinned at the commit that changed a
workflow still resolves that workflow scripts from the previous release.
The callers name the pin advance; the policy-tool refs name where the
scripts landed.
Copilot AI lite review requested due to automatic review settings September 10, 2026 20:21
@2xburnt
2xburnt marked this pull request as ready for review September 10, 2026 20:22
@2xburnt
2xburnt requested a review from a team September 10, 2026 20:22
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-10T20:27:07.999225Z eb84860 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The updated compose-placeholder documentation doesn’t fully match the workflow’s accepted placeholder syntax, and the new test can fail with a TypeError instead of a clear assertion.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the Phala deploy workflow’s attested identity by rendering the compose file with an image digest (not a mutable tag), so dstack’s measured compose hash binds to the exact image bytes being deployed. It also updates internal workflow pins to v1.6.0 and adds regression coverage plus documentation describing the new behavior.

Changes:

  • Render the Phala compose file before deploy, substituting image.composeVariable with ghcr.io/...@sha256:... from docker/build-push-action’s digest output.
  • Remove the image reference from the CVM environment-file writer to avoid dual sources of truth.
  • Update workflow refs/pins to v1.6.0 and add a test asserting the digest-rendering and wiring.
File summaries
File Description
tests/workflows.test.mjs Adds assertions covering digest-based compose rendering and removal of image env propagation.
README.md Documents that image.composeVariable is substituted into compose by digest before deploy.
AGENTS.md Updates platform guidance to explain digest-based compose substitution and its attestation implications.
.github/workflows/required-quality.yml Advances central policy-tools checkout ref to v1.6.0.
.github/workflows/phala-deploy.yml Implements digest substitution into a rendered compose file and deploys that rendered file.
.github/workflows/npm-release.yml Advances called workflow pins to v1.6.0.
.github/workflows/npm-pr.yml Advances called workflow pins to v1.6.0.
.github/workflows/npm-main.yml Advances called workflow pins to v1.6.0 and updates central checkout ref to v1.6.0.
.github/workflows/npm-changesets.yml Advances called workflow pins to v1.6.0.
.github/workflows/cloudflare-release.yml Advances called workflow pins to v1.6.0.
.github/workflows/cloudflare-pr.yml Advances called workflow pins to v1.6.0.
.github/workflows/cloudflare-main.yml Advances called workflow pins to v1.6.0 and updates central checkout ref to v1.6.0.
Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 3
  • Review effort level: Lite

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

Comment thread .github/workflows/phala-deploy.yml Outdated
Comment thread AGENTS.md Outdated
Comment thread tests/workflows.test.mjs
The substitution regex accepted ${VAR:-fallback} alongside the two forms
the docs named. That form has no business in a compose file whose hash
is the attested identity: a default image reference is one a deploy
could silently fall back to, which is the opposite of what measuring the
file is for. It is rejected now and fails the placeholder check, and the
comment and AGENTS.md say why rather than just listing what is allowed.

The new test also asserted through a step it had not proven exists, so a
missing build step would have surfaced as a TypeError instead of a
failed assertion.
Copilot AI review requested due to automatic review settings September 10, 2026 20:32
@2xburnt
2xburnt merged commit d603eb7 into main Sep 10, 2026
3 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The new compose rendering step’s placeholder-survival check is overly broad and can incorrectly fail valid compose files that merely mention the variable name outside ${...}.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/phala-deploy.yml:198

  • rendered.includes(variable) is overly broad for detecting an unexpanded compose placeholder: it will also fail if the compose file mentions the variable name in a comment or in another identifier (e.g. APP_IMAGE_VERSION), even though the image placeholder was correctly substituted. This can cause unexpected deploy failures unrelated to digest pinning.

Instead, check specifically for unresolved ${VAR...} patterns after substitution (including unsupported forms like ${VAR:-fallback}) rather than any occurrence of the bare variable name.

  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

2 participants