amadan

carlos / platform Public

settings-1 final fixes: ratchet marks fail closed and never collide

One flat mark file per channel ("/" escaped as %2F), a mark must be a
regular file, and only ENOENT/ENOTDIR read as unmarked; any other error
refuses a digest-less pointer (Astra r1 #1, #5).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude dcab1cf4ad95cd5a53d242766e106459ed042520 parent bb343ec
2 files changed, +140 −12
  • internal/release/ratchet.go +46 −12
  • internal/release/ratchet_test.go +94 −0
diff --git a/internal/release/ratchet.go b/internal/release/ratchet.go
index 74d2d4d1a..3fb553378 100644
--- a/internal/release/ratchet.go
+++ b/internal/release/ratchet.go
@@ -3,12 +3,14 @@ package release
import (
"errors"
"fmt"
+ "io/fs"
"log"
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
+ "syscall"
"time"
)
@@ -37,7 +39,9 @@ func RequirePointerDigest(getenv func(string) string) bool {
// RootRatchetDir is root's marks (/opt/carlos/pointer-digests), and
// EdgeRatchetDir the sandboxed agent's and edge's, which cannot write /opt
-// (/data/pointer-digests). Each process writes its own and reads both.
+// (/data/pointer-digests). Each process writes its own and reads both. A mark
+// is one flat file per channel, <dir>/<acct>/<app>/<channel>.digests, with
+// every "/" in the channel written as "%2F" (see markPath).
func RootRatchetDir(optDir string) string { return filepath.Join(optDir, "pointer-digests") }
func EdgeRatchetDir(dataDir string) string { return filepath.Join(dataDir, "pointer-digests") }
@@ -65,10 +69,13 @@ func NewDigestRatchet(own string, also []string, require bool) *DigestRatchet {
return r
}
-// markPath is a channel's mark. The ".digests" suffix lets "canary" and
-// "canary/x" both be marked: the second needs a directory named canary.
+// markPath is a channel's mark: <dir>/<acct>/<app>/<channel>.digests, one
+// flat file, with every "/" in the channel escaped as "%2F". A channel name
+// never contains "%", so the encoding is collision-free: "canary" and
+// "canary/x", or "foo" and "foo.digests/bar", each get a file of their own
+// and never need the other's path to be a directory (Astra r1 #5).
func markPath(dir, acct, app, channel string) string {
- return filepath.Join(dir, acct, app, filepath.FromSlash(channel)+".digests")
+ return filepath.Join(dir, acct, app, strings.ReplaceAll(channel, "/", "%2F")+".digests")
}
// Admit is the ratchet's verdict on a verified pointer. A nil ratchet admits
@@ -77,7 +84,9 @@ func markPath(dir, acct, app, channel string) string {
// process refuses a digestless replay from that moment, and on disk before
// the pointer is admitted. A mark that cannot be persisted refuses the
// pointer (ErrRatchetUnpersisted): installing or extracting on it would leave
-// the box unprotected after a restart.
+// the box unprotected after a restart. A mark that cannot be read (anything
+// but a confirmed absence) refuses a pointer without digests: only a box that
+// knows it has never seen digests for the channel admits one (Astra r1 #1).
func (r *DigestRatchet) Admit(acct string, p *ChannelPointer) error {
if r == nil || p == nil {
return nil
@@ -88,30 +97,55 @@ func (r *DigestRatchet) Admit(acct string, p *ChannelPointer) error {
if r.require.Load() {
return fmt.Errorf("%w: %s/%s/%s, and %s is on", ErrPointerWithoutDigests, acct, p.App, p.Channel, RequirePointerDigestEnv)
}
- if r.marked(acct, p.App, p.Channel) {
+ marked, err := r.marked(acct, p.App, p.Channel)
+ if err != nil {
+ return fmt.Errorf("%w: %s/%s/%s, and this box cannot tell whether it has already verified one with digests for that channel: %w",
+ ErrPointerWithoutDigests, acct, p.App, p.Channel, err)
+ }
+ if marked {
return fmt.Errorf("%w: %s/%s/%s, and this box has already verified one with digests for that channel",
ErrPointerWithoutDigests, acct, p.App, p.Channel)
}
return nil
}
-func (r *DigestRatchet) marked(acct, app, channel string) bool {
+// marked reports whether any of the ratchet's directories holds the channel's
+// mark. Only a confirmed absence (ENOENT, or ENOTDIR on a path component)
+// counts as "not marked"; a mark must be a regular file, and any other answer
+// (EACCES, EIO, a directory where the mark belongs) is an error, which the
+// caller treats as fail-closed. A mark found in any directory wins over an
+// error in another, since both refuse.
+func (r *DigestRatchet) marked(acct, app, channel string) (bool, error) {
k := acct + "|" + app + "|" + channel
r.mu.Lock()
defer r.mu.Unlock()
if r.seen[k] {
- return true
+ return true, nil
}
+ var firstErr error
for _, dir := range append([]string{r.own}, r.also...) {
if dir == "" {
continue
}
- if _, err := os.Stat(markPath(dir, acct, app, channel)); err == nil {
+ path := markPath(dir, acct, app, channel)
+ fi, err := os.Stat(path)
+ switch {
+ case err == nil && fi.Mode().IsRegular():
r.seen[k] = true
- return true
+ return true, nil
+ case err == nil:
+ if firstErr == nil {
+ firstErr = fmt.Errorf("pointer digest mark %s is not a regular file", path)
+ }
+ case errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR):
+ // confirmed absent
+ default:
+ if firstErr == nil {
+ firstErr = err
+ }
}
}
- return false
+ return false, firstErr
}
func (r *DigestRatchet) note(acct, app, channel string) error {
@@ -123,7 +157,7 @@ func (r *DigestRatchet) note(acct, app, channel string) error {
return nil
}
path := markPath(r.own, acct, app, channel)
- if _, err := os.Stat(path); err == nil {
+ if fi, err := os.Stat(path); err == nil && fi.Mode().IsRegular() {
return nil
}
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
diff --git a/internal/release/ratchet_test.go b/internal/release/ratchet_test.go
index 5f0ef6926..cea6cf622 100644
--- a/internal/release/ratchet_test.go
+++ b/internal/release/ratchet_test.go
@@ -151,3 +151,97 @@ func TestSharedRatchetLookupAndAdmitAreRaceFree(t *testing.T) {
t.Fatalf("require, widened by a lookup, was lost: %v", err)
}
}
+
+// Astra r1 #5: "foo" and "foo.digests/bar" are both valid channel names; with
+// a suffix only on the last segment the first one's mark file is the path the
+// second needs as a directory. The flat, escaped mark gives each its own file,
+// in either order.
+func TestRatchetMarksCollidingChannelNamesInEitherOrder(t *testing.T) {
+ for _, order := range [][2]string{{"foo", "foo.digests/bar"}, {"foo.digests/bar", "foo"}} {
+ if !ValidChannel(order[0]) || !ValidChannel(order[1]) {
+ t.Fatalf("%q or %q is not a valid channel", order[0], order[1])
+ }
+ dir := t.TempDir()
+ r := NewDigestRatchet(dir, nil, false)
+ if err := r.Admit("bab", ptr(order[0], dA)); err != nil {
+ t.Fatalf("%v: marking %q: %v", order, order[0], err)
+ }
+ if err := NewDigestRatchet(dir, nil, false).Admit("bab", ptr(order[1], "")); err != nil {
+ t.Fatalf("%v: marking %q made %q read as marked: %v", order, order[0], order[1], err)
+ }
+ if err := r.Admit("bab", ptr(order[1], dA)); err != nil {
+ t.Fatalf("%v: %q could not be marked beside %q: %v", order, order[1], order[0], err)
+ }
+ for _, ch := range order {
+ if err := NewDigestRatchet(dir, nil, false).Admit("bab", ptr(ch, "")); !errors.Is(err, ErrPointerWithoutDigests) {
+ t.Fatalf("%v: %q not ratcheted after a restart: %v", order, ch, err)
+ }
+ }
+ }
+}
+
+// A mark must be a regular file: a directory where the mark belongs is not a
+// mark, and not an absence either. Writing the mark over it fails, so the
+// pointer with digests is refused as unpersisted rather than taking the
+// directory for a durable mark.
+func TestRatchetRefusesWhenTheMarkIsADirectory(t *testing.T) {
+ dir := t.TempDir()
+ if err := os.MkdirAll(markPath(dir, "bab", "km", "stable"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ err := NewDigestRatchet(dir, nil, false).Admit("bab", ptr("stable", ""))
+ if !errors.Is(err, ErrPointerWithoutDigests) {
+ t.Fatalf("a directory at the mark path admitted a legacy pointer: %v", err)
+ }
+ if err := NewDigestRatchet(dir, nil, false).Admit("bab", ptr("stable", dA)); !errors.Is(err, ErrRatchetUnpersisted) {
+ t.Fatalf("a directory at the mark path was taken for a durable mark: %v", err)
+ }
+}
+
+func TestRatchetMarkPathIsFlatAndEscaped(t *testing.T) {
+ got := markPath("/d", "bab", "km", "canary/x")
+ if want := filepath.Join("/d", "bab", "km", "canary%2Fx.digests"); got != want {
+ t.Fatalf("markPath = %q, want %q", got, want)
+ }
+}
+
+// Astra r1 #1: a mark directory this process cannot read is not "no mark".
+// A fresh ratchet (a restarted process, nothing in memory) must refuse a
+// legacy pointer whether the unreadable directory is its own or another
+// process's.
+func TestRatchetFailsClosedWhenAMarkDirCannotBeRead(t *testing.T) {
+ if os.Geteuid() == 0 {
+ t.Skip("root reads a 0000 directory")
+ }
+ for _, which := range []string{"own", "also"} {
+ t.Run(which, func(t *testing.T) {
+ locked, open := t.TempDir(), t.TempDir()
+ if err := NewDigestRatchet(locked, nil, false).Admit("bab", ptr("stable", dA)); err != nil {
+ t.Fatal(err)
+ }
+ acctDir := filepath.Join(locked, "bab")
+ if err := os.Chmod(acctDir, 0); err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { _ = os.Chmod(acctDir, 0o755) })
+ var r *DigestRatchet
+ if which == "own" {
+ r = NewDigestRatchet(locked, []string{open}, false)
+ } else {
+ r = NewDigestRatchet(open, []string{locked}, false)
+ }
+ if err := r.Admit("bab", ptr("stable", "")); !errors.Is(err, ErrPointerWithoutDigests) {
+ t.Fatalf("an unreadable %s mark dir admitted a legacy pointer: %v", which, err)
+ }
+ // Control: with the directory readable again, the same answer
+ // comes from the mark itself, and an unmarked channel elsewhere
+ // is still admitted.
+ if err := os.Chmod(acctDir, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := NewDigestRatchet(open, nil, false).Admit("bab", ptr("stable", "")); err != nil {
+ t.Fatalf("control: an empty readable dir refused a legacy pointer: %v", err)
+ }
+ })
+ }
+}