Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions pkg/devcontainer/config/extends.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config

import (
"context"
"encoding/json"
"fmt"
"maps"
Expand Down Expand Up @@ -43,12 +44,13 @@ func (e *ExtendsRef) UnmarshalJSON(data []byte) error {
// merging each on top of the previous result. The final merged config
// is returned as the combined parent for the declaring file.
func resolveExtendsArray(
ctx context.Context,
refs ExtendsRef, declaringDir string,
visited map[string]bool,
) (*DevContainerConfig, error) {
var merged *DevContainerConfig
for _, ref := range refs {
resolved, err := resolveExtendsSingle(ref, declaringDir, visited)
resolved, err := resolveExtendsSingle(ctx, ref, declaringDir, visited)
if err != nil {
return nil, err
}
Expand All @@ -64,11 +66,12 @@ func resolveExtendsArray(
// resolveExtendsSingle resolves a single extends reference.
// It dispatches to OCI resolution for registry refs or local file resolution otherwise.
func resolveExtendsSingle(
ctx context.Context,
extendsRef, declaringDir string,
visited map[string]bool,
) (*DevContainerConfig, error) {
if isOCIRef(extendsRef) {
return resolveOCIExtends(extendsRef, visited)
return resolveOCIExtends(ctx, extendsRef, visited)
}

refPath := extendsRef
Expand All @@ -85,11 +88,12 @@ func resolveExtendsSingle(
return nil, fmt.Errorf("extends: cycle detected, %q already in chain", absPath)
}

return parseDevContainerJSONFileWithVisited(absPath, visited)
return parseDevContainerJSONFileWithVisited(ctx, absPath, visited)
}

// parseDevContainerJSONFileWithVisited parses a devcontainer.json and recursively resolves extends.
func parseDevContainerJSONFileWithVisited(
ctx context.Context,
path string,
visited map[string]bool,
) (*DevContainerConfig, error) {
Expand Down Expand Up @@ -121,7 +125,7 @@ func parseDevContainerJSONFileWithVisited(
// Recursively resolve extends
if !devContainer.Extends.IsEmpty() {
declaringDir := filepath.Dir(absPath)
parent, err := resolveExtendsArray(devContainer.Extends, declaringDir, visited)
parent, err := resolveExtendsArray(ctx, devContainer.Extends, declaringDir, visited)
if err != nil {
return nil, err
}
Expand Down
125 changes: 111 additions & 14 deletions pkg/devcontainer/config/extends_oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,21 @@ package config
import (
"archive/tar"
"compress/gzip"
"context"
"crypto/sha256"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"os"
"path/filepath"
"strings"
"time"

pkgconfig "github.com/devsy-org/devsy/pkg/config"
"github.com/devsy-org/devsy/pkg/image"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
Expand All @@ -27,8 +34,11 @@ var ociExtendsBackoff = wait.Backoff{
}

// isOCIRef returns true if the extends reference looks like an OCI image ref
// (e.g. "ghcr.io/owner/repo:tag") rather than a local file path.
// (e.g. "ghcr.io/owner/repo:tag" or "oci://ghcr.io/owner/repo:tag") rather than a local file path.
func isOCIRef(ref string) bool {
if strings.HasPrefix(ref, "oci://") {
return true
}
if strings.HasPrefix(ref, ".") || strings.HasPrefix(ref, "/") {
return false
}
Expand All @@ -38,34 +48,41 @@ func isOCIRef(ref string) bool {
return strings.Contains(ref, "/")
}

// stripOCIPrefix removes the "oci://" prefix from an OCI reference if present.
func stripOCIPrefix(ref string) string {
return strings.TrimPrefix(ref, "oci://")
}

// resolveOCIExtends fetches a devcontainer.json from an OCI artifact and
// recursively resolves any extends within it.
func resolveOCIExtends(
ctx context.Context,
ociRef string,
visited map[string]bool,
) (*DevContainerConfig, error) {
if visited[ociRef] {
return nil, fmt.Errorf("extends: cycle detected, OCI ref %q already in chain", ociRef)
bare := stripOCIPrefix(ociRef)
if visited[bare] {
return nil, fmt.Errorf("extends: cycle detected, OCI ref %q already in chain", bare)
}
visited[ociRef] = true
visited[bare] = true

data, err := pullOCIExtendsJSON(ociRef)
data, err := pullOCIExtendsJSON(ctx, bare)
if err != nil {
return nil, fmt.Errorf("extends: fetch OCI %q: %w", ociRef, err)
return nil, fmt.Errorf("extends: fetch OCI %q: %w", bare, err)
}

devContainer := &DevContainerConfig{}
normalized, err := hujson.Standardize(data)
if err != nil {
return nil, fmt.Errorf("extends: parse jsonc from OCI %q: %w", ociRef, err)
return nil, fmt.Errorf("extends: parse jsonc from OCI %q: %w", bare, err)
}
if err := json.Unmarshal(normalized, devContainer); err != nil {
return nil, fmt.Errorf("extends: unmarshal OCI %q: %w", ociRef, err)
return nil, fmt.Errorf("extends: unmarshal OCI %q: %w", bare, err)
}
devContainer.Origin = "oci://" + ociRef
devContainer.Origin = "oci://" + bare

if !devContainer.Extends.IsEmpty() {
parent, err := resolveExtendsArray(devContainer.Extends, "", visited)
parent, err := resolveExtendsArray(ctx, devContainer.Extends, "", visited)
if err != nil {
return nil, err
}
Expand All @@ -76,24 +93,104 @@ func resolveOCIExtends(
}

// pullOCIExtendsJSON fetches an OCI image and extracts devcontainer.json
// from its first layer (expected to be a gzipped tarball).
func pullOCIExtendsJSON(ociRef string) ([]byte, error) {
// from its first layer (expected to be a gzipped tarball). Uses digest-based
// caching to avoid repeated pulls.
func pullOCIExtendsJSON(ctx context.Context, ociRef string) ([]byte, error) {
ref, err := name.ParseReference(ociRef)
if err != nil {
return nil, fmt.Errorf("parse reference: %w", err)
}

kc := getKeychain(ctx)

cacheDir, cacheErr := extendsCacheDir(ociRef)
if cacheErr == nil {
if data, ok := checkExtendsCache(cacheDir, ref, kc); ok {
return data, nil
}
}

var img v1.Image
err = retryOCIExtendsPull(func() error {
var fetchErr error
img, fetchErr = remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain))
img, fetchErr = remote.Image(ref, remote.WithAuthFromKeychain(kc))
return fetchErr
Comment on lines +108 to 117

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'remote\.(Image|Head)\(' pkg/devcontainer/config/extends_oci.go -C2
rg -n 'WithContext\(' pkg/devcontainer/config/extends_oci.go -C2

Repository: devsy-org/devsy

Length of output: 569


🏁 Script executed:

# Check the pullOCIExtendsJSON function signature to see if ctx is available
rg -n 'func pullOCIExtendsJSON' pkg/devcontainer/config/extends_oci.go -A 3

Repository: devsy-org/devsy

Length of output: 262


🏁 Script executed:

# Check all calls to checkExtendsCache and writeExtendsCache to understand context availability
rg -n '(checkExtendsCache|writeExtendsCache)\(' pkg/devcontainer/config/extends_oci.go -B 2 -A 2

Repository: devsy-org/devsy

Length of output: 757


🏁 Script executed:

# Check the complete function signatures for the helper functions mentioned
rg -n 'func (checkExtendsCache|writeExtendsCache)' pkg/devcontainer/config/extends_oci.go -A 1

Repository: devsy-org/devsy

Length of output: 390


🌐 Web query:

go-containerregistry remote.Image WithContext option

💡 Result:

Use remote.WithContext(ctx) when calling github.com/google/go-containerregistry/pkg/v1/remote.Image to control the context used for all HTTP requests that remote makes. Example: img, err:= remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain), remote.WithContext(ctx),) remote.WithContext(ctx) is a functional option that sets the context used by the remote package’s HTTP client requests, not only the initial manifest fetch. The GoDoc for WithContext notes that the provided context is used for all HTTP requests performed by the given function, and that for remote.Image it will be used for subsequent operations like RawConfigFile and for methods on layers returned by Layers. The default context is context.Background if you don’t set one [1]. So, when you cancel ctx or it times out, in-flight remote HTTP operations (including later layer/config requests) should be affected because requests are issued with req.WithContext(...) using the stored context [2].

Citations:


Add context parameter to cache helpers and propagate context to registry I/O calls.

Lines 116, 173, and 185 call remote.Image and remote.Head without context options, so caller cancellation and deadlines are not honored during OCI fetch and HEAD check operations. Pass ctx through checkExtendsCache and writeExtendsCache, and add remote.WithContext(ctx) to each registry call.

Proposed fix
-		if data, ok := checkExtendsCache(cacheDir, ref, kc); ok {
+		if data, ok := checkExtendsCache(ctx, cacheDir, ref, kc); ok {
 			return data, nil
 		}
-		img, fetchErr = remote.Image(ref, remote.WithAuthFromKeychain(kc))
+		img, fetchErr = remote.Image(ref, remote.WithAuthFromKeychain(kc), remote.WithContext(ctx))
-		writeExtendsCache(cacheDir, ref, kc, data)
+		writeExtendsCache(ctx, cacheDir, ref, kc, data)
-func checkExtendsCache(cacheDir string, ref name.Reference, kc authn.Keychain) ([]byte, bool) {
+func checkExtendsCache(ctx context.Context, cacheDir string, ref name.Reference, kc authn.Keychain) ([]byte, bool) {
-	desc, err := remote.Head(ref, remote.WithAuthFromKeychain(kc))
+	desc, err := remote.Head(ref, remote.WithAuthFromKeychain(kc), remote.WithContext(ctx))
-func writeExtendsCache(cacheDir string, ref name.Reference, kc authn.Keychain, data []byte) {
-	desc, err := remote.Head(ref, remote.WithAuthFromKeychain(kc))
+func writeExtendsCache(ctx context.Context, cacheDir string, ref name.Reference, kc authn.Keychain, data []byte) {
+	desc, err := remote.Head(ref, remote.WithAuthFromKeychain(kc), remote.WithContext(ctx))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/devcontainer/config/extends_oci.go` around lines 108 - 117, The cache
helpers and OCI calls must accept and use the request context: add a ctx
context.Context parameter to checkExtendsCache and writeExtendsCache and update
their callsites (where cacheDir, ref, kc are passed) to pass ctx; inside
retryOCIExtendsPull's body, add remote.WithContext(ctx) to remote.Image and
remote.Head invocations (in addition to remote.WithAuthFromKeychain(kc)) so
registry fetches honor cancellation/deadlines; ensure any internal calls within
checkExtendsCache/writeExtendsCache that perform registry I/O also accept and
forward ctx.

})
if err != nil {
return nil, fmt.Errorf("pull image: %w", err)
}

return extractDevContainerJSON(img)
data, err := extractDevContainerJSON(img)
if err != nil {
return nil, err
}

if cacheErr == nil {
writeExtendsCache(cacheDir, ref, kc, data)
}

return data, nil
}

func getKeychain(ctx context.Context) authn.Keychain {
kc, err := image.GetKeychain(ctx)
if err != nil {
return authn.DefaultKeychain
}
return kc
}

func extendsCacheDir(ociRef string) (string, error) {
h := sha256.Sum256([]byte(ociRef))
hashed := hex.EncodeToString(h[:])

base, err := pkgconfig.DefaultPathManager().CacheDir()
if err != nil {
return "", err
}
dir := filepath.Join(base, "extends", hashed)
if err := os.MkdirAll(dir, 0o700); err != nil {
return "", err
}
return dir, nil
}

func checkExtendsCache(cacheDir string, ref name.Reference, kc authn.Keychain) ([]byte, bool) {
jsonPath := filepath.Join(cacheDir, "devcontainer.json")
digestPath := filepath.Join(cacheDir, "digest")

// #nosec G304 -- paths derived from our own cache directory, not user input
storedDigest, err := os.ReadFile(digestPath)
if err != nil {
return nil, false
}
// #nosec G304 -- paths derived from our own cache directory, not user input
cachedJSON, err := os.ReadFile(jsonPath)
if err != nil {
return nil, false
}

desc, err := remote.Head(ref, remote.WithAuthFromKeychain(kc))
if err != nil {
return nil, false
}

if desc.Digest.String() == strings.TrimSpace(string(storedDigest)) {
return cachedJSON, true
}
return nil, false
}

func writeExtendsCache(cacheDir string, ref name.Reference, kc authn.Keychain, data []byte) {
desc, err := remote.Head(ref, remote.WithAuthFromKeychain(kc))
if err != nil {
return
}

jsonPath := filepath.Join(cacheDir, "devcontainer.json")
digestPath := filepath.Join(cacheDir, "digest")
_ = os.WriteFile(jsonPath, data, 0o600)
_ = os.WriteFile(digestPath, []byte(desc.Digest.String()), 0o600)
}

// extractDevContainerJSON reads the first layer of an OCI image as a
Expand Down
Loading
Loading