From ec31c35c3fe67f17ca00c5a831905af5415764f4 Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 24 Jun 2026 16:10:27 -0400 Subject: [PATCH 1/2] Detect re-published private content via depot checksum Private ingredients use a timeless, mutable pointer: the same artifact ID can point at different content over time. A depot keyed by ID alone would serve the old plaintext forever. Record the build-plan content checksum on each private depot entry and, in the cache-hit decision, re-fetch and re-deploy a private artifact whose stored checksum no longer matches the build plan, undeploying the stale plaintext first. The comparison is gated on the depot's Private flag, so public artifacts (which are content-addressed and immutable) and projects without private ingredients are unaffected. The artifact directory is also cleared before unpacking so a re-fetch replaces stale content rather than merging into it. ENG-1636 Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/runtime/decrypt_test.go | 60 +++++++++++++++++++++++++++++++++++++ pkg/runtime/depot.go | 26 ++++++++++++++-- pkg/runtime/setup.go | 32 ++++++++++++++++---- 3 files changed, 110 insertions(+), 8 deletions(-) diff --git a/pkg/runtime/decrypt_test.go b/pkg/runtime/decrypt_test.go index 075726f1c7..66dcb1a940 100644 --- a/pkg/runtime/decrypt_test.go +++ b/pkg/runtime/decrypt_test.go @@ -226,6 +226,66 @@ func TestPrivateArtifactSurvivesEviction(t *testing.T) { } } +func TestMarkPrivateStoresChecksum(t *testing.T) { + id := strfmt.UUID("11111111-1111-1111-1111-111111111111") + d := &depot{ + config: depotConfig{ + Deployments: map[strfmt.UUID][]deployment{}, + Cache: map[strfmt.UUID]*artifactInfo{id: {Size: 1}}, // pre-seeded so no on-disk size lookup + }, + depotPath: t.TempDir(), + artifacts: map[strfmt.UUID]struct{}{}, + } + + if err := d.MarkPrivate(id, "sha256:abc"); err != nil { + t.Fatalf("MarkPrivate: %v", err) + } + info := d.config.Cache[id] + if !info.Private { + t.Error("artifact was not marked private") + } + if info.Checksum != "sha256:abc" { + t.Errorf("stored checksum = %q, want sha256:abc", info.Checksum) + } +} + +func TestPrivateContentChanged(t *testing.T) { + const ( + privFresh = strfmt.UUID("aaaaaaaa-0000-0000-0000-000000000000") + privStale = strfmt.UUID("bbbbbbbb-0000-0000-0000-000000000000") + public = strfmt.UUID("cccccccc-0000-0000-0000-000000000000") + absent = strfmt.UUID("dddddddd-0000-0000-0000-000000000000") + ) + d := &depot{ + config: depotConfig{ + Cache: map[strfmt.UUID]*artifactInfo{ + privFresh: {Private: true, Checksum: "sha256:aaa"}, + privStale: {Private: true, Checksum: "sha256:old"}, + public: {Checksum: "sha256:pub"}, // not private + }, + }, + artifacts: map[strfmt.UUID]struct{}{}, + } + + cases := []struct { + name string + id strfmt.UUID + checksum string + want bool + }{ + {"fresh private matches", privFresh, "sha256:aaa", false}, + {"stale private mismatches", privStale, "sha256:new", true}, + {"public artifact is never stale", public, "sha256:different", false}, + {"absent entry", absent, "sha256:any", false}, + {"empty build-plan checksum does not churn", privStale, "", false}, + } + for _, tc := range cases { + if got := d.PrivateContentChanged(tc.id, tc.checksum); got != tc.want { + t.Errorf("%s: PrivateContentChanged = %v, want %v", tc.name, got, tc.want) + } + } +} + func exists(path string) bool { _, err := os.Stat(path) return err == nil diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index a558400442..f74c19f9f0 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -60,6 +60,9 @@ type artifactInfo struct { // For private decrypted artifacts. Private bool `json:"private,omitempty"` + // Checksum is the build-plan content checksum a private artifact was stored + // under, used to detect re-published content for a fixed artifact ID. + Checksum string `json:"checksum,omitempty"` id strfmt.UUID // for convenience when removing stale artifacts; should NOT have json tag } @@ -195,9 +198,11 @@ func (d *depot) Put(id strfmt.UUID) error { return nil } -// MarkPrivate flags an artifact as a decrypted private artifact, ensuring a -// cache entry exists for it. Private artifacts are exempt from stale removal. -func (d *depot) MarkPrivate(id strfmt.UUID) error { +// MarkPrivate flags an artifact as a decrypted private artifact and records the +// build-plan checksum its content was stored under, ensuring a cache entry +// exists for it. Private artifacts are exempt from stale removal, and the stored +// checksum lets the next update detect re-published content. +func (d *depot) MarkPrivate(id strfmt.UUID, checksum string) error { d.mapMutex.Lock() defer d.mapMutex.Unlock() @@ -209,9 +214,24 @@ func (d *depot) MarkPrivate(id strfmt.UUID) error { d.config.Cache[id] = &artifactInfo{Size: size, id: id} } d.config.Cache[id].Private = true + d.config.Cache[id].Checksum = checksum return nil } +// PrivateContentChanged reports whether the depot holds a private artifact for id +// whose stored content checksum differs from the given build-plan checksum — the +// timeless/re-published case where a fixed artifact ID now points at new content. +func (d *depot) PrivateContentChanged(id strfmt.UUID, checksum string) bool { + d.mapMutex.Lock() + defer d.mapMutex.Unlock() + + info, exists := d.config.Cache[id] + if !exists || !info.Private || checksum == "" { + return false + } + return info.Checksum != checksum +} + // DeployViaLink will take an artifact from the depot and link it to the target path. // It should return deployment info to be used for tracking the artifact. func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) (*deployment, error) { diff --git a/pkg/runtime/setup.go b/pkg/runtime/setup.go index 84023f47f3..c39047c1d1 100644 --- a/pkg/runtime/setup.go +++ b/pkg/runtime/setup.go @@ -139,28 +139,44 @@ func newSetup(path string, bp *buildplan.BuildPlan, env *envdef.Collection, depo // Start off with the full range of artifacts relevant to our platform installableArtifacts := bp.Artifacts(filterInstallable...) + installableArtifactsMap := installableArtifacts.ToIDMap() + + // Identify installed private artifacts whose depot content no longer matches + // the build plan: a private ingredient re-published under the same artifact ID. + statePrivateArtifacts := map[strfmt.UUID]bool{} + for id := range installedArtifacts { + a, required := installableArtifactsMap[id] + if required && depot.PrivateContentChanged(id, a.Checksum) { + logging.Debug("Private artifact %s content changed; re-fetching", id) + statePrivateArtifacts[id] = true + } + } - // Identify which artifacts we'll need to install, this filters out any artifacts that are already installed. + // Identify which artifacts we'll need to install. This filters out artifacts + // that are already installed, except stale private artifacts which must be + // re-installed with their new content. artifactsToInstall := installableArtifacts.Filter(func(a *buildplan.Artifact) bool { _, installed := installedArtifacts[a.ArtifactID] - return !installed + return !installed || statePrivateArtifacts[a.ArtifactID] }) // Identify which artifacts we can uninstall - installableArtifactsMap := installableArtifacts.ToIDMap() artifactsToUninstall := map[strfmt.UUID]bool{} for id := range installedArtifacts { if _, required := installableArtifactsMap[id]; !required { artifactsToUninstall[id] = true } } + for id := range statePrivateArtifacts { + artifactsToUninstall[id] = true + } // Calculate which artifacts need to be downloaded; if an artifact we want to install is not in our depot then // by definition we'll need to download it (unless we're setting up the runtime from an archive). // We also calculate which artifacts are immediately ready to be installed, as its the inverse condition of the above. artifactsToDownload := artifactsToInstall.Filter(func(a *buildplan.Artifact) bool { exists, _ := depot.Exists(a.ArtifactID) - return !exists + return !exists || statePrivateArtifacts[a.ArtifactID] }) artifactsToUnpack := artifactsToDownload if opts.FromArchive != nil { @@ -476,6 +492,12 @@ func (s *setup) unpack(artifact *buildplan.Artifact, b []byte) (rerr error) { }, }, bytes.NewReader(b)) unpackPath := s.depot.Path(artifact.ArtifactID) + if fileutils.DirExists(unpackPath) { + // Clear prior private artifact that is being updated. + if err := os.RemoveAll(unpackPath); err != nil { + return errs.Wrap(err, "could not clear private artifact directory") + } + } if err := ua.Unarchive(proxy, unpackPath); err != nil { if err2 := os.RemoveAll(unpackPath); err2 != nil { return errs.Pack(err, errs.Wrap(err2, "unable to remove partially-unpacked directory")) @@ -505,7 +527,7 @@ func (s *setup) unpack(artifact *buildplan.Artifact, b []byte) (rerr error) { } if outcome == decryptDone { logging.Debug("Decrypted private artifact %s (%s)", artifact.ArtifactID, artifact.Name()) - if err := s.depot.MarkPrivate(artifact.ArtifactID); err != nil { + if err := s.depot.MarkPrivate(artifact.ArtifactID, artifact.Checksum); err != nil { return errs.Wrap(err, "Could not mark decrypted artifact as private") } } From 2385a1d545b2ec1183cb9a810f239fe924532ab1 Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 24 Jun 2026 16:47:28 -0400 Subject: [PATCH 2/2] Re-check private artifact content on refresh and pull A private artifact re-published under the same artifact ID keeps the project's commit hash unchanged, so the hash short-circuits would report the runtime as up to date and never run the depot content check. When the runtime has private artifacts, state refresh and state pull now bypass those short-circuits (the NeedsUpdate gate, the runbits hash check, and the pkg/runtime hash check) to re-solve and re-check content. Routine commands keep the fast path, and projects without private artifacts are unaffected. Also renames the depot's HasPrivateArtifacts helper and standardizes comment terminology on "private artifact". ENG-1636 Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/runbits/runtime/runtime.go | 18 +++++++++-- internal/runners/pull/pull.go | 2 +- internal/runners/refresh/refresh.go | 14 +++++++-- pkg/runtime/decrypt_test.go | 46 +++++++++++++++++++++++++++++ pkg/runtime/depot.go | 23 +++++++++++++++ pkg/runtime/options.go | 6 ++++ pkg/runtime/runtime.go | 16 ++++++---- pkg/runtime/setup.go | 9 ++++-- pkg/runtime_helpers/helpers.go | 8 +++++ 9 files changed, 129 insertions(+), 13 deletions(-) diff --git a/internal/runbits/runtime/runtime.go b/internal/runbits/runtime/runtime.go index dcdf29b446..17085edd2e 100644 --- a/internal/runbits/runtime/runtime.go +++ b/internal/runbits/runtime/runtime.go @@ -57,6 +57,8 @@ type Opts struct { ValidateBuildscript bool IgnoreAsync bool + + CheckForPrivateArtifactUpdates bool } type SetOpt func(*Opts) @@ -105,6 +107,15 @@ func WithIgnoreAsync() SetOpt { } } +// WithCheckForPrivateArtifactUpdates re-checks private artifact content even when +// the commit hash is unchanged, so re-published private artifacts are picked +// up. +func WithCheckForPrivateArtifactUpdates() SetOpt { + return func(opts *Opts) { + opts.CheckForPrivateArtifactUpdates = true + } +} + type primeable interface { primer.Projecter primer.Auther @@ -186,7 +197,7 @@ func Update( prime.Output().Notice(output.Title(locale.T("install_runtime"))) } - if rt.Hash() == rtHash { + if rt.Hash() == rtHash && (!opts.CheckForPrivateArtifactUpdates || !rt.HasPrivateArtifacts()) { prime.Output().Notice(locale.T("pkg_already_uptodate")) return rt, nil } @@ -288,8 +299,11 @@ func Update( rtOpts = append(rtOpts, runtime.WithPortable()) } rtOpts = append(rtOpts, runtime.WithCacheSize(prime.Config().GetInt(constants.RuntimeCacheSizeConfigKey))) + if opts.CheckForPrivateArtifactUpdates { + rtOpts = append(rtOpts, runtime.WithCheckForPrivateArtifactUpdates()) + } - // Fetch the organization key for private ingredients, if a key service is configured. + // Fetch the organization key for private artifacts, if a key service is configured. orgKeyProvider := orgkey.New(prime.Config(), proj.Owner()) if orgKeyProvider.Configured() { defer orgKeyProvider.Close() diff --git a/internal/runners/pull/pull.go b/internal/runners/pull/pull.go index 4048e4097d..df7dfd3ef0 100644 --- a/internal/runners/pull/pull.go +++ b/internal/runners/pull/pull.go @@ -218,7 +218,7 @@ func (p *Pull) Run(params *PullParams) (rerr error) { }) } - _, err = runtime_runbit.Update(p.prime, trigger.TriggerPull) + _, err = runtime_runbit.Update(p.prime, trigger.TriggerPull, runtime_runbit.WithCheckForPrivateArtifactUpdates()) if err != nil { return locale.WrapError(err, "err_pull_refresh", "Could not refresh runtime after pull") } diff --git a/internal/runners/refresh/refresh.go b/internal/runners/refresh/refresh.go index 41a3e3c352..4f45a0f0e2 100644 --- a/internal/runners/refresh/refresh.go +++ b/internal/runners/refresh/refresh.go @@ -92,11 +92,19 @@ func (r *Refresh) Run(params *Params) error { } if !needsUpdate { - r.out.Notice(locale.T("refresh_runtime_uptodate")) - return nil + // Even when the commit hash is unchanged, a private artifact may have been + // re-published under the same artifact ID; those runtimes still need a refresh. + hasPrivate, err := runtime_helpers.HasPrivateArtifacts(proj) + if err != nil { + return errs.Wrap(err, "could not determine if runtime has private artifacts") + } + if !hasPrivate { + r.out.Notice(locale.T("refresh_runtime_uptodate")) + return nil + } } - rti, err := runtime_runbit.Update(r.prime, trigger.TriggerRefresh, runtime_runbit.WithoutHeaders(), runtime_runbit.WithIgnoreAsync()) + rti, err := runtime_runbit.Update(r.prime, trigger.TriggerRefresh, runtime_runbit.WithoutHeaders(), runtime_runbit.WithIgnoreAsync(), runtime_runbit.WithCheckForPrivateArtifactUpdates()) if err != nil { return locale.WrapError(err, "err_refresh_runtime_new", "Could not update runtime for this project.") } diff --git a/pkg/runtime/decrypt_test.go b/pkg/runtime/decrypt_test.go index 66dcb1a940..ab592a52cf 100644 --- a/pkg/runtime/decrypt_test.go +++ b/pkg/runtime/decrypt_test.go @@ -286,6 +286,52 @@ func TestPrivateContentChanged(t *testing.T) { } } +func TestHasPrivateArtifacts(t *testing.T) { + const ( + priv = strfmt.UUID("aaaaaaaa-0000-0000-0000-000000000000") + public = strfmt.UUID("bbbbbbbb-0000-0000-0000-000000000000") + ) + path := t.TempDir() + other := t.TempDir() + + newDepotWith := func(cache map[strfmt.UUID]*artifactInfo, deps map[strfmt.UUID][]deployment) *depot { + return &depot{ + config: depotConfig{Cache: cache, Deployments: deps}, + artifacts: map[strfmt.UUID]struct{}{}, + } + } + + t.Run("private deployed at path", func(t *testing.T) { + d := newDepotWith( + map[strfmt.UUID]*artifactInfo{priv: {Private: true}}, + map[strfmt.UUID][]deployment{priv: {{Path: path}}}, + ) + if !d.HasPrivateArtifacts(path) { + t.Error("expected the private deployment to be detected") + } + }) + + t.Run("only public deployed", func(t *testing.T) { + d := newDepotWith( + map[strfmt.UUID]*artifactInfo{public: {}}, + map[strfmt.UUID][]deployment{public: {{Path: path}}}, + ) + if d.HasPrivateArtifacts(path) { + t.Error("a public-only runtime should not report private deployments") + } + }) + + t.Run("private deployed only at another path", func(t *testing.T) { + d := newDepotWith( + map[strfmt.UUID]*artifactInfo{priv: {Private: true}}, + map[strfmt.UUID][]deployment{priv: {{Path: other}}}, + ) + if d.HasPrivateArtifacts(path) { + t.Error("a private deployment at another path should not match") + } + }) +} + func exists(path string) bool { _, err := os.Stat(path) return err == nil diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index f74c19f9f0..d44910a3cd 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -218,6 +218,29 @@ func (d *depot) MarkPrivate(id strfmt.UUID, checksum string) error { return nil } +// HasPrivateArtifacts reports whether any artifact deployed at path is a private +// artifact. A runtime with private artifacts cannot be trusted as up-to-date +// by commit hash alone, since the same artifact ID may point at re-published +// content. +func (d *depot) HasPrivateArtifacts(path string) bool { + d.mapMutex.Lock() + defer d.mapMutex.Unlock() + + resolved := fileutils.ResolvePathIfPossible(path) + for id, deploys := range d.config.Deployments { + info, ok := d.config.Cache[id] + if !ok || !info.Private { + continue + } + for _, dep := range deploys { + if fileutils.ResolvePathIfPossible(dep.Path) == resolved { + return true + } + } + } + return false +} + // PrivateContentChanged reports whether the depot holds a private artifact for id // whose stored content checksum differs from the given build-plan checksum — the // timeless/re-published case where a fixed artifact ID now points at new content. diff --git a/pkg/runtime/options.go b/pkg/runtime/options.go index 63431b74a7..53151bdf58 100644 --- a/pkg/runtime/options.go +++ b/pkg/runtime/options.go @@ -24,6 +24,12 @@ func WithDecryptionKey(key []byte, keyID string) SetOpt { } } +// WithCheckForPrivateArtifactUpdates makes the update re-check private artifact +// content even when the commit hash is unchanged (re-published content). +func WithCheckForPrivateArtifactUpdates() SetOpt { + return func(opts *Opts) { opts.CheckForPrivateArtifactUpdates = true } +} + func WithBuildlogFilePath(path string) SetOpt { return func(opts *Opts) { opts.BuildlogFilePath = path } } diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go index 5f4b255e31..3258251701 100644 --- a/pkg/runtime/runtime.go +++ b/pkg/runtime/runtime.go @@ -77,17 +77,23 @@ func (r *Runtime) HasCache() bool { return r.hash != "" } -func (r *Runtime) Update(bp *buildplan.BuildPlan, hash string, setOpts ...SetOpt) error { - if r.hash == hash { - logging.Debug("Runtime is already up to date") - return nil - } +// HasPrivateArtifacts reports whether this runtime has any private (decrypted) +// artifacts deployed, whose content may change under an unchanged commit hash. +func (r *Runtime) HasPrivateArtifacts() bool { + return r.depot.HasPrivateArtifacts(r.path) +} +func (r *Runtime) Update(bp *buildplan.BuildPlan, hash string, setOpts ...SetOpt) error { opts := &Opts{} for _, setOpt := range setOpts { setOpt(opts) } + if r.hash == hash && (!opts.CheckForPrivateArtifactUpdates || !r.HasPrivateArtifacts()) { + logging.Debug("Runtime is already up to date") + return nil + } + if opts.BuildlogFilePath == "" { opts.BuildlogFilePath = filepath.Join(r.path, configDir, buildLogFile) } diff --git a/pkg/runtime/setup.go b/pkg/runtime/setup.go index c39047c1d1..889148ff96 100644 --- a/pkg/runtime/setup.go +++ b/pkg/runtime/setup.go @@ -61,10 +61,15 @@ type Opts struct { // OrgKey is the organization AES-256 key used to decrypt private artifacts // during install, with OrgKeyID identifying which key it is. Both are empty - // when the runtime has no private ingredients. + // when the runtime has no private artifacts. OrgKey []byte OrgKeyID string + // CheckForPrivateArtifactUpdates makes the update re-solve and re-check + // content even when the commit hash is unchanged, so a private artifact + // re-published under the same artifact ID is detected. + CheckForPrivateArtifactUpdates bool + FromArchive *fromArchive // Annotations are used strictly to pass information for the purposes of analytics @@ -142,7 +147,7 @@ func newSetup(path string, bp *buildplan.BuildPlan, env *envdef.Collection, depo installableArtifactsMap := installableArtifacts.ToIDMap() // Identify installed private artifacts whose depot content no longer matches - // the build plan: a private ingredient re-published under the same artifact ID. + // the build plan: a private artifact re-published under the same artifact ID. statePrivateArtifacts := map[strfmt.UUID]bool{} for id := range installedArtifacts { a, required := installableArtifactsMap[id] diff --git a/pkg/runtime_helpers/helpers.go b/pkg/runtime_helpers/helpers.go index 3ecad8c694..79ba067b31 100644 --- a/pkg/runtime_helpers/helpers.go +++ b/pkg/runtime_helpers/helpers.go @@ -45,6 +45,14 @@ func NeedsUpdate(proj *project.Project, overrideCommitID *strfmt.UUID) (bool, er return hash != rt.Hash(), nil } +func HasPrivateArtifacts(proj *project.Project) (bool, error) { + rt, err := FromProject(proj) + if err != nil { + return false, errs.Wrap(err, "Could not obtain runtime") + } + return rt.HasPrivateArtifacts(), nil +} + func Hash(proj *project.Project, overrideCommitID *strfmt.UUID) (string, error) { var err error var commitID strfmt.UUID