ENG-1636: Detect re-published private-ingredient content in the depot

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()

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")
 	}

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.")
 	}

pkg/runtime/decrypt_test.go

@@ -226,6 +226,112 @@ 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 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

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,47 @@ 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
 }
 
+// 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.
+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) {

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 }
 }

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)
 	}

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
@@ -139,28 +144,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 artifact 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 +497,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 +532,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")
 		}
 	}