amadan

carlos / platform Public

host: system apply never blocks on a planted FIFO; refusals are typed

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QCUCpfxbCyEAVQ2m51dH5
Paul Campbell pushed by paul@keymail.dev 29414924c6c3fb7ff793b3ab517f0b88bab3dc21 parent 21bc5d1
8 files changed, +159 −32
  • internal/host/systemapply.go +38 −9
  • internal/host/systemapply_test.go +7 −4
  • internal/host/systemapply_unix.go +7 −3
  • internal/host/systemapply_unix_test.go +63 −0
  • internal/host/systemapply_windows.go +4 −7
  • internal/host/systemstate.go +13 −2
  • internal/release/system.go +21 −3
  • internal/release/system_test.go +6 −4
diff --git a/internal/host/systemapply.go b/internal/host/systemapply.go
index 0d46a97fe..df6995191 100644
--- a/internal/host/systemapply.go
+++ b/internal/host/systemapply.go
@@ -45,6 +45,11 @@ var (
errSystemRefused = errors.New("refused")
errSystemDeferred = errors.New("deferred")
errNotYetImplemented = errors.New("system apply: install path not implemented")
+
+ // errSystemUnsupported is what the non-linux build's primitives return.
+ // It is declared here so Run can tell it from a lock another apply holds
+ // without knowing which build it is in.
+ errSystemUnsupported = errors.New("carlos system apply: unsupported on this platform")
)
// sysOps is the seam between the apply and the box: systemctl, exec with a
@@ -95,6 +100,10 @@ func (a *systemApplier) Run(ctx context.Context, recover bool) error {
return err
}
} else if err := flockExclusiveNonblock(lock); err != nil {
+ if errors.Is(err, errSystemUnsupported) {
+ a.ops.logf("system apply: %v", err)
+ return err
+ }
a.ops.logf("system apply: another apply is running; leaving it to finish")
return nil
}
@@ -137,7 +146,7 @@ func (a *systemApplier) Run(ctx context.Context, recover bool) error {
if req.NoOp || p.Current.Version == a.onDiskBuild(ctx) {
return a.writeResult(req, systemOutcomeNoted, "")
}
- staged, err := a.stageCandidate(p, req)
+ staged, err := a.stageCandidate(ctx, p, req)
if err != nil {
return a.record(req, fmt.Errorf("%w: %v", errSystemRefused, err))
}
@@ -187,6 +196,8 @@ func (a *systemApplier) readRequest() (systemRequest, error) {
if err != nil {
return systemRequest{}, err
}
+ // Regular-file only: a FIFO planted here is refused, having already been
+ // opened non-blocking so the open itself could not park root.
if !st.Mode().IsRegular() || st.Size() > systemMaxRequest {
return systemRequest{}, fmt.Errorf("request: not a regular file under %d bytes", systemMaxRequest)
}
@@ -213,7 +224,7 @@ func (a *systemApplier) verifyPointer(ctx context.Context) (*release.SystemPoint
return p, nil
case errors.Is(err, os.ErrNotExist):
return nil, fmt.Errorf("%w: no %s pointer for this box's channel", errSystemRefused, a.channel)
- case strings.Contains(err.Error(), "system pointer"):
+ case errors.Is(err, release.ErrBadSystemPointer):
return nil, fmt.Errorf("%w: %v", errSystemRefused, err)
default:
return nil, fmt.Errorf("%w: %v", errSystemDeferred, err)
@@ -227,13 +238,24 @@ func (a *systemApplier) onDiskBuild(ctx context.Context) string {
if err != nil {
return ""
}
- return strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(out), "carlos "))
+ return carlosVersionOf(out)
+}
+
+// carlosVersionOf reads `carlos version` output — exactly "carlos <build>",
+// nothing else. A build that merely starts with the expected version, or
+// output with anything else on the line, yields "" and matches nothing.
+func carlosVersionOf(out string) string {
+ v, ok := strings.CutPrefix(strings.TrimSpace(out), "carlos ")
+ if !ok {
+ return ""
+ }
+ return v
}
// stageCandidate copies the agent's cache into staging/ (0700) as 0600,
// verifies the copy's sha against the pointer, makes it executable, and
// runs its `version`. Nothing under /data is read after the copy.
-func (a *systemApplier) stageCandidate(p *release.SystemPointer, req systemRequest) (string, error) {
+func (a *systemApplier) stageCandidate(ctx context.Context, p *release.SystemPointer, req systemRequest) (string, error) {
art, ok := p.Current.Artifacts[a.platform]
if !ok {
return "", fmt.Errorf("pointer names no artifact for %s", a.platform)
@@ -245,11 +267,13 @@ func (a *systemApplier) stageCandidate(p *release.SystemPointer, req systemReque
if err := os.Chmod(a.stagingDir(), 0o700); err != nil {
return "", err
}
- src, err := openBeneath(a.dataDir, "system", systemComponent, req.Version, "carlos")
+ src, err := openBeneath(a.dataDir, systemCacheParts(req.Version)...)
if err != nil {
return "", fmt.Errorf("candidate: %w", err)
}
defer src.Close()
+ // Regular-file only: a FIFO or device the sandboxed user planted at the
+ // cache path is refused here, having already been opened non-blocking.
if st, err := src.Stat(); err != nil || !st.Mode().IsRegular() {
return "", errors.New("candidate: not a regular file")
}
@@ -260,9 +284,11 @@ func (a *systemApplier) stageCandidate(p *release.SystemPointer, req systemReque
}
h := sha256.New()
n, err := io.Copy(io.MultiWriter(dst, h), io.LimitReader(src, systemArtifactLimit+1))
- dst.Close()
+ if cerr := dst.Close(); err == nil {
+ err = cerr // a close that fails is a copy that did not land
+ }
if err != nil {
- return "", err
+ return "", fmt.Errorf("candidate: copying into staging: %w", err)
}
if n > systemArtifactLimit {
return "", fmt.Errorf("candidate: larger than %d bytes", systemArtifactLimit)
@@ -273,8 +299,11 @@ func (a *systemApplier) stageCandidate(p *release.SystemPointer, req systemReque
if err := os.Chmod(staged, 0o755); err != nil {
return "", err
}
- out, err := a.ops.run(context.Background(), systemVersionWait, staged, "version")
- if err != nil || !strings.Contains(out, "carlos "+p.Current.Version) {
+ out, err := a.ops.run(ctx, systemVersionWait, staged, "version")
+ if err != nil {
+ return "", fmt.Errorf("candidate: running its version: %w", err)
+ }
+ if carlosVersionOf(out) != p.Current.Version {
return "", fmt.Errorf("candidate reports %q, not %s", strings.TrimSpace(out), p.Current.Version)
}
return staged, nil
diff --git a/internal/host/systemapply_test.go b/internal/host/systemapply_test.go
index 26ffb937d..3a5aca1d8 100644
--- a/internal/host/systemapply_test.go
+++ b/internal/host/systemapply_test.go
@@ -319,7 +319,7 @@ func TestApplyStagesTheCandidateFromARootOwnedCopy(t *testing.T) {
t.Fatal(err)
}
req, _ := f.a.readRequest()
- staged, err := f.a.stageCandidate(p, req)
+ staged, err := f.a.stageCandidate(context.Background(), p, req)
if err != nil {
t.Fatal(err)
}
@@ -343,16 +343,19 @@ func TestApplyStagesTheCandidateFromARootOwnedCopy(t *testing.T) {
os.WriteFile(systemCachePath(f2.a.dataDir, "v2"), []byte("evil"), 0o644)
p2, _ := f2.a.verifyPointer(context.Background())
req2, _ := f2.a.readRequest()
- if _, err := f2.a.stageCandidate(p2, req2); err == nil || !strings.Contains(err.Error(), "sha256") {
+ if _, err := f2.a.stageCandidate(context.Background(), p2, req2); err == nil || !strings.Contains(err.Error(), "sha256") {
t.Fatalf("err = %v", err)
}
+ if st, err := os.Stat(filepath.Join(f2.a.stagingDir(), "carlos")); err == nil && st.Mode().Perm() != 0o600 {
+ t.Fatalf("an unverified copy was made executable: mode = %v", st.Mode().Perm())
+ }
// A candidate that lies about its version is refused.
f3 := newApplyFixture(t, "stable", "v1")
f3.promote("v2", body, false)
f3.answer["version:"+filepath.Join(f3.a.stagingDir(), "carlos")] = "carlos v9\n"
p3, _ := f3.a.verifyPointer(context.Background())
req3, _ := f3.a.readRequest()
- if _, err := f3.a.stageCandidate(p3, req3); err == nil || !strings.Contains(err.Error(), "reports") {
+ if _, err := f3.a.stageCandidate(context.Background(), p3, req3); err == nil || !strings.Contains(err.Error(), "reports") {
t.Fatalf("err = %v", err)
}
// A symlinked cache file is refused.
@@ -363,7 +366,7 @@ func TestApplyStagesTheCandidateFromARootOwnedCopy(t *testing.T) {
os.Symlink(filepath.Join(f4.a.dataDir, "real"), systemCachePath(f4.a.dataDir, "v2"))
p4, _ := f4.a.verifyPointer(context.Background())
req4, _ := f4.a.readRequest()
- if _, err := f4.a.stageCandidate(p4, req4); err == nil {
+ if _, err := f4.a.stageCandidate(context.Background(), p4, req4); err == nil {
t.Fatal("followed a symlinked cache file")
}
}
diff --git a/internal/host/systemapply_unix.go b/internal/host/systemapply_unix.go
index d906a1116..50efc63c7 100644
--- a/internal/host/systemapply_unix.go
+++ b/internal/host/systemapply_unix.go
@@ -15,14 +15,18 @@ import (
// openBeneath opens root/parts... one component at a time with
// O_DIRECTORY|O_NOFOLLOW for every directory and O_NOFOLLOW for the leaf,
-// so no component under a user-owned tree can be a symlink.
+// so no component under a user-owned tree can be a symlink. O_NONBLOCK is
+// on every open: it is a no-op for regular files and directories, and it is
+// what stops a FIFO planted at the leaf by the sandboxed user from parking
+// root inside open(2) forever while it holds the apply lock. The callers'
+// IsRegular() check then refuses the FIFO.
func openBeneath(root string, parts ...string) (*os.File, error) {
- fd, err := syscall.Open(root, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_CLOEXEC, 0)
+ fd, err := syscall.Open(root, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NONBLOCK|syscall.O_CLOEXEC, 0)
if err != nil {
return nil, fmt.Errorf("open %s: %w", root, err)
}
for i, part := range parts {
- flags := syscall.O_RDONLY | syscall.O_NOFOLLOW | syscall.O_CLOEXEC
+ flags := syscall.O_RDONLY | syscall.O_NOFOLLOW | syscall.O_NONBLOCK | syscall.O_CLOEXEC
if i < len(parts)-1 {
flags |= syscall.O_DIRECTORY
}
diff --git a/internal/host/systemapply_unix_test.go b/internal/host/systemapply_unix_test.go
new file mode 100644
index 000000000..9895f89dc
--- /dev/null
+++ b/internal/host/systemapply_unix_test.go
@@ -0,0 +1,63 @@
+//go:build !windows
+
+package host
+
+import (
+ "context"
+ "os"
+ "strings"
+ "syscall"
+ "testing"
+ "time"
+)
+
+// The sandboxed agent owns /data. A FIFO planted where root expects a
+// regular file would park root inside open(2) — while it holds the apply
+// lock, wedging every future apply on the box. Both reads under /data must
+// come back, refusing, in no time at all.
+func TestApplyNeverBlocksOnAPlantedFIFO(t *testing.T) {
+ within := func(t *testing.T, what string, fn func() error) {
+ t.Helper()
+ done := make(chan error, 1)
+ go func() { done <- fn() }()
+ select {
+ case err := <-done:
+ if err == nil {
+ t.Fatalf("%s: a FIFO was accepted", what)
+ }
+ if !strings.Contains(err.Error(), "regular file") {
+ t.Fatalf("%s: err = %v, want the not-a-regular-file refusal", what, err)
+ }
+ case <-time.After(2 * time.Second):
+ t.Fatalf("%s: parked on a FIFO — root would hold the apply lock forever", what)
+ }
+ }
+
+ // The request.
+ f := newApplyFixture(t, "stable", "v1")
+ os.MkdirAll(systemDataDir(f.a.dataDir), 0o755)
+ if err := syscall.Mkfifo(systemRequestPath(f.a.dataDir), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ within(t, "readRequest", func() error { _, err := f.a.readRequest(); return err })
+
+ // The artifact cache.
+ f2 := newApplyFixture(t, "stable", "v1")
+ f2.promote("v2", []byte("#!/bin/sh\necho carlos v2\n"), false)
+ os.Remove(systemCachePath(f2.a.dataDir, "v2"))
+ if err := syscall.Mkfifo(systemCachePath(f2.a.dataDir, "v2"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ p, err := f2.a.verifyPointer(context.Background())
+ if err != nil {
+ t.Fatal(err)
+ }
+ req, err := f2.a.readRequest()
+ if err != nil {
+ t.Fatal(err)
+ }
+ within(t, "stageCandidate", func() error {
+ _, err := f2.a.stageCandidate(context.Background(), p, req)
+ return err
+ })
+}
diff --git a/internal/host/systemapply_windows.go b/internal/host/systemapply_windows.go
index 1dec23f4d..f28035db3 100644
--- a/internal/host/systemapply_windows.go
+++ b/internal/host/systemapply_windows.go
@@ -2,17 +2,14 @@
package host
-import (
- "errors"
- "os"
-)
+import "os"
// The platform self-update is a root oneshot on a Linux box; the Windows
// build exists only so the carlos CLI compiles for workstation use. Both
// primitives refuse rather than pretend: an apply that cannot lock, or that
-// cannot open a path without following symlinks, must do nothing at all.
-
-var errSystemUnsupported = errors.New("carlos system apply: unsupported on this platform")
+// cannot open a path without following symlinks, must do nothing at all —
+// and Run tells this refusal apart from a lock another apply is holding, so
+// an unsupported platform is an error rather than a quiet no-op.
func openBeneath(root string, parts ...string) (*os.File, error) {
return nil, errSystemUnsupported
diff --git a/internal/host/systemstate.go b/internal/host/systemstate.go
index 560c28e3f..1bef0ebe9 100644
--- a/internal/host/systemstate.go
+++ b/internal/host/systemstate.go
@@ -110,8 +110,16 @@ func systemDataDir(dd string) string { return filepath.Join(dd, "system", system
func systemRequestPath(dd string) string {
return filepath.Join(systemDataDir(dd), "requested.json")
}
+
+// systemCacheParts are the components of the agent's cache entry below the
+// data dir. Root walks them one at a time (openBeneath) rather than opening
+// the joined path, so the two must come from here and never drift apart.
+func systemCacheParts(version string) []string {
+ return []string{"system", systemComponent, version, "carlos"}
+}
+
func systemCachePath(dd, version string) string {
- return filepath.Join(systemDataDir(dd), version, "carlos")
+ return filepath.Join(append([]string{dd}, systemCacheParts(version)...)...)
}
func systemRootDir(optDir string) string { return filepath.Join(optDir, "system", systemComponent) }
func systemPrevPath(optDir string) string { return filepath.Join(optDir, "carlos.prev") }
@@ -186,7 +194,10 @@ func readSystemSeen(optDir string) (systemSeen, error) {
s := systemSeen{}
err := readJSONFile(filepath.Join(systemRootDir(optDir), "seen.json"), &s)
if errors.Is(err, os.ErrNotExist) {
- return s, nil
+ return systemSeen{}, nil
+ }
+ if s == nil {
+ s = systemSeen{} // a literal `null` on disk unmarshals to a nil map
}
return s, err
}
diff --git a/internal/release/system.go b/internal/release/system.go
index d4a8529e7..88c60b18d 100644
--- a/internal/release/system.go
+++ b/internal/release/system.go
@@ -54,6 +54,14 @@ const SystemComponentRouter = "router"
// moved: there is nothing older to go back to.
var ErrNoSystemHistory = errors.New("system pointer: nothing to roll back to")
+// ErrBadSystemPointer marks every way an object on the wire can fail to be
+// this component and channel's signed pointer: a torn wrapper, a grant, a
+// signature that does not verify, a payload that does not validate. Callers
+// that must tell "this is not authority" from "the bucket is unreachable" —
+// the box's self-update refuses on the first and defers on the second — test
+// with errors.Is instead of reading the message.
+var ErrBadSystemPointer = errors.New("system pointer does not verify")
+
const (
systemSpreadMax = 24 * time.Hour
systemSpreadDefault = 5 * time.Minute
@@ -166,10 +174,20 @@ func signSystemPointer(sg Signer, p *SystemPointer) ([]byte, error) {
return json.MarshalIndent(signedWrapper{Payload: b64, Sig: base64.StdEncoding.EncodeToString(sig)}, "", " ")
}
-// verifySystemPointer is the single parse site for system pointers. A grant
-// on the wire is refused before the signature is looked at (the production
-// flag's rule, production.go).
+// verifySystemPointer is the single parse site for system pointers, and so
+// the one place ErrBadSystemPointer is attached: every refusal below reaches
+// the caller wrapped, whatever its own wording.
func verifySystemPointer(pub ed25519.PublicKey, wrapper []byte, wantComponent, wantChannel string) (*SystemPointer, error) {
+ p, err := parseSystemPointer(pub, wrapper, wantComponent, wantChannel)
+ if err != nil {
+ return nil, fmt.Errorf("%w: %v", ErrBadSystemPointer, err)
+ }
+ return p, nil
+}
+
+// parseSystemPointer does the work. A grant on the wire is refused before
+// the signature is looked at (the production flag's rule, production.go).
+func parseSystemPointer(pub ed25519.PublicKey, wrapper []byte, wantComponent, wantChannel string) (*SystemPointer, error) {
var sw signedWrapper
if err := json.Unmarshal(wrapper, &sw); err != nil {
return nil, fmt.Errorf("system pointer %s/%s: %w", wantComponent, wantChannel, err)
diff --git a/internal/release/system_test.go b/internal/release/system_test.go
index dcb2a9b2b..bc5ee372b 100644
--- a/internal/release/system_test.go
+++ b/internal/release/system_test.go
@@ -148,8 +148,10 @@ func TestSystemPointerTamperAndCrossChannelRefused(t *testing.T) {
if _, err := s.Put(ctx, SystemPointerKey("router", "stable"), body, ""); err != nil {
t.Fatal(err)
}
- if _, _, err := GetSystemPointer(ctx, s, pub, "router", "stable"); err == nil {
- t.Fatal("a canary pointer verified as stable")
+ // Refusals are typed, not stringly: a box tells "not authority" (refuse)
+ // from "the bucket is unreachable" (defer) with errors.Is.
+ if _, _, err := GetSystemPointer(ctx, s, pub, "router", "stable"); err == nil || !errors.Is(err, ErrBadSystemPointer) {
+ t.Fatalf("a canary pointer verified as stable: err = %v", err)
}
// Tampered signature.
var sw signedWrapper
@@ -160,8 +162,8 @@ func TestSystemPointerTamperAndCrossChannelRefused(t *testing.T) {
if _, err := s.Put(ctx, SystemPointerKey("router", "canary"), bad, etag); err != nil {
t.Fatal(err)
}
- if _, _, err := GetSystemPointer(ctx, s, pub, "router", "canary"); err == nil {
- t.Fatal("tampered signature verified")
+ if _, _, err := GetSystemPointer(ctx, s, pub, "router", "canary"); err == nil || !errors.Is(err, ErrBadSystemPointer) {
+ t.Fatalf("tampered signature verified: err = %v", err)
}
}