Skip to content

Conversation

@dobrac
Copy link
Contributor

@dobrac dobrac commented Nov 11, 2025

Note

Add platform verification for pulled images and standardize platform handling, updating telemetry and tests accordingly.

  • OCI image pulling:
    • Introduce platform := DefaultPlatform and use it consistently in GetPublicImage and GetImage.
    • Add verifyImagePlatform(img, platform) to validate image architecture after pull (both remote and artifact registry paths).
    • Update telemetry message when using DockerHub proxy.
  • New helper:
    • Add verifyImagePlatform(img, platform) in packages/orchestrator/internal/template/build/core/oci/oci.go to enforce architecture match.
  • Tests:
    • In oci_test.go, set image config Architecture and OS to match expected platform before testing.

Written by Cursor Bugbot for commit 66c32bb. This will update automatically on new commits. Configure here.

@dobrac dobrac added the improvement Improvement for current functionality label Nov 11, 2025
@jakubno jakubno assigned jakubno and unassigned sitole Nov 11, 2025
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Architecture Validation Inconsistency

The early return path in GetPublicImage when using dockerhubRepository.GetImage() bypasses the new verifyImagePlatform check, allowing images with incorrect architectures to be returned. This inconsistency means images from the default registry without auth won't have their architecture validated, while all other images will.

packages/orchestrator/internal/template/build/core/oci/oci.go#L52-L60

if authProvider == nil && ref.Context().RegistryStr() == name.DefaultRegistry {
img, err := dockerhubRepository.GetImage(ctx, tag, DefaultPlatform)
if err != nil {
return nil, fmt.Errorf("error getting image: %w", err)
}
telemetry.ReportEvent(ctx, "pulled public image")
return img, nil

Fix in Cursor Fix in Web


@dobrac dobrac force-pushed the exit-early-on-invalid-image-architecture branch from 7f99e82 to 0174926 Compare November 11, 2025 17:05
@chatgpt-codex-connector
Copy link

💡 Codex Review

telemetry.ReportEvent(ctx, "pulled public image")
err = verifyImagePlatform(img)
if err != nil {
return nil, err

P1 Badge Enforce architecture check for DockerHub proxy path

The newly added verifyImagePlatform call only executes when the image is pulled via remote.Image, but when the function takes the default path through the DockerHub proxy (no auth provider and default registry) it still returns immediately without verification. If the cache serves a manifest for the wrong architecture—exactly what this change aims to detect—callers will continue with an incompatible image. Consider invoking verifyImagePlatform in the early return branch as well so all code paths enforce the same validation.

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dobrac dobrac enabled auto-merge (squash) November 11, 2025 17:18
@dobrac dobrac disabled auto-merge November 11, 2025 17:20
@dobrac dobrac enabled auto-merge (squash) November 11, 2025 17:21
@dobrac dobrac merged commit ee01d16 into main Nov 11, 2025
27 checks passed
@dobrac dobrac deleted the exit-early-on-invalid-image-architecture branch November 11, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement for current functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants