fix(ci,github-runner,k8s): NFC collision keys, manifest size cap, drop audit, runner group, restore detail (D-1130, D-1131, D-1132, D-1180, D-0632)

.github/wiki/plugins/K8s.md

@@ -331,7 +331,7 @@ unknown-flag envelope.
 | `backup` | `file`, `metadata_file`, `bytes`, `sha256`, `tables[]` (`schema`, `table`, `rows`) |
 | `restore` | `file`, `verified` (always `true`), `tables[]` (`schema`, `table`, `rows`) |
 
-Database error text never reaches an envelope: `cause` holds fixed text only. Human mode prints the first error line of `pg_dump` or `pg_restore` after the code, with every quoted part masked as `…`, `DETAIL`, `CONTEXT` and `HINT` dropped, at most 160 characters.
+Database error text never reaches an envelope: `cause` holds fixed text only. Human mode appends `[error category: …]` after the code: fixed words for the failure category of the first error line of `pg_dump` or `pg_restore` (for example `unique constraint violated`, `permission denied`), drawn from an allowlist, at most 160 bytes. None of the line's own text is shown; a line with no known category is reported as withheld.
 
 An error envelope carries `error.code`, `message`, optional `cause` and `remediation`, `exit_code` and `class`; the process exit
 status equals `exit_code`. Messages never carry a secret, the licence key or helm's release JSON. Without `--json` every

free/ci/go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/google/jsonschema-go v0.4.2
 	github.com/hashicorp/mdns v1.0.7
 	github.com/nself-org/cli/sdk/go/v2 v2.0.0-20261008225131-a4a60906574c
+	golang.org/x/text v0.42.0
 	gopkg.in/yaml.v3 v3.0.1
 	modernc.org/sqlite v1.60.1
 )

free/ci/go.sum

@@ -30,6 +30,8 @@ golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk=
 golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0=
 golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo=
 golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og=
+golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI=
+golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E=
 golang.org/x/tools v0.50.0 h1:c2ifzfcuY7L90lZ2aKd8S4K2NpASF08SZx9ZuJkHmSU=
 golang.org/x/tools v0.50.0/go.mod h1:7ulVMw3831Mwi5EZD6RomGyffr4VFjuNYXf2BbCEAV0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=

free/ci/internal/authz/extractor.go

@@ -73,8 +73,10 @@ func (x *Extractor) Admit(e Entry) (string, error) {
 // (or link target). Unicode-normalizing file systems (APFS, HFS+) give "e"+U+0301
 // and U+00E9 one directory, so a symlink placed under one spelling and a file
 // written under the other make the lexical depth that checkLink relies on
-// differ from the physical one. foldKey folds case only (no x/text in the
-// module), so the safe rule is no mixing of the two, in either order.
+// differ from the physical one. foldKey also NFC-normalizes, which stops two
+// plain files from aliasing; a symlink still cannot be judged against a
+// spelling the file system may rewrite, so the safe rule stays no mixing of
+// the two, in either order.
 func (x *Extractor) aliasRisk(e Entry) error {
 	link := e.Mode.Type() == fs.ModeSymlink
 	wide := !isASCII(e.Name) || (link && !isASCII(e.LinkTarget))

free/ci/internal/authz/hardening_test.go

@@ -1,6 +1,7 @@
 package authz_test
 
 import (
+	"errors"
 	"io/fs"
 	"math"
 	"testing"
@@ -49,6 +50,29 @@ func TestExtractor_UnicodeAliasWithSymlinkRefused(t *testing.T) {
 	}
 }
 
+// Two plain files whose names are canonically equivalent share one APFS entry.
+func TestExtractor_NormalizationCollisionRefused(t *testing.T) {
+	nfc, nfd := "d/\u00e9.txt", "d/e\u0301.txt"
+	for _, order := range [][]string{{nfc, nfd}, {nfd, nfc}} {
+		err := admitAll([]authz.Entry{dir("d/"), file(order[0]), file(order[1])}, authz.Limits{})
+		var pe *authz.PathError
+		if !errors.As(err, &pe) || pe.Reason != authz.CollisionFold {
+			t.Fatalf("%q then %q: want %s, got %v", order[0], order[1], authz.CollisionFold, err)
+		}
+	}
+	// children of two spellings of one directory share it
+	err := admitAll([]authz.Entry{file("\u00e9/a"), file("e\u0301/b")}, authz.Limits{})
+	if err != nil {
+		t.Fatalf("children of one normalized directory refused: %v", err)
+	}
+	if err := admitAll([]authz.Entry{file("\u00e9"), file("e\u0301/b")}, authz.Limits{}); err == nil {
+		t.Fatal("file used as directory under the other spelling admitted")
+	}
+	if err := admitAll([]authz.Entry{file("\u00e9.txt"), file("e.txt")}, authz.Limits{}); err != nil {
+		t.Fatalf("distinct names refused: %v", err)
+	}
+}
+
 func TestArtifactPath_DotGit(t *testing.T) {
 	for _, n := range []string{".git/config", "a/.GIT/hooks/x", ".git", "a/.git./x", "a/.git /x", "GIT~1/config", "x/.Git"} {
 		if _, err := authz.ArtifactPath(authz.Entry{Name: n, Mode: 0o644, Size: 1, Packed: 1}, authz.Limits{}); err == nil {

free/ci/internal/authz/path.go

@@ -19,6 +19,8 @@ import (
 	"strings"
 	"unicode"
 	"unicode/utf8"
+
+	"golang.org/x/text/unicode/norm"
 )
 
 // Entry is one archive header or collected file.
@@ -260,11 +262,13 @@ func checkLink(target string, dirDepth int, name string, lim Limits) error {
 	return nil
 }
 
-// foldKey maps a name to a case-insensitive identity: each rune becomes the
-// smallest member of its simple-fold orbit.
+// foldKey maps a name to a case- and normalization-insensitive identity: the
+// name is put in NFC (APFS and HFS+ give canonically equivalent spellings one
+// directory entry), then each rune becomes the smallest member of its
+// simple-fold orbit, then NFC again in case folding left a decomposed run.
 func foldKey(s string) string {
 	var b strings.Builder
-	for _, r := range s {
+	for _, r := range norm.NFC.String(s) {
 		f := r
 		for c := unicode.SimpleFold(r); c != r; c = unicode.SimpleFold(c) {
 			if c < f {
@@ -273,5 +277,5 @@ func foldKey(s string) string {
 		}
 		b.WriteRune(f)
 	}
-	return b.String()
+	return norm.NFC.String(b.String())
 }

free/ci/internal/cache/entries/lookup.go

@@ -100,7 +100,8 @@ func (s *Store) lookup(ctx context.Context, ns authz.Namespace, key string, acce
 		return Manifest{}, false, err
 	}
 	if reason != "" {
-		return Manifest{}, false, s.be.CacheDrop(ctx, m, reason)
+		_, err = s.be.CacheDrop(ctx, m, reason)
+		return Manifest{}, false, err
 	}
 	return m, true, s.be.CacheTouch(ctx, m.Namespace, key)
 }
@@ -135,8 +136,10 @@ func (s *Store) Verify(ctx context.Context) (VerifyReport, error) {
 			r.Checked++
 			var reason string
 			if reason, err = s.intact(m); err == nil && reason != "" && reason != "unstable" {
-				r.Dropped++
-				err = s.be.CacheDrop(ctx, m, reason)
+				var dropped bool
+				if dropped, err = s.be.CacheDrop(ctx, m, reason); dropped {
+					r.Dropped++
+				}
 			}
 			if err != nil {
 				break

free/ci/internal/cache/entries/lookup_test.go

@@ -1,6 +1,7 @@
 package entries_test
 
 import (
+	"context"
 	"errors"
 	"os"
 	"strings"
@@ -219,3 +220,26 @@ func TestLookupForRestoreChecksExistenceAndSizeOnly(t *testing.T) {
 		t.Fatalf("%d entries left, want the same-size one", e.count())
 	}
 }
+
+// lostDrop is a backend whose drop always loses the race against a replacement.
+type lostDrop struct{ entries.Backend }
+
+func (lostDrop) CacheDrop(context.Context, entries.Manifest, string) (bool, error) { return false, nil }
+
+// A drop that removed no row (the entry was replaced meanwhile) is not counted.
+func TestVerifyDoesNotCountADropThatRemovedNothing(t *testing.T) {
+	e := newEnv(t, envOpts{})
+	a := e.save("v1", trusted, "a", "aaaa")
+	if err := os.WriteFile(e.blobPath(a, 0), []byte("zzzz"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	es := entries.New(lostDrop{e.st}, e.blobs, entries.Options{
+		Clock:        func() time.Time { return time.Unix(0, e.clock.Load()) },
+		Getenv:       func(string) string { return "" },
+		DisableFloor: true,
+	})
+	rep, err := es.Verify(e.ctx)
+	if err != nil || rep.Checked != 1 || rep.Dropped != 0 || e.count() != 1 {
+		t.Fatalf("verify: %+v %v (%d left)", rep, err, e.count())
+	}
+}

free/ci/internal/cache/entries/manifest.go

@@ -96,12 +96,25 @@ func (m Manifest) validate(checkNS bool) error {
 // ErrPathOutside marks a manifest rejected for a blob path outside the entry.
 var ErrPathOutside = errors.New("entries: blob path outside the entry")
 
-// EncodeManifest validates and encodes m.
+// EncodeManifest validates and encodes m. It refuses a document that
+// DecodeManifest would reject as too large: JSON escaping can grow a path
+// (a control byte takes six bytes, a quote two), so the cap on count and path
+// length alone does not bound the encoded size.
 func EncodeManifest(m Manifest) ([]byte, error) {
 	if err := m.Validate(); err != nil {
 		return nil, err
 	}
-	return json.Marshal(m)
+	var buf bytes.Buffer
+	enc := json.NewEncoder(&buf)
+	enc.SetEscapeHTML(false) // "&" would cost six bytes for no safety gain: the document is never embedded in HTML
+	if err := enc.Encode(m); err != nil {
+		return nil, err
+	}
+	out := bytes.TrimSuffix(buf.Bytes(), []byte("\n"))
+	if len(out) > maxManifestBytes {
+		return nil, bad("encoded document larger than %d bytes", maxManifestBytes)
+	}
+	return out, nil
 }
 
 // DecodeManifest strictly decodes and validates one manifest document.

free/ci/internal/cache/entries/manifest_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -142,3 +143,37 @@ func TestDecodeManifestSizeLimitIsInclusive(t *testing.T) {
 		t.Fatalf("a document over the limit: %v", err)
 	}
 }
+
+// manifestOfPaths fills maxBlobs sorted, unique paths of maxPathBytes bytes whose
+// tail is fill.
+func manifestOfPaths(fill string) Manifest {
+	m := goodManifest()
+	m.Blobs = make([]Blob, maxBlobs)
+	for i := range m.Blobs {
+		p := fmt.Sprintf("%04d", i)
+		m.Blobs[i] = Blob{Path: p + strings.Repeat(fill, maxPathBytes-len(p)), SHA256: strings.Repeat("b", 64)}
+	}
+	return m
+}
+
+func TestEncodeManifestStaysDecodable(t *testing.T) {
+	// "&" must not be HTML-escaped to six bytes: 4096 x 1024 bytes of it would
+	// otherwise reach ~25 MiB, past the 8 MiB decode cap.
+	m := manifestOfPaths("&")
+	b, err := EncodeManifest(m)
+	if err != nil {
+		t.Fatalf("encode: %v", err)
+	}
+	if len(b) > maxManifestBytes {
+		t.Fatalf("encoded %d bytes, over the decode cap", len(b))
+	}
+	got, err := DecodeManifest(b)
+	if err != nil || !reflect.DeepEqual(got, m) {
+		t.Fatalf("round trip: %v", err)
+	}
+	// A control byte escapes to six bytes whatever the encoder flags: the
+	// encoder must refuse what the decoder would refuse.
+	if b, err := EncodeManifest(manifestOfPaths("\x01")); !errors.Is(err, ErrManifest) {
+		t.Fatalf("an over-cap document was encoded (%d bytes): %v", len(b), err)
+	}
+}

free/ci/internal/cache/entries/types.go

@@ -106,7 +106,9 @@ type Backend interface {
 	// CacheKeys lists the distinct keys of all entries.
 	CacheKeys(ctx context.Context) ([]string, error)
 	CacheTouch(ctx context.Context, ns []byte, key string) error
-	CacheDrop(ctx context.Context, m Manifest, reason string) error
+	// CacheDrop deletes m only if it is still the stored row and reports whether
+	// it removed one; a lost race removes nothing and leaves no audit row.
+	CacheDrop(ctx context.Context, m Manifest, reason string) (bool, error)
 	CachePrune(ctx context.Context, p PruneStep) (Pruned, error)
 	CacheRecordUse(ctx context.Context, u Use) error
 	CacheUses(ctx context.Context, attempt string) ([]Use, error)

free/ci/internal/cache/remote/coord.go

@@ -24,7 +24,7 @@ const StreamCache = "cache"
 
 type Ledger interface {
 	CacheEntries(context.Context, string) ([]entries.Manifest, error)
-	CacheDrop(context.Context, entries.Manifest, string) error
+	CacheDrop(context.Context, entries.Manifest, string) (bool, error)
 	AttemptStateEpoch(context.Context, string) (model.JobState, int64, error)
 	LeaseGet(context.Context, string) (store.RemoteLease, error)
 	CacheAuditOnce(context.Context, string, string, string) error

free/ci/internal/cache/remote/finalize.go

@@ -137,7 +137,7 @@ func (l *Lease) RetractUsing(ctx context.Context, ledger Ledger) error {
 		errs = append(errs, err)
 		for _, m := range rows {
 			if bytes.Equal(m.Namespace, committed.Namespace) && m.Writer.AttemptID == l.b.AttemptID && m.Writer.Epoch == l.b.Epoch {
-				if err := ledger.CacheDrop(ctx, m, "attempt_not_passed"); err != nil {
+				if _, err := ledger.CacheDrop(ctx, m, "attempt_not_passed"); err != nil {
 					errs = append(errs, err)
 					continue
 				}

free/ci/internal/cache/remote/plan_test.go

@@ -103,7 +103,9 @@ func (b failedPlanState) AttemptStateEpoch(context.Context, string) (model.JobSt
 	return "", 0, b.err
 }
 
-func (b *metadataPlanBackend) CacheDrop(context.Context, entries.Manifest, string) error { return nil }
+func (b *metadataPlanBackend) CacheDrop(context.Context, entries.Manifest, string) (bool, error) {
+	return false, nil
+}
 
 func TestBuildPlanReadOnlyProcessBindsNothingEvenWithSnapshotHit(t *testing.T) {
 	in := PlanInput{Facts: entries.Facts{Project: "proj", Trust: "owner", Protection: "protected"}, Isolation: "process", Image: "image@sha256:" + strings.Repeat("a", 64), Detected: []detect.Fact{{Kind: "stack", Name: "go", Confidence: detect.Certain}}}

free/ci/internal/cache/specs/d1039_test.go

@@ -86,7 +86,7 @@ func (b cancelAfterCheck) AttemptStateEpoch(ctx context.Context, id string) (mod
 
 // CacheDrop runs on a live context, so a drop that the restore wrongly attempts
 // after the cancel is not hidden by the cancelled context failing it.
-func (b cancelAfterCheck) CacheDrop(_ context.Context, m entries.Manifest, reason string) error {
+func (b cancelAfterCheck) CacheDrop(_ context.Context, m entries.Manifest, reason string) (bool, error) {
 	return b.Backend.CacheDrop(context.Background(), m, reason)
 }
 

free/ci/internal/cache/specs/engine.go

@@ -39,7 +39,7 @@ var ErrEmpty = errors.New("specs: the cache directories are empty")
 type Backend interface {
 	AttemptStateEpoch(ctx context.Context, id string) (model.JobState, int64, error)
 	CacheEntries(ctx context.Context, key string) ([]entries.Manifest, error)
-	CacheDrop(ctx context.Context, m entries.Manifest, reason string) error
+	CacheDrop(ctx context.Context, m entries.Manifest, reason string) (bool, error)
 	AppendAudit(ctx context.Context, actor, action, target, detail string) error
 }
 
@@ -97,7 +97,7 @@ func (e *Engine) Restore(ctx context.Context, base *os.Root, ns authz.Namespace,
 		return false, 0, ctx.Err()
 	}
 	if access == authz.ModeRWOwn { // a lower-trust reader never deletes a higher-trust entry, it only audits
-		_ = e.Backend.CacheDrop(ctx, m, "unrestorable")
+		_, _ = e.Backend.CacheDrop(ctx, m, "unrestorable")
 	}
 	_ = e.Backend.AppendAudit(ctx, "cache", "cache.restore.refused", key, "reason="+refusalKey(err))
 	return false, 0, fmt.Errorf("cache restore refused: %w", err)
@@ -123,7 +123,7 @@ func (e *Engine) writerPassed(ctx context.Context, m entries.Manifest) bool {
 	case state == "queued" || state == "leased" || state == "running" || state == "finalizing":
 		return false
 	}
-	_ = e.Backend.CacheDrop(ctx, m, "attempt_not_passed")
+	_, _ = e.Backend.CacheDrop(ctx, m, "attempt_not_passed")
 	return false
 }
 

free/ci/internal/cache/specs/finalize.go

@@ -89,7 +89,7 @@ func (s *Session) Retract(ctx context.Context) {
 		list, _ := s.in.Engine.Backend.CacheEntries(ctx, key)
 		for _, m := range list {
 			if bytes.Equal(m.Namespace, s.own.Canonical()) && m.Writer.AttemptID == s.in.Attempt {
-				_ = s.in.Engine.Backend.CacheDrop(ctx, m, "attempt_not_passed")
+				_, _ = s.in.Engine.Backend.CacheDrop(ctx, m, "attempt_not_passed")
 			}
 		}
 	}

free/ci/internal/store/cache_entries_read.go

@@ -97,14 +97,23 @@ func (s *Store) CacheTouch(ctx context.Context, ns []byte, key string) error {
 
 // CacheDrop deletes the entry m (only if it is still the row that was read: same
 // writer attempt, epoch and creation time) and appends the audit row in one
-// transaction.
-func (s *Store) CacheDrop(ctx context.Context, m entries.Manifest, reason string) error {
-	return s.write(ctx, func(tx *sql.Tx) error {
-		if err := execAll(ctx, tx, []any{"DELETE FROM cache_entry WHERE namespace=? AND cache_key=? AND attempt_id=? AND epoch=? AND created_at=?", m.Namespace, m.Key, m.Writer.AttemptID, m.Writer.Epoch, m.CreatedAt}); err != nil {
+// transaction. It reports whether a row was removed: when a newer entry already
+// replaced m, nothing is deleted, no audit row is written and dropped is false.
+func (s *Store) CacheDrop(ctx context.Context, m entries.Manifest, reason string) (dropped bool, err error) {
+	err = s.write(ctx, func(tx *sql.Tx) error {
+		dropped = false
+		res, err := tx.ExecContext(ctx, "DELETE FROM cache_entry WHERE namespace=? AND cache_key=? AND attempt_id=? AND epoch=? AND created_at=?", m.Namespace, m.Key, m.Writer.AttemptID, m.Writer.Epoch, m.CreatedAt)
+		if err != nil {
+			return err
+		}
+		n, err := res.RowsAffected()
+		if err != nil || n == 0 {
 			return err
 		}
+		dropped = true
 		return s.appendAudit(ctx, tx, "cache", "cache.entry.dropped", m.Key, "reason="+reason)
 	})
+	return dropped && err == nil, err
 }
 
 // CachePrune runs one prune step in a single transaction (all or nothing):

free/ci/internal/store/cache_entries_test.go

@@ -321,7 +321,7 @@ func TestCacheEntriesAreKeyedByNamespaceAndKey(t *testing.T) {
 		t.Fatalf("the two entries must keep their own blobs: %+v", by)
 	}
 	// dropping one leaves the other
-	if err = s.CacheDrop(ctx, by["second"], "corrupt"); err != nil {
+	if _, err = s.CacheDrop(ctx, by["second"], "corrupt"); err != nil {
 		t.Fatal(err)
 	}
 	if left, _ := s.CacheEntries(ctx, cacheKey); len(left) != 1 || left[0].Writer.AttemptID != "first" {
@@ -609,15 +609,22 @@ func TestCacheEntriesDropOnlyRemovesTheRowThatWasRead(t *testing.T) {
 		t.Fatalf("%v %v", first, err)
 	}
 	save("two")
-	if err = s.CacheDrop(ctx, first[0], "corrupt"); err != nil {
-		t.Fatal(err)
+	if dropped, err := s.CacheDrop(ctx, first[0], "corrupt"); err != nil || dropped {
+		t.Fatalf("a drop that matched no row: dropped=%v err=%v", dropped, err)
+	}
+	var audits int
+	if err := s.readers.QueryRow("SELECT count(*) FROM audit WHERE action='cache.entry.dropped'").Scan(&audits); err != nil || audits != 0 {
+		t.Fatalf("a drop that matched no row wrote %d audit rows (%v)", audits, err)
 	}
 	now, err := s.CacheEntries(ctx, cacheKey)
 	if err != nil || len(now) != 1 || now[0].Writer.AttemptID != "two" {
 		t.Fatalf("the replacement was dropped: %+v %v", now, err)
 	}
-	if err = s.CacheDrop(ctx, now[0], "corrupt"); err != nil {
-		t.Fatal(err)
+	if dropped, err := s.CacheDrop(ctx, now[0], "corrupt"); err != nil || !dropped {
+		t.Fatalf("the drop of the stored row: dropped=%v err=%v", dropped, err)
+	}
+	if err := s.readers.QueryRow("SELECT count(*) FROM audit WHERE action='cache.entry.dropped'").Scan(&audits); err != nil || audits != 1 {
+		t.Fatalf("a real drop wrote %d audit rows (%v)", audits, err)
 	}
 	if gone, _ := s.CacheEntries(ctx, cacheKey); len(gone) != 0 {
 		t.Fatal("the row that was read must be dropped")