Skip to content

publish.yml: security fixes for explicit tags, artifact hand-off and OCI checks - #6

Merged
DeviaVir merged 5 commits into
mainfrom
security-hardening
Oct 30, 2026
Merged

DeviaVir merged 5 commits into
mainfrom
security-hardening

Conversation

@DeviaVir

@DeviaVir DeviaVir commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Security fixes after a review of #4 (released in v1.1.0) and of the repository. The diff against main contains only the fixes.

Note: v1 has the explicit-tag issue below now. A release of these fixes to v1 closes it.

Fixes to #4 (live in v1.1.0)

  • Explicit tags could imitate derived tags. Any environment can pass tags, so a run in a non-production environment could publish prod-<hex>-<timestamp> (or <hex>-<timestamp>). Derived tags get prod- only in the production environment, and consumers can rely on that. Explicit tags that start with prod- (any case) or have the <hex>-<number> form are now rejected in prepare, before any build.
  • Platform artifacts were downloaded by name. Any job in the caller's run can replace a named artifact (upload-artifact with overwrite: true) between build and publish. publish now downloads every artifact by ID, as the single-platform path already did. Each matrix leg sets its own job output, and publish checks that each ID is numeric.
  • publish could sign a digest that the artifact does not contain. It took the digest from index.json and did not check that the manifest, config and layers are in the archive. If every explicit tag already pointed to that digest, nothing was pushed and the digest was still signed. Both paths now require every blob that the image references to be in the archive, with the declared size, before authentication.
  • Ambiguous OCI JSON. crane decodes with Go, which matches keys case-insensitively. So a manifest could show layers to the checks and Layers to crane, or an index blob could pose as a manifest. Duplicate keys and keys that fold to a known field without an exact match are rejected, and the manifest blob must match its descriptor.
  • A tag name could turn a registry error into "tag absent". The error parser read the whole crane error line, which contains the tag. A tag named MANIFEST_UNKNOWN made a 403 or 503 count as absent. The parser now removes the reference, the URL and quoted text first.
  • Reads were not bounded. Manifest and config blobs are read only when the file on disk matches the declared size, up to 4 MiB. index.json and oci-layout are limited to 1 MiB.

Repository hardening

  • build-environment must be a valid name and must differ from environment (case-insensitive), so caller code in build never gets the publish environment's secrets. No current caller sets it.
  • publish holds the OIDC token. It no longer runs a third-party installer action: crane is downloaded with curl and checked against the same pinned release checksum.
  • build pins the BuildKit image by version and digest instead of the moving default, because BuildKit produces the image that publish signs.
  • lint.yml: actionlint 1.7.12 pinned by digest, with no network and a read-only mount. zizmor 1.31.1 (from 1.5.2), which now also audits dependabot.yml.
  • Dependabot: 7-day cooldown, so a new action release ages before it reaches this workflow.
  • README: documents the tag rule, and recommends --certificate-github-workflow-ref for consumers that must accept only one branch.

Validation

  • actionlint 1.7.12: clean. zizmor 1.31.1 --min-severity low .github: no findings.
  • The changed embedded scripts were extracted and run locally:
    • The tag rule rejects prod-abc-1, PROD-x, deadbeef-20261030101010 and 2026-10. It accepts v1.2.3, latest, 1.2, sha-abc, abc123 and a 40-character commit SHA.
    • The artifact ID checks reject a missing or non-numeric ID for each platform path.
    • The crane installer installs v0.22.1 with the pinned checksum and fails on a wrong checksum.
    • The unpack and merge step accepts a real BuildKit linux/amd64 archive on both paths. It rejects an archive without the manifest, the same real archive with one layer removed, a blob with a false declared size, and an oversized index.json.
    • Crafted layers/Layers, duplicate layers, an index blob posing as a manifest (two variants), ſize/Size and URLs keys are rejected on both paths; annotation keys that differ only by case are accepted.
    • The tag check reports "absent" for a real MANIFEST_UNKNOWN answer, also for tags named MANIFEST_UNKNOWN or NAME_UNKNOWN. It fails on 403, 503 and DENIED.
  • CodeMender (gemini-4-argon, about 800k tokens) on the original publish.yml: explicit tags, native multi-arch builds, masked identity secrets #4 tree reported five HIGH findings. Four are the first four fixes above. The fifth (secrets in build) needs the caller to attack its own workflow; the build-environment guard covers the useful part of it.
  • Known limit (unchanged, documented): the explicit-tag immutability check and the write are not atomic. Registry-side tag immutability is the real control.
  • Live run 41053766093 from a temporary branch at 278947e (since deleted) called this publish.yml with a fake identity: one amd64+arm64 call and one single-platform call.
    • Both native builds ran. publish received each matrix job's artifact ID and downloaded each artifact by its ID.
    • The unpack and merge step accepted the real archives, with the platform checks.
    • crane installed with the checksum and cosign installed.
    • GitHub issued the OIDC token. The token exchange then failed as expected (invalid_target, because the provider does not exist).
  • Not verified yet: a real token exchange, crane push (including --index to Artifact Registry) and cosign signing. These steps did not change in this PR, but the --index push from publish.yml: explicit tags, native multi-arch builds, masked identity secrets #4 has not run against Artifact Registry.

… secrets

- tags / moving-tags: publish exactly these tags instead of the derived
  <sha>-<timestamp> tag. Tags are immutable unless listed in moving-tags;
  an existing tag with a different digest fails before any push.
- platforms: build linux/amd64 and linux/arm64 on native runners, one
  matrix job each, merge the archives into one OCI index and sign it once.
- build-args: literal KEY=VALUE build arguments.
- workload-identity-provider / service-account / image may come from
  secrets, so public repositories do not print them in workflow logs.
  image-name appends a plain name to a secret image prefix.

Without the new inputs the workflow behaves exactly as before.
- Reject explicit tags that start with prod- or have the derived
  <hex>-<number> form, so only derived tags look derived.
- Download every build artifact by ID; any job in the run can replace
  a named artifact.
- Require every blob an image references to be in the artifact before
  authentication, so publish cannot sign a digest it did not receive.
- Remove the reference, URL and quoted text from registry errors before
  reading error codes; the tag name can appear in all of them.
- Bound manifest, config and layout metadata reads.
- Install crane with curl and the pinned checksum instead of a
  third-party action in the job that holds the OIDC token.
- Pin the BuildKit image by digest.
- lint: actionlint 1.7.12 by digest without network, zizmor 1.31.1 on
  all of .github. Dependabot: 7-day cooldown.
@DeviaVir DeviaVir self-assigned this Oct 30, 2026
@DeviaVir DeviaVir changed the title publish.yml: #4 plus security fixes (explicit tags, artifact hand-off, OCI checks) publish.yml: security fixes for explicit tags, artifact hand-off and OCI checks Oct 30, 2026
build-environment must be a valid name and must differ from
environment, so the build job never receives the publish
environment's secrets.
@DeviaVir
DeviaVir requested a balanced review from Copilot October 30, 2026 11:46
@DeviaVir
DeviaVir marked this pull request as ready for review October 30, 2026 11:47
@DeviaVir
DeviaVir requested a review from a team as a code owner October 30, 2026 11:47

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

Manifest validation accepts structural-key collisions and mislabeled index blobs, allowing the workflow to skip pushing and sign digests with missing layers or child images.

2 open issues
What changed in this PR

This PR hardens the reusable image-publishing workflow’s build-to-publish boundary and artifact validation before signing.

Changes:

  • Restricts explicit tags and build environments, and downloads platform artifacts by ID.
  • Checks referenced OCI blobs and read sizes, strengthens registry-error parsing, and pins BuildKit and crane installation.
  • Hardens lint execution, adds a Dependabot cooldown, and documents tag restrictions and branch verification.
File Description
README.md Documents tag restrictions, environment separation, and branch verification.
.github/​workflows/​publish.yml Hardens artifact hand-off, OCI validation, input checks, and tool installation.
.github/​workflows/​lint.yml Pins and isolates actionlint; upgrades zizmor and expands its scope.
.github/​dependabot.yml Adds a seven-day cooldown for action updates.

Review effort: Balanced

Comment thread .github/workflows/publish.yml Resolved
Comment thread .github/workflows/publish.yml Resolved
crane decodes OCI JSON with Go, which matches keys case-insensitively,
so a manifest could show one layer list to the checks and another to
crane. Reject duplicate keys and keys that fold to a known field without
matching it exactly, in index.json, manifests, configs and descriptors.
Annotation and label maps keep their keys as they are.

The manifest blob must also be what its descriptor says: schema version
2, the same media type, and no manifests list.

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 security-sensitive artifact hand-off lacks a live caller run validating matrix-output aggregation, artifact-ID downloads on both native architectures, and subsequent OIDC authentication and signing.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@DeviaVir
DeviaVir merged commit e56e327 into main Oct 30, 2026
2 checks passed
@DeviaVir
DeviaVir deleted the security-hardening branch October 30, 2026 12:16
DeviaVir added a commit that referenced this pull request Oct 30, 2026
publish.yml: security fixes for explicit tags, artifact hand-off and OCI checks
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