From 286828d31be271f7ab23f6f73b054608ed0eae64 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 22 Jul 2026 22:36:52 -0500 Subject: [PATCH 1/6] feat(devcontainer): add devcontainer-lock.json lockfile support Adds genuine lockfile support to the native Go feature resolver, matching the reference devcontainer CLI: - Generates/updates .devcontainer-lock.json (or devcontainer-lock.json) next to the config during build/up, pinning OCI features by manifest digest and direct-tarball features by sha256, with a version/resolved/integrity entry per feature (dependencies included). - Loads an existing lockfile to pin resolution and verify integrity. - Adds --frozen-lockfile (build and up) which fails instead of writing when the lockfile is missing or does not match, with an early fail-fast check. Includes unit tests for the lockfile logic and e2e coverage under the up-features label (auto-write + frozen reuse, and frozen-missing failure). --- cmd/workspace/build.go | 3 + cmd/workspace/up/up_flags.go | 3 + .../.devcontainer.json | 8 + e2e/tests/up-features/up_features.go | 62 ++++ pkg/devcontainer/build.go | 3 + pkg/devcontainer/compose.go | 4 + pkg/devcontainer/compose_build.go | 1 + pkg/devcontainer/feature/extend.go | 128 ++++++- pkg/devcontainer/feature/features.go | 228 +++++++++--- pkg/devcontainer/feature/features_oci_test.go | 8 +- pkg/devcontainer/feature/lockfile.go | 232 ++++++++++++ pkg/devcontainer/feature/lockfile_test.go | 333 ++++++++++++++++++ pkg/flags/names/names.go | 1 + pkg/provider/workspace.go | 4 + 14 files changed, 968 insertions(+), 50 deletions(-) create mode 100644 e2e/tests/up-features/testdata/docker-features-lockfile/.devcontainer.json create mode 100644 pkg/devcontainer/feature/lockfile.go create mode 100644 pkg/devcontainer/feature/lockfile_test.go diff --git a/cmd/workspace/build.go b/cmd/workspace/build.go index b59802212..dc50f605b 100644 --- a/cmd/workspace/build.go +++ b/cmd/workspace/build.go @@ -109,6 +109,9 @@ func (cmd *BuildCmd) registerImageFlags(buildCmd *cobra.Command) { cliflags.String(&cmd.Output, names.Output, "", "Build output type (docker or oci)"), cliflags.String(&cmd.ExperimentalLockfile, names.ExperimentalLockfile, "", "Lockfile path for reproducible builds"), + cliflags.Bool(&cmd.FrozenLockfile, names.FrozenLockfile, false, + "Fail if devcontainer-lock.json is missing or does not match the resolved features "+ + "instead of writing it (useful for CI)"), cliflags.String( &cmd.ImageName, names.ImageName, diff --git a/cmd/workspace/up/up_flags.go b/cmd/workspace/up/up_flags.go index a8dbd2751..abbd7457a 100644 --- a/cmd/workspace/up/up_flags.go +++ b/cmd/workspace/up/up_flags.go @@ -70,6 +70,9 @@ func (cmd *UpCmd) registerBuildFlags(upCmd *cobra.Command) { "Default for updateRemoteUserUID when unset in the config (on, off)"), flags.Bool(cmd.MountWorkspaceGitRoot, names.MountWorkspaceGitRoot, true, "Mount the workspace git root as the workspace folder"), + flags.Bool(&cmd.FrozenLockfile, names.FrozenLockfile, false, + "Fail if devcontainer-lock.json is missing or does not match the resolved features "+ + "instead of writing it (useful for CI)"), ) flags.RegisterDevContainerModifierFlags(upCmd.Flags(), flags.DevContainerModifierFlags{ Image: &cmd.DevContainerImage, diff --git a/e2e/tests/up-features/testdata/docker-features-lockfile/.devcontainer.json b/e2e/tests/up-features/testdata/docker-features-lockfile/.devcontainer.json new file mode 100644 index 000000000..025339540 --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-lockfile/.devcontainer.json @@ -0,0 +1,8 @@ +{ + "name": "Test Lockfile", + "image": "ghcr.io/devsy-org/test-images/base:ubuntu", + "features": { + "ghcr.io/devcontainers/features/git:1": {} + }, + "postStartCommand": "git version" +} diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index 64f712933..2b69ea6fb 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/devsy-org/devsy/e2e/framework" + "github.com/devsy-org/devsy/pkg/flags/names" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" "github.com/onsi/gomega/ghttp" @@ -532,6 +533,67 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( ginkgo.SpecTimeout(framework.TimeoutLong()), ) + ginkgo.It( + "writes a devcontainer lockfile and reuses it under --frozen-lockfile", + ginkgo.Label("up-features", "lockfile"), + func(ctx context.Context) { + f, err := setupDockerProvider(initialDir+"/bin", "docker") + framework.ExpectNoError(err) + + tempDir, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-lockfile", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) + + wsName := filepath.Base(tempDir) + ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) + + // First up: no lockfile exists yet, so it is generated with the + // resolved digest and integrity for the OCI feature. + err = f.DevsyUp(ctx, tempDir) + framework.ExpectNoError(err) + + // The config basename starts with a dot, so the lockfile is hidden. + lockContent, err := f.DevsySSH(ctx, wsName, "cat /workspaces/*/.devcontainer-lock.json") + framework.ExpectNoError(err) + gomega.Expect(lockContent). + To(gomega.ContainSubstring("ghcr.io/devcontainers/features/git:1")) + gomega.Expect(lockContent).To(gomega.ContainSubstring("\"resolved\"")) + gomega.Expect(lockContent).To(gomega.ContainSubstring("@sha256:")) + gomega.Expect(lockContent).To(gomega.ContainSubstring("\"integrity\"")) + + // Second up with the frozen flag must succeed: the generated + // lockfile matches the resolved features exactly. + err = f.DevsyUpRecreate(ctx, tempDir, names.Flag(names.FrozenLockfile)) + framework.ExpectNoError(err) + }, + ginkgo.SpecTimeout(framework.TimeoutLong()), + ) + + ginkgo.It( + "fails --frozen-lockfile when no lockfile exists", + ginkgo.Label("up-features", "lockfile"), + func(ctx context.Context) { + f, err := setupDockerProvider(initialDir+"/bin", "docker") + framework.ExpectNoError(err) + + tempDir, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-lockfile", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) + + wsName := filepath.Base(tempDir) + ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) + + // No lockfile is committed, so frozen mode must fail fast. + err = f.DevsyUp(ctx, tempDir, names.Flag(names.FrozenLockfile)) + framework.ExpectError(err) + }, + ginkgo.SpecTimeout(framework.TimeoutLong()), + ) + ginkgo.It("resolves user variable in dockerfile", func(ctx context.Context) { f, err := setupDockerProvider(initialDir+"/bin", "docker") framework.ExpectNoError(err) diff --git a/pkg/devcontainer/build.go b/pkg/devcontainer/build.go index d1e69f395..a644ef734 100644 --- a/pkg/devcontainer/build.go +++ b/pkg/devcontainer/build.go @@ -78,6 +78,7 @@ func (r *runner) extendImage( DevContainerConfig: parsedConfig, ForceBuild: options.ForceBuild, SecretOpts: featureSecretOpts(options), + FrozenLockfile: options.FrozenLockfile, }) if err != nil { return nil, fmt.Errorf("get extended build info: %w", err) @@ -184,6 +185,7 @@ func (r *runner) buildAndExtendImage( DevContainerConfig: parsedConfig, ForceBuild: options.ForceBuild, SecretOpts: featureSecretOpts(options), + FrozenLockfile: options.FrozenLockfile, }) if err != nil { return nil, fmt.Errorf("get extended build info: %w", err) @@ -554,6 +556,7 @@ func (r *runner) buildDevImageCompose( featureSecretsFile: options.FeatureSecretsFile, pull: options.Pull, noCache: options.NoCache, + frozenLockfile: options.FrozenLockfile, }) if err != nil { return nil, fmt.Errorf("build and extend docker-compose: %w", err) diff --git a/pkg/devcontainer/compose.go b/pkg/devcontainer/compose.go index 87ed16822..7a69866c6 100644 --- a/pkg/devcontainer/compose.go +++ b/pkg/devcontainer/compose.go @@ -81,6 +81,9 @@ type buildAndExtendParams struct { // noCache disables the build cache during the compose build (--no-cache), // set from CLIOptions.NoCache. noCache bool + // frozenLockfile enforces that the devcontainer lockfile matches the + // resolved features, set from CLIOptions.FrozenLockfile. + frozenLockfile bool } // composeUpParams groups the inputs shared by extendedDockerComposeUp and @@ -757,6 +760,7 @@ func (r *runner) buildComposeOverrideArgs( featureSecretsFile: start.options.FeatureSecretsFile, pull: start.options.Pull, noCache: start.options.NoCache, + frozenLockfile: start.options.FrozenLockfile, }) if err != nil { return nil, fmt.Errorf("build and extend docker-compose: %w", err) diff --git a/pkg/devcontainer/compose_build.go b/pkg/devcontainer/compose_build.go index 575a1f1fd..9d06a9a0f 100644 --- a/pkg/devcontainer/compose_build.go +++ b/pkg/devcontainer/compose_build.go @@ -230,6 +230,7 @@ func (r *runner) prepareExtendedComposeBuild( SecretsFile: params.featureSecretsFile, Prompter: &feature.TerminalSecretPrompter{}, }, + FrozenLockfile: params.frozenLockfile, }) if err != nil { return preparedComposeBuild{}, err diff --git a/pkg/devcontainer/feature/extend.go b/pkg/devcontainer/feature/extend.go index e75b29890..f40d6bef7 100644 --- a/pkg/devcontainer/feature/extend.go +++ b/pkg/devcontainer/feature/extend.go @@ -3,6 +3,7 @@ package feature import ( "fmt" "os" + "path" "path/filepath" "regexp" "strconv" @@ -58,6 +59,9 @@ type ExtendedBuildParams struct { DevContainerConfig *config.SubstitutedConfig ForceBuild bool SecretOpts *SecretOptions + // FrozenLockfile enforces that the devcontainer lockfile exists and matches + // the resolved features; the build fails instead of writing the lockfile. + FrozenLockfile bool } func GetExtendedBuildInfo(params *ExtendedBuildParams) (*ExtendedBuildInfo, error) { @@ -67,7 +71,9 @@ func GetExtendedBuildInfo(params *ExtendedBuildParams) (*ExtendedBuildInfo, erro devContainerConfig := params.DevContainerConfig forceBuild := params.ForceBuild secretOpts := params.SecretOpts - features, err := fetchFeatures(devContainerConfig.Config, forceBuild, secretOpts) + features, err := fetchFeatures( + devContainerConfig.Config, forceBuild, secretOpts, true, params.FrozenLockfile, + ) if err != nil { return nil, fmt.Errorf("fetch features: %w", err) } @@ -287,7 +293,7 @@ func usersFromMetadata(meta *config.ImageMetadataConfig) (string, string) { func ResolveFeatureOrder( devContainerConfig *config.DevContainerConfig, ) ([]*config.FeatureSet, error) { - return fetchFeatures(devContainerConfig, false, nil) + return fetchFeatures(devContainerConfig, false, nil, false, false) } func applyUserFallback(user, composeServiceUser, imageUser string) string { @@ -304,11 +310,21 @@ func fetchFeatures( devContainerConfig *config.DevContainerConfig, forceBuild bool, secretOpts *SecretOptions, + lockWrite bool, + lockFrozen bool, ) ([]*config.FeatureSet, error) { + lock := newLockfileState(devContainerConfig) processor := &featureProcessor{ devContainerConfig: devContainerConfig, forceBuild: forceBuild, secretOpts: secretOpts, + lock: lock, + } + + if lockWrite && lockFrozen { + if err := lock.checkFrozenPrecondition(devContainerConfig); err != nil { + return nil, err + } } userFeatures, err := getUserFeatures(processor, devContainerConfig) @@ -331,6 +347,10 @@ func fetchFeatures( return nil, fmt.Errorf("failed to get sorted feature sets: %w", err) } + if err := lock.commit(devContainerConfig, lockWrite, lockFrozen); err != nil { + return nil, err + } + return featureSets, nil } @@ -354,16 +374,94 @@ type featureProcessor struct { devContainerConfig *config.DevContainerConfig forceBuild bool secretOpts *SecretOptions + lock *lockfileState +} + +// featureKind identifies how a feature is sourced, which determines whether it +// participates in the lockfile (only OCI and direct-tarball features do). +type featureKind int + +const ( + featureKindLocal featureKind = iota + featureKindOCI + featureKindTarball +) + +// featureResolution is the outcome of resolving a feature identifier to a local +// folder, plus the lockfile metadata (resolved reference and integrity digest). +type featureResolution struct { + folder string + kind featureKind + resolved string + integrity string +} + +// resolveFeatureSource resolves a feature identifier to a local folder, pinning +// OCI and direct-tarball features to the loaded lockfile when present. +func (p *featureProcessor) resolveFeatureSource(id string) (*featureResolution, error) { + switch { + case strings.HasPrefix(id, "https://") || strings.HasPrefix(id, "http://"): + return p.resolveTarballFeature(id) + case strings.HasPrefix(id, "./") || strings.HasPrefix(id, "../"): + return p.resolveLocalFeature(id) + default: + return p.resolveOCIFeature(id) + } +} + +func (p *featureProcessor) resolveTarballFeature(id string) (*featureResolution, error) { + log.Debugf("process feature: type=%s, id=%s", "url", id) + _, pinnedIntegrity, _ := p.lock.pin(id) + headers := config.GetDevsyCustomizations(p.devContainerConfig).FeatureDownloadHTTPHeaders + folder, integrity, err := processDirectTarFeature( + id, headers, p.forceBuild, pinnedIntegrity, + ) + if err != nil { + return nil, err + } + return &featureResolution{ + folder: folder, + kind: featureKindTarball, + resolved: id, + integrity: integrity, + }, nil +} + +func (p *featureProcessor) resolveLocalFeature(id string) (*featureResolution, error) { + log.Debugf("process feature: type=%s, id=%s", "local", id) + folder, err := filepath.Abs( + path.Join(filepath.ToSlash(filepath.Dir(p.devContainerConfig.Origin)), id), + ) + if err != nil { + return nil, err + } + return &featureResolution{folder: folder, kind: featureKindLocal}, nil +} + +func (p *featureProcessor) resolveOCIFeature(id string) (*featureResolution, error) { + log.Debugf("process feature: type=%s, id=%s", "oci", id) + pinnedResolved, pinnedIntegrity, _ := p.lock.pin(id) + res, err := processOCIFeature(id, pinnedResolved, pinnedIntegrity) + if err != nil { + return nil, err + } + return &featureResolution{ + folder: res.folder, + kind: featureKindOCI, + resolved: res.resolved, + integrity: res.integrity, + }, nil } func (p *featureProcessor) processFeature( featureID string, featureOptions any, ) (*config.FeatureSet, error) { - featureFolder, err := ProcessFeatureID(featureID, p.devContainerConfig, p.forceBuild) + res, err := p.resolveFeatureSource(featureID) if err != nil { return nil, fmt.Errorf("process feature ID %s: %w", featureID, err) } + featureFolder := res.folder log.Debugf("parse dev container feature in %s", featureFolder) featureConfig, err := config.ParseDevContainerFeature(featureFolder) @@ -389,6 +487,8 @@ func (p *featureProcessor) processFeature( return nil, err } + p.recordLockEntry(featureID, res, featureConfig) + return &config.FeatureSet{ ConfigID: normalizeFeatureID(featureID), Version: extractVersionFromFeatureID(featureID), @@ -398,6 +498,28 @@ func (p *featureProcessor) processFeature( }, nil } +// recordLockEntry adds a lockfile entry for OCI and direct-tarball features, +// keyed by the feature identifier as written in the config (or a dependency's +// identifier). Local features are not lockable and are skipped. +func (p *featureProcessor) recordLockEntry( + featureID string, + res *featureResolution, + cfg *config.FeatureConfig, +) { + if res.kind != featureKindOCI && res.kind != featureKindTarball { + return + } + entry := LockedFeature{ + Version: cfg.Version, + Resolved: res.resolved, + Integrity: res.integrity, + } + if len(cfg.DependsOn) > 0 { + entry.DependsOn = map[string]any(cfg.DependsOn) + } + p.lock.record(featureID, entry) +} + func resolveSecretsForFeature( featureID string, featureCfg *config.FeatureConfig, diff --git a/pkg/devcontainer/feature/features.go b/pkg/devcontainer/feature/features.go index 92c6fec71..762b6f6db 100644 --- a/pkg/devcontainer/feature/features.go +++ b/pkg/devcontainer/feature/features.go @@ -6,7 +6,6 @@ import ( "io" "net/http" "os" - "path" "path/filepath" "regexp" "strings" @@ -126,28 +125,24 @@ func maskSecretOptions(feature *config.FeatureConfig, options []string) []string return masked } +// ProcessFeatureID resolves a feature identifier to a local folder containing +// the extracted feature, without lockfile pinning. It is retained for callers +// that only need the folder; lockfile-aware resolution goes through +// featureProcessor.resolveFeatureSource. func ProcessFeatureID( id string, devContainerConfig *config.DevContainerConfig, forceBuild bool, ) (string, error) { - if strings.HasPrefix(id, "https://") || strings.HasPrefix(id, "http://") { - log.Debugf("process feature: type=%s, id=%s", "url", id) - return processDirectTarFeature( - id, - config.GetDevsyCustomizations(devContainerConfig).FeatureDownloadHTTPHeaders, - forceBuild, - ) - } else if strings.HasPrefix(id, "./") || strings.HasPrefix(id, "../") { - log.Debugf("process feature: type=%s, id=%s", "local", id) - return filepath.Abs( - path.Join(filepath.ToSlash(filepath.Dir(devContainerConfig.Origin)), id), - ) + processor := &featureProcessor{ + devContainerConfig: devContainerConfig, + forceBuild: forceBuild, } - - // get oci feature - log.Debugf("process feature: type=%s, id=%s", "oci", id) - return processOCIFeature(id) + res, err := processor.resolveFeatureSource(id) + if err != nil { + return "", err + } + return res.folder, nil } func checkFeatureCache(id string) (string, bool) { @@ -208,11 +203,14 @@ func PullFeatureToTemp(ref name.Reference, id string) (string, error) { featureExtractedFolder := filepath.Join(featureFolder, "extracted") - annotations, err := pullAndExtractOCIFeature(ref, id, featureFolder, featureExtractedFolder) + annotations, digest, err := pullAndExtractOCIFeature( + ref, id, featureFolder, featureExtractedFolder, + ) if err != nil { return "", err } + storeOCIDigest(featureFolder, digest) if len(annotations) > 0 { logOCIAnnotations(id, annotations) saveAnnotations(featureFolder, annotations) @@ -221,31 +219,81 @@ func PullFeatureToTemp(ref name.Reference, id string) (string, error) { return featureExtractedFolder, nil } -func processOCIFeature(id string) (string, error) { +// ociResolution is the outcome of resolving an OCI feature: the extracted +// folder, the canonical digest reference, and the manifest digest (integrity). +type ociResolution struct { + folder string + resolved string + integrity string +} + +// processOCIFeature resolves an OCI feature to a local folder and returns its +// resolved digest reference and manifest digest (integrity). When pinnedResolved +// is set the feature is fetched by that pinned reference; when pinnedIntegrity is +// set the resolved manifest digest must match it. +func processOCIFeature(id, pinnedResolved, pinnedIntegrity string) (*ociResolution, error) { log.Debugf("processing OCI feature: featureId=%s", id) - if cached, ok := checkFeatureCache(id); ok { - return cached, nil + refID := id + if pinnedResolved != "" { + refID = pinnedResolved } - featureFolder, err := getFeaturesTempFolder(id) + ref, err := name.ParseReference(refID) if err != nil { - return "", fmt.Errorf("resolve feature cache dir: %w", err) + log.Debugf("failed to parse OCI reference: error=%v, featureId=%s", err, refID) + return nil, err } - featureExtractedFolder := filepath.Join(featureFolder, "extracted") + featureFolder, err := getFeaturesTempFolder(refID) + if err != nil { + return nil, fmt.Errorf("resolve feature cache dir: %w", err) + } - ref, err := name.ParseReference(id) + var res *ociResolution + if cached, ok := checkFeatureCache(refID); ok { + res, err = resolveCachedOCIFeature(cached, featureFolder, ref) + } else { + res, err = pullOCIFeature(id, refID, ref, featureFolder) + } if err != nil { - log.Debugf("failed to parse OCI reference: error=%v, featureId=%s", err, id) - return "", err + return nil, err } - annotations, err := pullAndExtractOCIFeature(ref, id, featureFolder, featureExtractedFolder) + if err := verifyPinnedDigest(id, res.integrity, pinnedIntegrity); err != nil { + return nil, err + } + return res, nil +} + +// resolveCachedOCIFeature builds the resolution for an already-cached feature. +func resolveCachedOCIFeature( + folder, featureFolder string, ref name.Reference, +) (*ociResolution, error) { + digest, err := resolveCachedOCIDigest(featureFolder, ref) if err != nil { - return "", err + return nil, err } + return &ociResolution{ + folder: folder, + resolved: ociResolvedReference(ref, digest), + integrity: digest, + }, nil +} +// pullOCIFeature pulls, extracts, and records a not-yet-cached OCI feature. +func pullOCIFeature( + id, refID string, ref name.Reference, featureFolder string, +) (*ociResolution, error) { + featureExtractedFolder := filepath.Join(featureFolder, "extracted") + annotations, digest, err := pullAndExtractOCIFeature( + ref, refID, featureFolder, featureExtractedFolder, + ) + if err != nil { + return nil, err + } + + storeOCIDigest(featureFolder, digest) if len(annotations) > 0 { logOCIAnnotations(id, annotations) saveAnnotations(featureFolder, annotations) @@ -256,7 +304,72 @@ func processOCIFeature(id string) (string, error) { id, featureExtractedFolder, ) - return featureExtractedFolder, nil + return &ociResolution{ + folder: featureExtractedFolder, + resolved: ociResolvedReference(ref, digest), + integrity: digest, + }, nil +} + +const ociDigestFileName = "oci-digest" + +// storeOCIDigest persists the resolved manifest digest next to the cached +// tarball so it can be recovered on a cache hit without re-fetching. +func storeOCIDigest(featureFolder, digest string) { + if digest == "" { + return + } + path := filepath.Join(featureFolder, ociDigestFileName) + if err := os.WriteFile(path, []byte(digest), 0o600); err != nil { + log.Debugf("failed to write OCI digest sidecar: %v", err) + } +} + +func loadOCIDigest(featureFolder string) string { + path := filepath.Join(featureFolder, ociDigestFileName) + data, err := os.ReadFile(filepath.Clean(path)) + if err != nil { + return "" + } + return strings.TrimSpace(string(data)) +} + +// resolveCachedOCIDigest returns the manifest digest for a cached feature, +// using the sidecar when present and otherwise fetching the manifest. +func resolveCachedOCIDigest(featureFolder string, ref name.Reference) (string, error) { + if digest := loadOCIDigest(featureFolder); digest != "" { + return digest, nil + } + img, err := pullOCIImage(ref) + if err != nil { + return "", err + } + digest, err := img.Digest() + if err != nil { + return "", fmt.Errorf("read manifest digest: %w", err) + } + storeOCIDigest(featureFolder, digest.String()) + return digest.String(), nil +} + +// ociResolvedReference builds the canonical digest reference (repo@digest) used +// as the lockfile "resolved" value. +func ociResolvedReference(ref name.Reference, digest string) string { + if digest == "" { + return ref.String() + } + return ref.Context().Name() + "@" + digest +} + +// verifyPinnedDigest fails when a locked digest does not match the resolved one. +func verifyPinnedDigest(id, actual, pinned string) error { + if pinned == "" || actual == pinned { + return nil + } + return fmt.Errorf( + "integrity mismatch for feature %s: lockfile has %s but resolved %s", + id, pinned, actual, + ) } const annotationsFileName = "annotations.json" @@ -308,15 +421,20 @@ func LoadOCIAnnotations(featureFolder string) map[string]string { func pullAndExtractOCIFeature( ref name.Reference, id, featureFolder, destDir string, -) (map[string]string, error) { +) (map[string]string, string, error) { img, err := pullOCIImage(ref) if err != nil { - return nil, err + return nil, "", err } manifest, err := img.Manifest() if err != nil { - return nil, fmt.Errorf("read manifest: %w", err) + return nil, "", fmt.Errorf("read manifest: %w", err) + } + + digest, err := img.Digest() + if err != nil { + return nil, "", fmt.Errorf("read manifest digest: %w", err) } destFile := filepath.Join(featureFolder, "feature.tgz") @@ -324,12 +442,12 @@ func pullAndExtractOCIFeature( err = downloadLayer(img, id, destFile) if err != nil { log.Debugf("failed to download feature layer: error=%v, featureId=%s", err, id) - return nil, fmt.Errorf("download layer from %s: %w", registry, err) + return nil, "", fmt.Errorf("download layer from %s: %w", registry, err) } file, err := os.Open(filepath.Clean(destFile)) //nolint:gosec // path from internal resolution if err != nil { - return nil, err + return nil, "", err } defer func() { _ = file.Close() }() @@ -338,10 +456,10 @@ func pullAndExtractOCIFeature( if err != nil { log.Debugf("failed to extract feature: error=%v, destination=%s", err, destDir) _ = os.RemoveAll(destDir) - return nil, err + return nil, "", err } - return manifest.Annotations, nil + return manifest.Annotations, digest.String(), nil } func validateImageManifest(img v1.Image) (*v1.Manifest, error) { @@ -484,16 +602,20 @@ func extractTarball(downloadFile, dest string) error { return nil } +// processDirectTarFeature resolves an HTTP(S) tarball feature to a local folder +// and returns its integrity digest (sha256: of the tarball). When +// pinnedIntegrity is set the computed digest must match it. func processDirectTarFeature( id string, httpHeaders map[string]string, forceDownload bool, -) (string, error) { + pinnedIntegrity string, +) (folder, integrity string, err error) { log.Debugf("processing direct tar feature: featureId=%s, forceDownload=%v", id, forceDownload) downloadBase := id[strings.LastIndex(id, "/"):] if !directTarballRegEx.MatchString(downloadBase) { - return "", fmt.Errorf( + return "", "", fmt.Errorf( "expected tarball name to follow 'devcontainer-feature-.tgz' format. Received %q ", downloadBase, ) @@ -501,7 +623,7 @@ func processDirectTarFeature( featureFolder, err := getFeaturesTempFolder(id) if err != nil { - return "", fmt.Errorf("resolve feature cache dir: %w", err) + return "", "", fmt.Errorf("resolve feature cache dir: %w", err) } featureExtractedFolder := filepath.Join(featureFolder, "extracted") @@ -511,7 +633,11 @@ func processDirectTarFeature( if statErr == nil && !forceDownload { if verifyCacheIntegrity(featureFolder, id) { log.Debugf("direct tar feature already cached: folder=%s", featureExtractedFolder) - return featureExtractedFolder, nil + cachedIntegrity := tarballIntegrity(featureFolder) + if err := verifyPinnedDigest(id, cachedIntegrity, pinnedIntegrity); err != nil { + return "", "", err + } + return featureExtractedFolder, cachedIntegrity, nil } _ = os.RemoveAll(featureFolder) } @@ -521,14 +647,18 @@ func processDirectTarFeature( err = downloadFeatureFromURL(id, downloadFile, httpHeaders) if err != nil { log.Debugf("failed to download feature tarball: error=%v, url=%s", err, id) - return "", err + return "", "", err } storeIntegrityHash(featureFolder, downloadFile, id) + downloadIntegrity := tarballIntegrity(featureFolder) + if err := verifyPinnedDigest(id, downloadIntegrity, pinnedIntegrity); err != nil { + return "", "", err + } if err := extractTarball(downloadFile, featureExtractedFolder); err != nil { log.Debugf("failed to extract tarball: error=%v, featureId=%s", err, id) - return "", err + return "", "", err } log.Infof( @@ -536,7 +666,17 @@ func processDirectTarFeature( id, featureExtractedFolder, ) - return featureExtractedFolder, nil + return featureExtractedFolder, downloadIntegrity, nil +} + +// tarballIntegrity returns the sha256: digest of a cached feature tarball, +// or "" when it cannot be computed. +func tarballIntegrity(featureFolder string) string { + h, err := hash.File(filepath.Join(featureFolder, "feature.tgz")) + if err != nil || h == "" { + return "" + } + return "sha256:" + h } func downloadFeatureFromURL( diff --git a/pkg/devcontainer/feature/features_oci_test.go b/pkg/devcontainer/feature/features_oci_test.go index 4593da0b7..05a2c7cd5 100644 --- a/pkg/devcontainer/feature/features_oci_test.go +++ b/pkg/devcontainer/feature/features_oci_test.go @@ -29,10 +29,12 @@ func (s *OCIFeatureTestSuite) TestProcessOCIFeature_HappyPath() { } }() - result, err := processOCIFeature("ghcr.io/devcontainers/features/go:1") + res, err := processOCIFeature("ghcr.io/devcontainers/features/go:1", "", "") s.Require().NoError(err) - s.DirExists(result) - s.FileExists(filepath.Join(result, "devcontainer-feature.json")) + s.DirExists(res.folder) + s.FileExists(filepath.Join(res.folder, "devcontainer-feature.json")) + s.Contains(res.resolved, "@sha256:") + s.Contains(res.integrity, "sha256:") } func (s *OCIFeatureTestSuite) TestFetchCollection_GHCR() { diff --git a/pkg/devcontainer/feature/lockfile.go b/pkg/devcontainer/feature/lockfile.go new file mode 100644 index 000000000..78a6caf96 --- /dev/null +++ b/pkg/devcontainer/feature/lockfile.go @@ -0,0 +1,232 @@ +package feature + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/log" +) + +// LockedFeature is a single pinned entry in a devcontainer-lock.json file. It +// mirrors the structure produced by the reference devcontainer CLI. +type LockedFeature struct { + Version string `json:"version,omitempty"` + Resolved string `json:"resolved,omitempty"` + Integrity string `json:"integrity,omitempty"` + DependsOn map[string]any `json:"dependsOn,omitempty"` +} + +// Lockfile mirrors the devcontainer-lock.json structure: a map of feature +// identifier to its pinned resolution. +type Lockfile struct { + Features map[string]LockedFeature `json:"features"` +} + +var ( + errLockfileMissing = errors.New("lockfile does not exist") + errLockfileMismatch = errors.New("lockfile does not match") +) + +// LockfilePath returns the lockfile path for a devcontainer config origin, +// matching the reference CLI: a hidden config file (basename starting with a +// dot) yields a hidden lockfile, otherwise devcontainer-lock.json is used. +// It returns "" when the origin is unknown or not a local path. +func LockfilePath(configOrigin string) string { + if configOrigin == "" || strings.Contains(configOrigin, "://") { + return "" + } + name := "devcontainer-lock.json" + if strings.HasPrefix(filepath.Base(configOrigin), ".") { + name = ".devcontainer-lock.json" + } + return filepath.Join(filepath.Dir(configOrigin), name) +} + +// ReadLockfile loads the lockfile at path. A missing or empty file yields an +// empty lockfile rather than an error. +func ReadLockfile(path string) (*Lockfile, error) { + lf := &Lockfile{Features: map[string]LockedFeature{}} + if path == "" { + return lf, nil + } + + data, err := os.ReadFile(filepath.Clean(path)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return lf, nil + } + return nil, fmt.Errorf("read lockfile %s: %w", path, err) + } + + if len(bytes.TrimSpace(data)) == 0 { + return lf, nil + } + + if err := json.Unmarshal(data, lf); err != nil { + return nil, fmt.Errorf("parse lockfile %s: %w", path, err) + } + if lf.Features == nil { + lf.Features = map[string]LockedFeature{} + } + return lf, nil +} + +// WriteLockfile persists the lockfile at path, matching reference-CLI +// semantics: it only rewrites when the normalized content changed, and when +// frozen it never writes, instead erroring if the file is missing or stale. +func WriteLockfile(path string, lf *Lockfile, frozen bool) error { + if path == "" { + return nil + } + + newContent, err := marshalLockfile(lf) + if err != nil { + return err + } + + write, err := lockfileNeedsWrite(path, newContent, frozen) + if err != nil || !write { + return err + } + + if err := os.WriteFile(path, newContent, 0o600); err != nil { + return fmt.Errorf("write lockfile %s: %w", path, err) + } + log.Debugf("wrote devcontainer lockfile: %s", path) + return nil +} + +// lockfileNeedsWrite reports whether the lockfile at path must be rewritten, +// applying frozen-mode enforcement (error if missing or stale). +func lockfileNeedsWrite(path string, newContent []byte, frozen bool) (bool, error) { + existing, err := os.ReadFile(filepath.Clean(path)) + switch { + case err == nil: + if bytes.Equal(normalizeLockfileJSON(existing), normalizeLockfileJSON(newContent)) { + return false, nil + } + if frozen { + return false, errLockfileMismatch + } + return true, nil + case errors.Is(err, os.ErrNotExist): + if frozen { + return false, errLockfileMissing + } + return true, nil + default: + return false, fmt.Errorf("read lockfile %s: %w", path, err) + } +} + +// marshalLockfile renders the lockfile with two-space indentation and a +// trailing newline, matching the reference CLI output. +func marshalLockfile(lf *Lockfile) ([]byte, error) { + data, err := json.MarshalIndent(lf, "", " ") + if err != nil { + return nil, fmt.Errorf("marshal lockfile: %w", err) + } + return append(data, '\n'), nil +} + +// normalizeLockfileJSON round-trips JSON so cosmetic differences (indentation, +// trailing whitespace) do not trigger a rewrite. Invalid input normalizes to +// nil, which forces a rewrite. +func normalizeLockfileJSON(data []byte) []byte { + var v any + if err := json.Unmarshal(data, &v); err != nil { + return nil + } + out, err := json.Marshal(v) + if err != nil { + return nil + } + return out +} + +// lockfileState carries the lockfile loaded for pinning and collects the +// entries resolved during a fetch so they can be written afterwards. +type lockfileState struct { + loaded *Lockfile + entries map[string]LockedFeature +} + +// pin returns the resolved reference and integrity digest recorded for a +// feature identifier, if the loaded lockfile contains it. +func (l *lockfileState) pin(featureID string) (resolved, integrity string, ok bool) { + if l == nil || l.loaded == nil { + return "", "", false + } + entry, found := l.loaded.Features[featureID] + if !found { + return "", "", false + } + return entry.Resolved, entry.Integrity, true +} + +// record stores the resolved entry for a feature identifier. +func (l *lockfileState) record(featureID string, entry LockedFeature) { + if l == nil { + return + } + l.entries[featureID] = entry +} + +// newLockfileState loads the lockfile for the given config to enable pinning. +// It always returns a usable state; a missing lockfile simply yields no pins. +func newLockfileState(cfg *config.DevContainerConfig) *lockfileState { + state := &lockfileState{entries: map[string]LockedFeature{}} + loaded, err := ReadLockfile(LockfilePath(cfg.Origin)) + if err != nil { + log.Warnf("ignoring unreadable devcontainer lockfile: %v", err) + return state + } + state.loaded = loaded + return state +} + +// checkFrozenPrecondition fails fast in frozen mode when the config declares +// features but no lockfile exists, avoiding wasted feature downloads before the +// commit-time enforcement would reject the build anyway. +func (l *lockfileState) checkFrozenPrecondition(cfg *config.DevContainerConfig) error { + if l == nil || len(cfg.Features) == 0 { + return nil + } + path := LockfilePath(cfg.Origin) + if path == "" || fileExists(path) { + return nil + } + return errLockfileMissing +} + +// commit writes the collected entries to the lockfile when write is requested. +// It regenerates the lockfile fully from the freshly resolved entries so that +// removed features drop out. To avoid creating spurious empty lockfiles, it +// skips writing when there are no entries and no lockfile already exists. +func (l *lockfileState) commit(cfg *config.DevContainerConfig, write, frozen bool) error { + if l == nil || !write { + return nil + } + + path := LockfilePath(cfg.Origin) + if path == "" { + return nil + } + + if len(l.entries) == 0 && !fileExists(path) { + return nil + } + + return WriteLockfile(path, &Lockfile{Features: l.entries}, frozen) +} + +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} diff --git a/pkg/devcontainer/feature/lockfile_test.go b/pkg/devcontainer/feature/lockfile_test.go new file mode 100644 index 000000000..324657ebc --- /dev/null +++ b/pkg/devcontainer/feature/lockfile_test.go @@ -0,0 +1,333 @@ +package feature + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" +) + +const ( + lockTestFeatureA = "ghcr.io/a/feature:1" + lockTestVersion = "1.0.0" + lockTestResolvedA = "ghcr.io/a/feature@sha256:a" + lockTestShaA = "sha256:a" + lockTestShaB = "sha256:b" +) + +func TestLockfilePath(t *testing.T) { + tests := []struct { + name string + origin string + want string + }{ + { + name: "standard config", + origin: "/repo/.devcontainer/devcontainer.json", + want: "/repo/.devcontainer/devcontainer-lock.json", + }, + { + name: "hidden config at root", + origin: "/repo/.devcontainer.json", + want: "/repo/.devcontainer-lock.json", + }, + { + name: "empty origin", + origin: "", + want: "", + }, + { + name: "non-local origin", + origin: "oci://ghcr.io/org/config", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := LockfilePath(tt.origin); got != tt.want { + t.Errorf("LockfilePath(%q) = %q, want %q", tt.origin, got, tt.want) + } + }) + } +} + +func TestReadLockfile_MissingReturnsEmpty(t *testing.T) { + lf, err := ReadLockfile(filepath.Join(t.TempDir(), "does-not-exist.json")) + if err != nil { + t.Fatalf("ReadLockfile: %v", err) + } + if lf == nil || lf.Features == nil || len(lf.Features) != 0 { + t.Fatalf("expected empty lockfile, got %+v", lf) + } +} + +func TestReadLockfile_ParsesEntries(t *testing.T) { + path := filepath.Join(t.TempDir(), "devcontainer-lock.json") + content := `{ + "features": { + "ghcr.io/devcontainers/features/git:1": { + "version": "1.3.8", + "resolved": "ghcr.io/devcontainers/features/git@sha256:abc", + "integrity": "sha256:abc" + } + } +}` + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + + lf, err := ReadLockfile(path) + if err != nil { + t.Fatalf("ReadLockfile: %v", err) + } + entry, ok := lf.Features["ghcr.io/devcontainers/features/git:1"] + if !ok { + t.Fatal("expected git feature entry") + } + if entry.Version != "1.3.8" || entry.Integrity != "sha256:abc" { + t.Errorf("unexpected entry: %+v", entry) + } +} + +func TestWriteLockfile_CreatesSortedStable(t *testing.T) { + path := filepath.Join(t.TempDir(), "devcontainer-lock.json") + lf := &Lockfile{Features: map[string]LockedFeature{ + "ghcr.io/b/feature:1": { + Version: "2.0.0", + Resolved: "ghcr.io/b/feature@sha256:b", + Integrity: lockTestShaB, + }, + lockTestFeatureA: { + Version: lockTestVersion, + Resolved: lockTestResolvedA, + Integrity: lockTestShaA, + }, + }} + + if err := WriteLockfile(path, lf, false); err != nil { + t.Fatalf("WriteLockfile: %v", err) + } + + data, err := os.ReadFile(filepath.Clean(path)) + if err != nil { + t.Fatal(err) + } + content := string(data) + if !strings.HasSuffix(content, "}\n") { + t.Errorf("expected trailing newline, got %q", content[len(content)-3:]) + } + // Keys must be emitted in sorted order. + if strings.Index(content, "ghcr.io/a/feature") > strings.Index(content, "ghcr.io/b/feature") { + t.Errorf("expected sorted keys, got:\n%s", content) + } +} + +func TestWriteLockfile_SkipsUnchanged(t *testing.T) { + path := filepath.Join(t.TempDir(), "devcontainer-lock.json") + lf := &Lockfile{Features: map[string]LockedFeature{ + lockTestFeatureA: {Version: lockTestVersion, Integrity: lockTestShaA}, + }} + if err := WriteLockfile(path, lf, false); err != nil { + t.Fatal(err) + } + info1, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + + // Rewrite identical content; file must not be modified. + if err := WriteLockfile(path, lf, false); err != nil { + t.Fatal(err) + } + info2, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !info1.ModTime().Equal(info2.ModTime()) { + t.Error("expected unchanged lockfile to be left untouched") + } +} + +func TestWriteLockfile_FrozenMissing(t *testing.T) { + path := filepath.Join(t.TempDir(), "devcontainer-lock.json") + lf := &Lockfile{Features: map[string]LockedFeature{ + lockTestFeatureA: {Integrity: lockTestShaA}, + }} + err := WriteLockfile(path, lf, true) + if err == nil { + t.Fatal("expected error for frozen missing lockfile") + } + if _, statErr := os.Stat(path); statErr == nil { + t.Error("frozen mode must not create the lockfile") + } +} + +func TestWriteLockfile_FrozenMismatch(t *testing.T) { + path := filepath.Join(t.TempDir(), "devcontainer-lock.json") + existing := &Lockfile{Features: map[string]LockedFeature{ + lockTestFeatureA: {Integrity: "sha256:old"}, + }} + if err := WriteLockfile(path, existing, false); err != nil { + t.Fatal(err) + } + + updated := &Lockfile{Features: map[string]LockedFeature{ + lockTestFeatureA: {Integrity: "sha256:new"}, + }} + if err := WriteLockfile(path, updated, true); err == nil { + t.Fatal("expected error for frozen mismatched lockfile") + } + + // Original content must be preserved. + lf, err := ReadLockfile(path) + if err != nil { + t.Fatal(err) + } + if lf.Features[lockTestFeatureA].Integrity != "sha256:old" { + t.Error("frozen mismatch must not overwrite the lockfile") + } +} + +func TestWriteLockfile_FrozenMatchSucceeds(t *testing.T) { + path := filepath.Join(t.TempDir(), "devcontainer-lock.json") + lf := &Lockfile{Features: map[string]LockedFeature{ + lockTestFeatureA: {Integrity: lockTestShaA}, + }} + if err := WriteLockfile(path, lf, false); err != nil { + t.Fatal(err) + } + if err := WriteLockfile(path, lf, true); err != nil { + t.Errorf("frozen matching lockfile should succeed, got %v", err) + } +} + +func TestLockfileState_PinAndRecord(t *testing.T) { + state := &lockfileState{ + loaded: &Lockfile{Features: map[string]LockedFeature{ + lockTestFeatureA: { + Resolved: lockTestResolvedA, + Integrity: lockTestShaA, + }, + }}, + entries: map[string]LockedFeature{}, + } + + resolved, integrity, ok := state.pin(lockTestFeatureA) + if !ok || resolved != lockTestResolvedA || integrity != lockTestShaA { + t.Errorf("pin returned %q %q %v", resolved, integrity, ok) + } + + if _, _, ok := state.pin("ghcr.io/missing:1"); ok { + t.Error("expected no pin for missing feature") + } + + state.record("ghcr.io/b/feature:1", LockedFeature{Integrity: lockTestShaB}) + if state.entries["ghcr.io/b/feature:1"].Integrity != lockTestShaB { + t.Error("record did not store entry") + } +} + +func TestLockfileState_NilSafe(t *testing.T) { + var state *lockfileState + if _, _, ok := state.pin("x"); ok { + t.Error("nil state must not pin") + } + state.record("x", LockedFeature{}) // must not panic +} + +func TestLockfileState_CommitSkipsEmptyWhenNoFile(t *testing.T) { + dir := t.TempDir() + cfg := &config.DevContainerConfig{} + cfg.Origin = filepath.Join(dir, "devcontainer.json") + + state := &lockfileState{entries: map[string]LockedFeature{}} + if err := state.commit(cfg, true, false); err != nil { + t.Fatalf("commit: %v", err) + } + if _, err := os.Stat(LockfilePath(cfg.Origin)); err == nil { + t.Error("commit must not create an empty lockfile when none exists") + } +} + +func TestLockfileState_CommitWritesEntries(t *testing.T) { + dir := t.TempDir() + cfg := &config.DevContainerConfig{} + cfg.Origin = filepath.Join(dir, "devcontainer.json") + + state := &lockfileState{entries: map[string]LockedFeature{ + lockTestFeatureA: {Version: lockTestVersion, Integrity: lockTestShaA}, + }} + if err := state.commit(cfg, true, false); err != nil { + t.Fatalf("commit: %v", err) + } + + lf, err := ReadLockfile(LockfilePath(cfg.Origin)) + if err != nil { + t.Fatal(err) + } + if lf.Features[lockTestFeatureA].Version != lockTestVersion { + t.Errorf("expected written entry, got %+v", lf.Features) + } +} + +func TestLockfileState_CommitReadOnlySkipsWrite(t *testing.T) { + dir := t.TempDir() + cfg := &config.DevContainerConfig{} + cfg.Origin = filepath.Join(dir, "devcontainer.json") + + state := &lockfileState{entries: map[string]LockedFeature{ + lockTestFeatureA: {Integrity: lockTestShaA}, + }} + if err := state.commit(cfg, false, false); err != nil { + t.Fatalf("commit: %v", err) + } + if _, err := os.Stat(LockfilePath(cfg.Origin)); err == nil { + t.Error("read-only commit must not write the lockfile") + } +} + +func TestLockfileState_CheckFrozenPrecondition(t *testing.T) { + dir := t.TempDir() + cfg := &config.DevContainerConfig{} + cfg.Features = map[string]any{lockTestFeatureA: map[string]any{}} + cfg.Origin = filepath.Join(dir, "devcontainer.json") + + state := &lockfileState{entries: map[string]LockedFeature{}} + + // Features declared but no lockfile on disk -> fail fast. + if err := state.checkFrozenPrecondition(cfg); err == nil { + t.Error("expected frozen precondition to fail when lockfile missing") + } + + // No declared features -> nothing to enforce. + empty := &config.DevContainerConfig{} + empty.Origin = cfg.Origin + if err := state.checkFrozenPrecondition(empty); err != nil { + t.Errorf("expected no error with no features, got %v", err) + } + + // Lockfile present -> precondition passes (mismatch enforced later). + if err := WriteLockfile(LockfilePath(cfg.Origin), &Lockfile{Features: map[string]LockedFeature{ + lockTestFeatureA: {Integrity: lockTestShaA}, + }}, false); err != nil { + t.Fatal(err) + } + if err := state.checkFrozenPrecondition(cfg); err != nil { + t.Errorf("expected precondition to pass with lockfile present, got %v", err) + } +} + +func TestVerifyPinnedDigest(t *testing.T) { + if err := verifyPinnedDigest("id", lockTestShaA, ""); err != nil { + t.Errorf("no pin should pass: %v", err) + } + if err := verifyPinnedDigest("id", lockTestShaA, lockTestShaA); err != nil { + t.Errorf("matching pin should pass: %v", err) + } + if err := verifyPinnedDigest("id", lockTestShaA, lockTestShaB); err == nil { + t.Error("mismatched pin should fail") + } +} diff --git a/pkg/flags/names/names.go b/pkg/flags/names/names.go index 08202c4bb..a06187fac 100644 --- a/pkg/flags/names/names.go +++ b/pkg/flags/names/names.go @@ -163,6 +163,7 @@ const ( Label = "label" Output = "output" ExperimentalLockfile = "experimental-lockfile" + FrozenLockfile = "frozen-lockfile" ImageName = "image-name" NoBuild = "no-build" ForceBuild = "force-build" diff --git a/pkg/provider/workspace.go b/pkg/provider/workspace.go index 92d813df6..611449969 100644 --- a/pkg/provider/workspace.go +++ b/pkg/provider/workspace.go @@ -291,6 +291,10 @@ type CLIOptions struct { Labels []string `json:"labels,omitempty"` Output string `json:"output,omitempty"` ExperimentalLockfile string `json:"experimentalLockfile,omitempty"` + // FrozenLockfile enforces that the devcontainer-lock.json feature lockfile + // already exists and matches the resolved features; the build fails instead + // of creating or updating it. + FrozenLockfile bool `json:"frozenLockfile,omitempty"` // ImageName specifies an alternative name for the built image. ImageName string `json:"imageName,omitempty"` // NoBuild prevents building; the command will fail if the image does not exist. From 3246528dd391c565c7e5b9b143a96added4bf016 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 22 Jul 2026 22:42:02 -0500 Subject: [PATCH 2/6] test(e2e): validate generated lockfile contents and reject bogus pins Parse the generated devcontainer-lock.json and assert structural invariants (exactly the declared feature, sha256-formatted integrity, resolved == repo@integrity, semver version) instead of loose substring checks. Add a negative spec that a committed lockfile pinning a non-existent digest is honored and fails resolution. --- .../.devcontainer-lock.json | 9 ++++ .../.devcontainer.json | 7 +++ e2e/tests/up-features/up_features.go | 51 +++++++++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer-lock.json create mode 100644 e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer.json diff --git a/e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer-lock.json b/e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer-lock.json new file mode 100644 index 000000000..a4135940e --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer-lock.json @@ -0,0 +1,9 @@ +{ + "features": { + "ghcr.io/devcontainers/features/git:1": { + "version": "1.0.0", + "resolved": "ghcr.io/devcontainers/features/git@sha256:0000000000000000000000000000000000000000000000000000000000000000", + "integrity": "sha256:0000000000000000000000000000000000000000000000000000000000000000" + } + } +} diff --git a/e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer.json b/e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer.json new file mode 100644 index 000000000..b5566bc05 --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer.json @@ -0,0 +1,7 @@ +{ + "name": "Test Lockfile Bad Pin", + "image": "ghcr.io/devsy-org/test-images/base:ubuntu", + "features": { + "ghcr.io/devcontainers/features/git:1": {} + } +} diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index 2b69ea6fb..83a9c6f7f 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -4,13 +4,16 @@ package up import ( "context" + "encoding/json" "net/http" "os" "path" "path/filepath" + "regexp" "strings" "github.com/devsy-org/devsy/e2e/framework" + "github.com/devsy-org/devsy/pkg/devcontainer/feature" "github.com/devsy-org/devsy/pkg/flags/names" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" @@ -557,11 +560,24 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( // The config basename starts with a dot, so the lockfile is hidden. lockContent, err := f.DevsySSH(ctx, wsName, "cat /workspaces/*/.devcontainer-lock.json") framework.ExpectNoError(err) - gomega.Expect(lockContent). - To(gomega.ContainSubstring("ghcr.io/devcontainers/features/git:1")) - gomega.Expect(lockContent).To(gomega.ContainSubstring("\"resolved\"")) - gomega.Expect(lockContent).To(gomega.ContainSubstring("@sha256:")) - gomega.Expect(lockContent).To(gomega.ContainSubstring("\"integrity\"")) + + // Validate the generated lockfile is structurally correct rather + // than merely present: it must parse, contain exactly the declared + // feature, and its digest fields must be internally consistent. + var lock feature.Lockfile + framework.ExpectNoError(json.Unmarshal([]byte(strings.TrimSpace(lockContent)), &lock)) + + const featureKey = "ghcr.io/devcontainers/features/git:1" + gomega.Expect(lock.Features).To(gomega.HaveKey(featureKey)) + gomega.Expect(lock.Features).To(gomega.HaveLen(1)) + + entry := lock.Features[featureKey] + digestRe := regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) + gomega.Expect(entry.Integrity).To(gomega.MatchRegexp(digestRe.String())) + // For OCI features resolved is repo@; the two must agree. + gomega.Expect(entry.Resolved). + To(gomega.Equal("ghcr.io/devcontainers/features/git@" + entry.Integrity)) + gomega.Expect(entry.Version).To(gomega.MatchRegexp(`^\d+\.\d+\.\d+`)) // Second up with the frozen flag must succeed: the generated // lockfile matches the resolved features exactly. @@ -594,6 +610,31 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( ginkgo.SpecTimeout(framework.TimeoutLong()), ) + ginkgo.It( + "honors a committed lockfile and rejects a bogus pinned digest", + ginkgo.Label("up-features", "lockfile"), + func(ctx context.Context) { + f, err := setupDockerProvider(initialDir+"/bin", "docker") + framework.ExpectNoError(err) + + tempDir, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-lockfile-bad", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) + + wsName := filepath.Base(tempDir) + ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) + + // The committed lockfile pins the feature to a non-existent digest. + // If the lockfile is honored, resolution fails; a pass would mean the + // pin was silently ignored. + err = f.DevsyUp(ctx, tempDir) + framework.ExpectError(err) + }, + ginkgo.SpecTimeout(framework.TimeoutLong()), + ) + ginkgo.It("resolves user variable in dockerfile", func(ctx context.Context) { f, err := setupDockerProvider(initialDir+"/bin", "docker") framework.ExpectNoError(err) From 825af6f21077d2b68ea133d07dca2e0d01b6ae9d Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 22 Jul 2026 22:46:06 -0500 Subject: [PATCH 3/6] test(e2e): assert exact lockfile digest for a served tarball feature Add a spec that serves a tarball feature from the in-test http server and asserts the lockfile records the exact integrity (sha256 of the served bytes, computed with the same hash helper devsy uses) and the tarball URL as resolved. Deterministic exact-value check with no upstream coupling. --- e2e/tests/up-features/up_features.go | 71 ++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index 83a9c6f7f..f153eaa40 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -15,6 +15,7 @@ import ( "github.com/devsy-org/devsy/e2e/framework" "github.com/devsy-org/devsy/pkg/devcontainer/feature" "github.com/devsy-org/devsy/pkg/flags/names" + "github.com/devsy-org/devsy/pkg/hash" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" "github.com/onsi/gomega/ghttp" @@ -635,6 +636,76 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( ginkgo.SpecTimeout(framework.TimeoutLong()), ) + ginkgo.It( + "records the exact tarball digest in the lockfile", + ginkgo.Label("up-features", "lockfile"), + func(ctx context.Context) { + server := ghttp.NewServer() + ginkgo.DeferCleanup(server.Close) + + tempDir, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-http-headers", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) + + featureArchiveFilePath := path.Join(tempDir, "devcontainer-feature-hello.tgz") + featureFiles := []string{ + path.Join(tempDir, "devcontainer-feature.json"), + path.Join(tempDir, "install.sh"), + } + framework.ExpectNoError(createTarGzArchive(featureArchiveFilePath, featureFiles)) + + devContainerFile := filepath.Clean(path.Join(tempDir, ".devcontainer.json")) + devContainerFileBuf, err := os.ReadFile(devContainerFile) + framework.ExpectNoError(err) + output := strings.ReplaceAll(string(devContainerFileBuf), "#{server_url}", server.URL()) + // #nosec G306 -- test file, permissive mode is acceptable. + err = os.WriteFile(path.Join(tempDir, ".devcontainer.json"), []byte(output), 0o644) + framework.ExpectNoError(err) + + respHeader := http.Header{} + respHeader.Set( + "Content-Disposition", + "attachment; filename=devcontainer-feature-hello.tgz", + ) + featureArchiveFileBuf, err := os.ReadFile(filepath.Clean(featureArchiveFilePath)) + framework.ExpectNoError(err) + server.AppendHandlers(ghttp.CombineHandlers( + ghttp.VerifyRequest("GET", "/devcontainer-feature-hello.tgz"), + ghttp.RespondWith(http.StatusOK, featureArchiveFileBuf, respHeader), + )) + + f := framework.NewDefaultFramework(initialDir + "/bin") + _ = f.DevsyProviderDelete(ctx, "docker") + framework.ExpectNoError(f.DevsyProviderAdd(ctx, "docker")) + framework.ExpectNoError(f.DevsyProviderUse(ctx, "docker")) + + wsName := filepath.Base(tempDir) + ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) + framework.ExpectNoError(f.DevsyUp(ctx, tempDir)) + + lockContent, err := f.DevsySSH(ctx, wsName, "cat /workspaces/*/.devcontainer-lock.json") + framework.ExpectNoError(err) + + var lock feature.Lockfile + framework.ExpectNoError(json.Unmarshal([]byte(strings.TrimSpace(lockContent)), &lock)) + + // The lockfile is keyed by the tarball URL, and the integrity must be + // the exact sha256 of the bytes devsy downloaded — computed here with + // the same helper devsy uses internally, so the value cannot drift. + featureURL := server.URL() + "/devcontainer-feature-hello.tgz" + digestHex, err := hash.File(featureArchiveFilePath) + framework.ExpectNoError(err) + + gomega.Expect(lock.Features).To(gomega.HaveKey(featureURL)) + entry := lock.Features[featureURL] + gomega.Expect(entry.Resolved).To(gomega.Equal(featureURL)) + gomega.Expect(entry.Integrity).To(gomega.Equal("sha256:" + digestHex)) + }, + ginkgo.SpecTimeout(framework.TimeoutLong()), + ) + ginkgo.It("resolves user variable in dockerfile", func(ctx context.Context) { f, err := setupDockerProvider(initialDir+"/bin", "docker") framework.ExpectNoError(err) From 44faf504e9d85cd51def1fc98dcef6c44843a701 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Jul 2026 00:34:06 -0500 Subject: [PATCH 4/6] feat(lockfile): add --no-lockfile opt-out, mutually exclusive with --frozen-lockfile Match the official devcontainer CLI, which creates the lockfile by default but lets you disable it. --no-lockfile turns the feature lockfile off entirely (no pinning, no writing); it is mutually exclusive with --frozen-lockfile, enforced via cobra's MarkFlagsMutuallyExclusive on both build and up. Refactors the two positional lock bools threaded through fetchFeatures into a lockfileMode{write,frozen,disabled} struct. Adds unit tests (flag registration, mutual exclusivity, disabled no-op commit) and an e2e spec asserting --no-lockfile writes no devcontainer-lock.json. --- cmd/workspace/build.go | 3 +++ cmd/workspace/build_test.go | 18 ++++++++++++++++ cmd/workspace/up/up.go | 1 + cmd/workspace/up/up_flags.go | 2 ++ cmd/workspace/up/up_test.go | 11 ++++++++++ e2e/tests/up-features/up_features.go | 26 +++++++++++++++++++++++ pkg/devcontainer/build.go | 3 +++ pkg/devcontainer/compose.go | 4 ++++ pkg/devcontainer/compose_build.go | 1 + pkg/devcontainer/feature/extend.go | 19 +++++++++++------ pkg/devcontainer/feature/lockfile.go | 18 +++++++++++++--- pkg/devcontainer/feature/lockfile_test.go | 21 +++++++++++++++--- pkg/flags/names/names.go | 1 + pkg/provider/workspace.go | 3 +++ 14 files changed, 118 insertions(+), 13 deletions(-) diff --git a/cmd/workspace/build.go b/cmd/workspace/build.go index dc50f605b..63f5585c5 100644 --- a/cmd/workspace/build.go +++ b/cmd/workspace/build.go @@ -45,6 +45,7 @@ func NewBuildCmd(flags *cmdflags.GlobalFlags) *cobra.Command { } cmd.registerFlags(buildCmd) + buildCmd.MarkFlagsMutuallyExclusive(names.NoLockfile, names.FrozenLockfile) return buildCmd } @@ -112,6 +113,8 @@ func (cmd *BuildCmd) registerImageFlags(buildCmd *cobra.Command) { cliflags.Bool(&cmd.FrozenLockfile, names.FrozenLockfile, false, "Fail if devcontainer-lock.json is missing or does not match the resolved features "+ "instead of writing it (useful for CI)"), + cliflags.Bool(&cmd.NoLockfile, names.NoLockfile, false, + "Disable devcontainer-lock.json generation and verification"), cliflags.String( &cmd.ImageName, names.ImageName, diff --git a/cmd/workspace/build_test.go b/cmd/workspace/build_test.go index a43a1f5f5..f40858f7e 100644 --- a/cmd/workspace/build_test.go +++ b/cmd/workspace/build_test.go @@ -16,6 +16,24 @@ func TestBuildCmd_NoCacheFlag(t *testing.T) { assert.Equal(t, "false", flag.DefValue) } +func TestBuildCmd_NoLockfileFlag(t *testing.T) { + buildCmd := NewBuildCmd(&flags.GlobalFlags{}) + flag := buildCmd.Flags().Lookup(names.NoLockfile) + require.NotNil(t, flag) + assert.Equal(t, "false", flag.DefValue) +} + +func TestBuildCmd_NoLockfileAndFrozenLockfileMutuallyExclusive(t *testing.T) { + buildCmd := NewBuildCmd(&flags.GlobalFlags{}) + require.NoError(t, buildCmd.Flags().Parse( + []string{names.Flag(names.NoLockfile), names.Flag(names.FrozenLockfile)}, + )) + err := buildCmd.ValidateFlagGroups() + require.Error(t, err) + assert.Contains(t, err.Error(), names.NoLockfile) + assert.Contains(t, err.Error(), names.FrozenLockfile) +} + func TestBuildCmd_NoCacheFlagParsesValue(t *testing.T) { buildCmd := NewBuildCmd(&flags.GlobalFlags{}) err := buildCmd.ParseFlags([]string{"--no-cache"}) diff --git a/cmd/workspace/up/up.go b/cmd/workspace/up/up.go index 3f8b7d44e..8c5b14ddd 100644 --- a/cmd/workspace/up/up.go +++ b/cmd/workspace/up/up.go @@ -148,6 +148,7 @@ func NewUpCmd(f *flags.GlobalFlags) *cobra.Command { RunE: cmd.execute, } cmd.registerFlags(upCmd) + upCmd.MarkFlagsMutuallyExclusive(names.NoLockfile, names.FrozenLockfile) return upCmd } diff --git a/cmd/workspace/up/up_flags.go b/cmd/workspace/up/up_flags.go index abbd7457a..53ab5a279 100644 --- a/cmd/workspace/up/up_flags.go +++ b/cmd/workspace/up/up_flags.go @@ -73,6 +73,8 @@ func (cmd *UpCmd) registerBuildFlags(upCmd *cobra.Command) { flags.Bool(&cmd.FrozenLockfile, names.FrozenLockfile, false, "Fail if devcontainer-lock.json is missing or does not match the resolved features "+ "instead of writing it (useful for CI)"), + flags.Bool(&cmd.NoLockfile, names.NoLockfile, false, + "Disable devcontainer-lock.json generation and verification"), ) flags.RegisterDevContainerModifierFlags(upCmd.Flags(), flags.DevContainerModifierFlags{ Image: &cmd.DevContainerImage, diff --git a/cmd/workspace/up/up_test.go b/cmd/workspace/up/up_test.go index a6e920729..d31abc2c5 100644 --- a/cmd/workspace/up/up_test.go +++ b/cmd/workspace/up/up_test.go @@ -16,6 +16,17 @@ const ( testBindMountAB = "type=bind,source=/a,target=/b" ) +func TestUpCmd_NoLockfileAndFrozenLockfileMutuallyExclusive(t *testing.T) { + upCmd := NewUpCmd(&flags.GlobalFlags{}) + require.NoError(t, upCmd.Flags().Parse( + []string{names.Flag(names.NoLockfile), names.Flag(names.FrozenLockfile)}, + )) + err := upCmd.ValidateFlagGroups() + require.Error(t, err) + assert.Contains(t, err.Error(), names.NoLockfile) + assert.Contains(t, err.Error(), names.FrozenLockfile) +} + func TestUpCmd_ValidateDefaultUserEnvProbe(t *testing.T) { tests := []struct { name string diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index f153eaa40..b033c417b 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -611,6 +611,32 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( ginkgo.SpecTimeout(framework.TimeoutLong()), ) + ginkgo.It( + "does not write a lockfile with --no-lockfile", + ginkgo.Label("up-features", "lockfile"), + func(ctx context.Context) { + f, err := setupDockerProvider(initialDir+"/bin", "docker") + framework.ExpectNoError(err) + + tempDir, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-lockfile", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) + + wsName := filepath.Base(tempDir) + ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) + + err = f.DevsyUp(ctx, tempDir, names.Flag(names.NoLockfile)) + framework.ExpectNoError(err) + + // With --no-lockfile the lockfile must not be created, so cat fails. + _, err = f.DevsySSH(ctx, wsName, "cat /workspaces/*/.devcontainer-lock.json") + framework.ExpectError(err) + }, + ginkgo.SpecTimeout(framework.TimeoutLong()), + ) + ginkgo.It( "honors a committed lockfile and rejects a bogus pinned digest", ginkgo.Label("up-features", "lockfile"), diff --git a/pkg/devcontainer/build.go b/pkg/devcontainer/build.go index a644ef734..200b0f328 100644 --- a/pkg/devcontainer/build.go +++ b/pkg/devcontainer/build.go @@ -79,6 +79,7 @@ func (r *runner) extendImage( ForceBuild: options.ForceBuild, SecretOpts: featureSecretOpts(options), FrozenLockfile: options.FrozenLockfile, + NoLockfile: options.NoLockfile, }) if err != nil { return nil, fmt.Errorf("get extended build info: %w", err) @@ -186,6 +187,7 @@ func (r *runner) buildAndExtendImage( ForceBuild: options.ForceBuild, SecretOpts: featureSecretOpts(options), FrozenLockfile: options.FrozenLockfile, + NoLockfile: options.NoLockfile, }) if err != nil { return nil, fmt.Errorf("get extended build info: %w", err) @@ -557,6 +559,7 @@ func (r *runner) buildDevImageCompose( pull: options.Pull, noCache: options.NoCache, frozenLockfile: options.FrozenLockfile, + noLockfile: options.NoLockfile, }) if err != nil { return nil, fmt.Errorf("build and extend docker-compose: %w", err) diff --git a/pkg/devcontainer/compose.go b/pkg/devcontainer/compose.go index 7a69866c6..1a5fc4e16 100644 --- a/pkg/devcontainer/compose.go +++ b/pkg/devcontainer/compose.go @@ -84,6 +84,9 @@ type buildAndExtendParams struct { // frozenLockfile enforces that the devcontainer lockfile matches the // resolved features, set from CLIOptions.FrozenLockfile. frozenLockfile bool + // noLockfile disables the devcontainer lockfile entirely, set from + // CLIOptions.NoLockfile. + noLockfile bool } // composeUpParams groups the inputs shared by extendedDockerComposeUp and @@ -761,6 +764,7 @@ func (r *runner) buildComposeOverrideArgs( pull: start.options.Pull, noCache: start.options.NoCache, frozenLockfile: start.options.FrozenLockfile, + noLockfile: start.options.NoLockfile, }) if err != nil { return nil, fmt.Errorf("build and extend docker-compose: %w", err) diff --git a/pkg/devcontainer/compose_build.go b/pkg/devcontainer/compose_build.go index 9d06a9a0f..47dd7e6e7 100644 --- a/pkg/devcontainer/compose_build.go +++ b/pkg/devcontainer/compose_build.go @@ -231,6 +231,7 @@ func (r *runner) prepareExtendedComposeBuild( Prompter: &feature.TerminalSecretPrompter{}, }, FrozenLockfile: params.frozenLockfile, + NoLockfile: params.noLockfile, }) if err != nil { return preparedComposeBuild{}, err diff --git a/pkg/devcontainer/feature/extend.go b/pkg/devcontainer/feature/extend.go index f40d6bef7..dc95441c7 100644 --- a/pkg/devcontainer/feature/extend.go +++ b/pkg/devcontainer/feature/extend.go @@ -62,6 +62,8 @@ type ExtendedBuildParams struct { // FrozenLockfile enforces that the devcontainer lockfile exists and matches // the resolved features; the build fails instead of writing the lockfile. FrozenLockfile bool + // NoLockfile disables the lockfile entirely: no pinning and no writing. + NoLockfile bool } func GetExtendedBuildInfo(params *ExtendedBuildParams) (*ExtendedBuildInfo, error) { @@ -72,7 +74,8 @@ func GetExtendedBuildInfo(params *ExtendedBuildParams) (*ExtendedBuildInfo, erro forceBuild := params.ForceBuild secretOpts := params.SecretOpts features, err := fetchFeatures( - devContainerConfig.Config, forceBuild, secretOpts, true, params.FrozenLockfile, + devContainerConfig.Config, forceBuild, secretOpts, + lockfileMode{write: true, frozen: params.FrozenLockfile, disabled: params.NoLockfile}, ) if err != nil { return nil, fmt.Errorf("fetch features: %w", err) @@ -293,7 +296,7 @@ func usersFromMetadata(meta *config.ImageMetadataConfig) (string, string) { func ResolveFeatureOrder( devContainerConfig *config.DevContainerConfig, ) ([]*config.FeatureSet, error) { - return fetchFeatures(devContainerConfig, false, nil, false, false) + return fetchFeatures(devContainerConfig, false, nil, lockfileMode{}) } func applyUserFallback(user, composeServiceUser, imageUser string) string { @@ -310,10 +313,12 @@ func fetchFeatures( devContainerConfig *config.DevContainerConfig, forceBuild bool, secretOpts *SecretOptions, - lockWrite bool, - lockFrozen bool, + lockMode lockfileMode, ) ([]*config.FeatureSet, error) { - lock := newLockfileState(devContainerConfig) + var lock *lockfileState + if !lockMode.disabled { + lock = newLockfileState(devContainerConfig) + } processor := &featureProcessor{ devContainerConfig: devContainerConfig, forceBuild: forceBuild, @@ -321,7 +326,7 @@ func fetchFeatures( lock: lock, } - if lockWrite && lockFrozen { + if lockMode.write && lockMode.frozen { if err := lock.checkFrozenPrecondition(devContainerConfig); err != nil { return nil, err } @@ -347,7 +352,7 @@ func fetchFeatures( return nil, fmt.Errorf("failed to get sorted feature sets: %w", err) } - if err := lock.commit(devContainerConfig, lockWrite, lockFrozen); err != nil { + if err := lock.commit(devContainerConfig, lockMode); err != nil { return nil, err } diff --git a/pkg/devcontainer/feature/lockfile.go b/pkg/devcontainer/feature/lockfile.go index 78a6caf96..d9a71d5aa 100644 --- a/pkg/devcontainer/feature/lockfile.go +++ b/pkg/devcontainer/feature/lockfile.go @@ -150,6 +150,18 @@ func normalizeLockfileJSON(data []byte) []byte { return out } +// lockfileMode controls how the feature lockfile participates in a fetch. +type lockfileMode struct { + // write creates or updates the lockfile after resolution. + write bool + // frozen fails the build instead of writing when the lockfile is missing or + // would change. + frozen bool + // disabled turns the lockfile off entirely — no load, no pinning, no write + // (the --no-lockfile opt-out). + disabled bool +} + // lockfileState carries the lockfile loaded for pinning and collects the // entries resolved during a fetch so they can be written afterwards. type lockfileState struct { @@ -209,8 +221,8 @@ func (l *lockfileState) checkFrozenPrecondition(cfg *config.DevContainerConfig) // It regenerates the lockfile fully from the freshly resolved entries so that // removed features drop out. To avoid creating spurious empty lockfiles, it // skips writing when there are no entries and no lockfile already exists. -func (l *lockfileState) commit(cfg *config.DevContainerConfig, write, frozen bool) error { - if l == nil || !write { +func (l *lockfileState) commit(cfg *config.DevContainerConfig, mode lockfileMode) error { + if l == nil || !mode.write { return nil } @@ -223,7 +235,7 @@ func (l *lockfileState) commit(cfg *config.DevContainerConfig, write, frozen boo return nil } - return WriteLockfile(path, &Lockfile{Features: l.entries}, frozen) + return WriteLockfile(path, &Lockfile{Features: l.entries}, mode.frozen) } func fileExists(path string) bool { diff --git a/pkg/devcontainer/feature/lockfile_test.go b/pkg/devcontainer/feature/lockfile_test.go index 324657ebc..dbfab6d80 100644 --- a/pkg/devcontainer/feature/lockfile_test.go +++ b/pkg/devcontainer/feature/lockfile_test.go @@ -244,7 +244,7 @@ func TestLockfileState_CommitSkipsEmptyWhenNoFile(t *testing.T) { cfg.Origin = filepath.Join(dir, "devcontainer.json") state := &lockfileState{entries: map[string]LockedFeature{}} - if err := state.commit(cfg, true, false); err != nil { + if err := state.commit(cfg, lockfileMode{write: true}); err != nil { t.Fatalf("commit: %v", err) } if _, err := os.Stat(LockfilePath(cfg.Origin)); err == nil { @@ -260,7 +260,7 @@ func TestLockfileState_CommitWritesEntries(t *testing.T) { state := &lockfileState{entries: map[string]LockedFeature{ lockTestFeatureA: {Version: lockTestVersion, Integrity: lockTestShaA}, }} - if err := state.commit(cfg, true, false); err != nil { + if err := state.commit(cfg, lockfileMode{write: true}); err != nil { t.Fatalf("commit: %v", err) } @@ -281,7 +281,7 @@ func TestLockfileState_CommitReadOnlySkipsWrite(t *testing.T) { state := &lockfileState{entries: map[string]LockedFeature{ lockTestFeatureA: {Integrity: lockTestShaA}, }} - if err := state.commit(cfg, false, false); err != nil { + if err := state.commit(cfg, lockfileMode{}); err != nil { t.Fatalf("commit: %v", err) } if _, err := os.Stat(LockfilePath(cfg.Origin)); err == nil { @@ -289,6 +289,21 @@ func TestLockfileState_CommitReadOnlySkipsWrite(t *testing.T) { } } +func TestLockfileState_CommitDisabledIsNoOp(t *testing.T) { + dir := t.TempDir() + cfg := &config.DevContainerConfig{} + cfg.Origin = filepath.Join(dir, "devcontainer.json") + + // --no-lockfile yields a nil state; commit must never write. + var state *lockfileState + if err := state.commit(cfg, lockfileMode{write: true, disabled: true}); err != nil { + t.Fatalf("commit: %v", err) + } + if _, err := os.Stat(LockfilePath(cfg.Origin)); err == nil { + t.Error("disabled lockfile must not write a file") + } +} + func TestLockfileState_CheckFrozenPrecondition(t *testing.T) { dir := t.TempDir() cfg := &config.DevContainerConfig{} diff --git a/pkg/flags/names/names.go b/pkg/flags/names/names.go index a06187fac..45679e6bf 100644 --- a/pkg/flags/names/names.go +++ b/pkg/flags/names/names.go @@ -164,6 +164,7 @@ const ( Output = "output" ExperimentalLockfile = "experimental-lockfile" FrozenLockfile = "frozen-lockfile" + NoLockfile = "no-lockfile" ImageName = "image-name" NoBuild = "no-build" ForceBuild = "force-build" diff --git a/pkg/provider/workspace.go b/pkg/provider/workspace.go index 611449969..46cebcb30 100644 --- a/pkg/provider/workspace.go +++ b/pkg/provider/workspace.go @@ -295,6 +295,9 @@ type CLIOptions struct { // already exists and matches the resolved features; the build fails instead // of creating or updating it. FrozenLockfile bool `json:"frozenLockfile,omitempty"` + // NoLockfile disables devcontainer-lock.json generation and verification + // entirely. Mutually exclusive with FrozenLockfile. + NoLockfile bool `json:"noLockfile,omitempty"` // ImageName specifies an alternative name for the built image. ImageName string `json:"imageName,omitempty"` // NoBuild prevents building; the command will fail if the image does not exist. From 745a07daddac5f5af887a457523845d9a3999270 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Jul 2026 16:16:37 +0000 Subject: [PATCH 5/6] chore: comment cleanup Signed-off-by: GitHub --- cmd/workspace/build.go | 2 +- cmd/workspace/up/up_flags.go | 2 +- e2e/tests/up-features/up_features.go | 17 ----------------- pkg/devcontainer/compose.go | 16 ++++------------ pkg/devcontainer/feature/extend.go | 7 ++----- pkg/devcontainer/feature/features.go | 10 +++------- pkg/devcontainer/feature/lockfile.go | 27 ++++----------------------- 7 files changed, 15 insertions(+), 66 deletions(-) diff --git a/cmd/workspace/build.go b/cmd/workspace/build.go index 63f5585c5..e534f85e3 100644 --- a/cmd/workspace/build.go +++ b/cmd/workspace/build.go @@ -112,7 +112,7 @@ func (cmd *BuildCmd) registerImageFlags(buildCmd *cobra.Command) { "Lockfile path for reproducible builds"), cliflags.Bool(&cmd.FrozenLockfile, names.FrozenLockfile, false, "Fail if devcontainer-lock.json is missing or does not match the resolved features "+ - "instead of writing it (useful for CI)"), + "instead of writing it"), cliflags.Bool(&cmd.NoLockfile, names.NoLockfile, false, "Disable devcontainer-lock.json generation and verification"), cliflags.String( diff --git a/cmd/workspace/up/up_flags.go b/cmd/workspace/up/up_flags.go index 53ab5a279..54f92dfb0 100644 --- a/cmd/workspace/up/up_flags.go +++ b/cmd/workspace/up/up_flags.go @@ -72,7 +72,7 @@ func (cmd *UpCmd) registerBuildFlags(upCmd *cobra.Command) { "Mount the workspace git root as the workspace folder"), flags.Bool(&cmd.FrozenLockfile, names.FrozenLockfile, false, "Fail if devcontainer-lock.json is missing or does not match the resolved features "+ - "instead of writing it (useful for CI)"), + "instead of writing it"), flags.Bool(&cmd.NoLockfile, names.NoLockfile, false, "Disable devcontainer-lock.json generation and verification"), ) diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index b033c417b..1a4357a2c 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -553,18 +553,12 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( wsName := filepath.Base(tempDir) ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) - // First up: no lockfile exists yet, so it is generated with the - // resolved digest and integrity for the OCI feature. err = f.DevsyUp(ctx, tempDir) framework.ExpectNoError(err) - // The config basename starts with a dot, so the lockfile is hidden. lockContent, err := f.DevsySSH(ctx, wsName, "cat /workspaces/*/.devcontainer-lock.json") framework.ExpectNoError(err) - // Validate the generated lockfile is structurally correct rather - // than merely present: it must parse, contain exactly the declared - // feature, and its digest fields must be internally consistent. var lock feature.Lockfile framework.ExpectNoError(json.Unmarshal([]byte(strings.TrimSpace(lockContent)), &lock)) @@ -575,13 +569,10 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( entry := lock.Features[featureKey] digestRe := regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) gomega.Expect(entry.Integrity).To(gomega.MatchRegexp(digestRe.String())) - // For OCI features resolved is repo@; the two must agree. gomega.Expect(entry.Resolved). To(gomega.Equal("ghcr.io/devcontainers/features/git@" + entry.Integrity)) gomega.Expect(entry.Version).To(gomega.MatchRegexp(`^\d+\.\d+\.\d+`)) - // Second up with the frozen flag must succeed: the generated - // lockfile matches the resolved features exactly. err = f.DevsyUpRecreate(ctx, tempDir, names.Flag(names.FrozenLockfile)) framework.ExpectNoError(err) }, @@ -604,7 +595,6 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( wsName := filepath.Base(tempDir) ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) - // No lockfile is committed, so frozen mode must fail fast. err = f.DevsyUp(ctx, tempDir, names.Flag(names.FrozenLockfile)) framework.ExpectError(err) }, @@ -630,7 +620,6 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( err = f.DevsyUp(ctx, tempDir, names.Flag(names.NoLockfile)) framework.ExpectNoError(err) - // With --no-lockfile the lockfile must not be created, so cat fails. _, err = f.DevsySSH(ctx, wsName, "cat /workspaces/*/.devcontainer-lock.json") framework.ExpectError(err) }, @@ -653,9 +642,6 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( wsName := filepath.Base(tempDir) ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) - // The committed lockfile pins the feature to a non-existent digest. - // If the lockfile is honored, resolution fails; a pass would mean the - // pin was silently ignored. err = f.DevsyUp(ctx, tempDir) framework.ExpectError(err) }, @@ -717,9 +703,6 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features"), func( var lock feature.Lockfile framework.ExpectNoError(json.Unmarshal([]byte(strings.TrimSpace(lockContent)), &lock)) - // The lockfile is keyed by the tarball URL, and the integrity must be - // the exact sha256 of the bytes devsy downloaded — computed here with - // the same helper devsy uses internally, so the value cannot drift. featureURL := server.URL() + "/devcontainer-feature-hello.tgz" digestHex, err := hash.File(featureArchiveFilePath) framework.ExpectNoError(err) diff --git a/pkg/devcontainer/compose.go b/pkg/devcontainer/compose.go index 1a5fc4e16..b7e89f1f1 100644 --- a/pkg/devcontainer/compose.go +++ b/pkg/devcontainer/compose.go @@ -75,18 +75,10 @@ type buildAndExtendParams struct { composeService *composetypes.ServiceConfig globalArgs []string featureSecretsFile string - // pull re-pulls base images during the compose build (--pull), set from - // CLIOptions.Pull. - pull bool - // noCache disables the build cache during the compose build (--no-cache), - // set from CLIOptions.NoCache. - noCache bool - // frozenLockfile enforces that the devcontainer lockfile matches the - // resolved features, set from CLIOptions.FrozenLockfile. - frozenLockfile bool - // noLockfile disables the devcontainer lockfile entirely, set from - // CLIOptions.NoLockfile. - noLockfile bool + pull bool + noCache bool + frozenLockfile bool + noLockfile bool } // composeUpParams groups the inputs shared by extendedDockerComposeUp and diff --git a/pkg/devcontainer/feature/extend.go b/pkg/devcontainer/feature/extend.go index dc95441c7..edfaca7d3 100644 --- a/pkg/devcontainer/feature/extend.go +++ b/pkg/devcontainer/feature/extend.go @@ -59,11 +59,8 @@ type ExtendedBuildParams struct { DevContainerConfig *config.SubstitutedConfig ForceBuild bool SecretOpts *SecretOptions - // FrozenLockfile enforces that the devcontainer lockfile exists and matches - // the resolved features; the build fails instead of writing the lockfile. - FrozenLockfile bool - // NoLockfile disables the lockfile entirely: no pinning and no writing. - NoLockfile bool + FrozenLockfile bool + NoLockfile bool } func GetExtendedBuildInfo(params *ExtendedBuildParams) (*ExtendedBuildInfo, error) { diff --git a/pkg/devcontainer/feature/features.go b/pkg/devcontainer/feature/features.go index 762b6f6db..e54e96131 100644 --- a/pkg/devcontainer/feature/features.go +++ b/pkg/devcontainer/feature/features.go @@ -126,9 +126,7 @@ func maskSecretOptions(feature *config.FeatureConfig, options []string) []string } // ProcessFeatureID resolves a feature identifier to a local folder containing -// the extracted feature, without lockfile pinning. It is retained for callers -// that only need the folder; lockfile-aware resolution goes through -// featureProcessor.resolveFeatureSource. +// the extracted feature, without lockfile pinning. func ProcessFeatureID( id string, devContainerConfig *config.DevContainerConfig, @@ -219,8 +217,7 @@ func PullFeatureToTemp(ref name.Reference, id string) (string, error) { return featureExtractedFolder, nil } -// ociResolution is the outcome of resolving an OCI feature: the extracted -// folder, the canonical digest reference, and the manifest digest (integrity). +// ociResolution is the outcome of resolving an OCI feature. type ociResolution struct { folder string resolved string @@ -603,8 +600,7 @@ func extractTarball(downloadFile, dest string) error { } // processDirectTarFeature resolves an HTTP(S) tarball feature to a local folder -// and returns its integrity digest (sha256: of the tarball). When -// pinnedIntegrity is set the computed digest must match it. +// and returns its integrity digest (sha256: of the tarball). func processDirectTarFeature( id string, httpHeaders map[string]string, diff --git a/pkg/devcontainer/feature/lockfile.go b/pkg/devcontainer/feature/lockfile.go index d9a71d5aa..f4cc59a0b 100644 --- a/pkg/devcontainer/feature/lockfile.go +++ b/pkg/devcontainer/feature/lockfile.go @@ -33,10 +33,7 @@ var ( errLockfileMismatch = errors.New("lockfile does not match") ) -// LockfilePath returns the lockfile path for a devcontainer config origin, -// matching the reference CLI: a hidden config file (basename starting with a -// dot) yields a hidden lockfile, otherwise devcontainer-lock.json is used. -// It returns "" when the origin is unknown or not a local path. +// LockfilePath returns the lockfile path for a devcontainer config origin. func LockfilePath(configOrigin string) string { if configOrigin == "" || strings.Contains(configOrigin, "://") { return "" @@ -77,9 +74,7 @@ func ReadLockfile(path string) (*Lockfile, error) { return lf, nil } -// WriteLockfile persists the lockfile at path, matching reference-CLI -// semantics: it only rewrites when the normalized content changed, and when -// frozen it never writes, instead erroring if the file is missing or stale. +// WriteLockfile persists the lockfile at path. func WriteLockfile(path string, lf *Lockfile, frozen bool) error { if path == "" { return nil @@ -102,8 +97,6 @@ func WriteLockfile(path string, lf *Lockfile, frozen bool) error { return nil } -// lockfileNeedsWrite reports whether the lockfile at path must be rewritten, -// applying frozen-mode enforcement (error if missing or stale). func lockfileNeedsWrite(path string, newContent []byte, frozen bool) (bool, error) { existing, err := os.ReadFile(filepath.Clean(path)) switch { @@ -125,8 +118,6 @@ func lockfileNeedsWrite(path string, newContent []byte, frozen bool) (bool, erro } } -// marshalLockfile renders the lockfile with two-space indentation and a -// trailing newline, matching the reference CLI output. func marshalLockfile(lf *Lockfile) ([]byte, error) { data, err := json.MarshalIndent(lf, "", " ") if err != nil { @@ -135,9 +126,6 @@ func marshalLockfile(lf *Lockfile) ([]byte, error) { return append(data, '\n'), nil } -// normalizeLockfileJSON round-trips JSON so cosmetic differences (indentation, -// trailing whitespace) do not trigger a rewrite. Invalid input normalizes to -// nil, which forces a rewrite. func normalizeLockfileJSON(data []byte) []byte { var v any if err := json.Unmarshal(data, &v); err != nil { @@ -157,8 +145,7 @@ type lockfileMode struct { // frozen fails the build instead of writing when the lockfile is missing or // would change. frozen bool - // disabled turns the lockfile off entirely — no load, no pinning, no write - // (the --no-lockfile opt-out). + // disabled turns the lockfile off entirely disabled bool } @@ -191,7 +178,6 @@ func (l *lockfileState) record(featureID string, entry LockedFeature) { } // newLockfileState loads the lockfile for the given config to enable pinning. -// It always returns a usable state; a missing lockfile simply yields no pins. func newLockfileState(cfg *config.DevContainerConfig) *lockfileState { state := &lockfileState{entries: map[string]LockedFeature{}} loaded, err := ReadLockfile(LockfilePath(cfg.Origin)) @@ -204,8 +190,7 @@ func newLockfileState(cfg *config.DevContainerConfig) *lockfileState { } // checkFrozenPrecondition fails fast in frozen mode when the config declares -// features but no lockfile exists, avoiding wasted feature downloads before the -// commit-time enforcement would reject the build anyway. +// features but no lockfile exists. func (l *lockfileState) checkFrozenPrecondition(cfg *config.DevContainerConfig) error { if l == nil || len(cfg.Features) == 0 { return nil @@ -217,10 +202,6 @@ func (l *lockfileState) checkFrozenPrecondition(cfg *config.DevContainerConfig) return errLockfileMissing } -// commit writes the collected entries to the lockfile when write is requested. -// It regenerates the lockfile fully from the freshly resolved entries so that -// removed features drop out. To avoid creating spurious empty lockfiles, it -// skips writing when there are no entries and no lockfile already exists. func (l *lockfileState) commit(cfg *config.DevContainerConfig, mode lockfileMode) error { if l == nil || !mode.write { return nil From 05edc8aa239ad974412fb8ae43eaf9554f345c77 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Jul 2026 17:12:58 +0000 Subject: [PATCH 6/6] ci: free disk space to resolve runner storage flakiness Signed-off-by: GitHub --- .github/workflows/pr-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-ci.yml b/.github/workflows/pr-ci.yml index fbaff0e0c..2d9e44291 100644 --- a/.github/workflows/pr-ci.yml +++ b/.github/workflows/pr-ci.yml @@ -237,7 +237,7 @@ jobs: - label: provider runner: ubuntu-latest - free-disk-space: false + free-disk-space: true install-kind: true requires-secret: false