amadan
host: CARLOS_REGION_HA is on only for a ParseBool true; a box record already under this label for another instance is logged loudly
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+102
−22
internal/host/boxrecord.go+52 −14internal/host/boxrecord_test.go+50 −8
diff --git a/internal/host/boxrecord.go b/internal/host/boxrecord.go| index 9069d3524..c338b6a25 100644 |
| --- a/internal/host/boxrecord.go |
| +++ b/internal/host/boxrecord.go |
| @@ -3,12 +3,14 @@ package host |
| import ( |
| "context" |
| "encoding/base64" |
| + "encoding/json" |
| "errors" |
| "fmt" |
| "io" |
| "log" |
| "net/http" |
| "os" |
| + "strconv" |
| "strings" |
| "time" |
| @@ -16,14 +18,13 @@ import ( |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| -// regionHAEnabled is CARLOS_REGION_HA: on for any value but "" and "0". |
| -// Every region HA behaviour on a box is behind it (spec 1 §8 step 2). |
| +// regionHAEnabled is CARLOS_REGION_HA: on only for a value |
| +// strconv.ParseBool reads as true ("1", "t", "true" in any case); "", |
| +// "0", "false", "off", "no" and anything unparseable are off. Every region |
| +// HA behaviour on a box is behind it (spec 1 §8 step 2). |
| func regionHAEnabled(getenv func(string) string) bool { |
| - switch strings.TrimSpace(getenv("CARLOS_REGION_HA")) { |
| - case "", "0": |
| - return false |
| - } |
| - return true |
| + on, err := strconv.ParseBool(strings.TrimSpace(getenv("CARLOS_REGION_HA"))) |
| + return err == nil && on |
| } |
| const ( |
| @@ -93,22 +94,59 @@ func readIdentityRecord(client *http.Client, base, box string, now time.Time) (b |
| IdentitySignature: cleaned, WrittenAt: now.UTC().Format(time.RFC3339)}, nil |
| } |
| +// recordInstanceID is the instanceId in a record's identity document. It |
| +// does not check the signature: it only tells whether a record names this |
| +// box's instance. |
| +func recordInstanceID(rec boxid.Record) (string, error) { |
| + doc, err := base64.StdEncoding.DecodeString(rec.IdentityDocument) |
| + if err != nil { |
| + return "", errors.New("identity document is not base64") |
| + } |
| + var d struct { |
| + InstanceID string `json:"instanceId"` |
| + } |
| + if err := json.Unmarshal(doc, &d); err != nil || d.InstanceID == "" { |
| + return "", errors.New("identity document names no instance") |
| + } |
| + return d.InstanceID, nil |
| +} |
| + |
| // publishBoxRecord makes one attempt to publish this box's enrollment |
| // evidence. done is true once there is nothing left to retry: the record |
| // is in place, or a different one already holds this reference, which |
| -// only an operator can sort out and which is logged loudly. |
| +// only an operator can sort out and which is logged loudly. A record |
| +// already under this reference is never rewritten, but it is checked |
| +// against this box's own identity: one signed for another instance, or |
| +// one that cannot be read, is reported the same loud way. |
| func publishBoxRecord(ctx context.Context, s release.Store, box string, read func() (boxid.Record, error)) (done bool, err error) { |
| if !boxid.ValidBox(box) { |
| return true, fmt.Errorf("box reference %q is not a label; no enrollment evidence is published", box) |
| } |
| - if _, err := boxid.Load(ctx, s, box); err == nil { |
| - return true, nil // create-only: an existing record is never rewritten |
| - } else if !errors.Is(err, os.ErrNotExist) { |
| + existing, err := boxid.Load(ctx, s, box) |
| + if err != nil && !errors.Is(err, os.ErrNotExist) { |
| return false, err |
| } |
| - rec, err := read() |
| - if err != nil { |
| - return false, err |
| + rec, rerr := read() |
| + if rerr != nil { |
| + return false, rerr |
| + } |
| + if err == nil { |
| + var held boxid.Record |
| + if jerr := json.Unmarshal(existing, &held); jerr != nil { |
| + return true, fmt.Errorf("the record already under %s cannot be read: %v", box, jerr) |
| + } |
| + theirs, ierr := recordInstanceID(held) |
| + if ierr != nil { |
| + return true, fmt.Errorf("the record already under %s: %v", box, ierr) |
| + } |
| + mine, ierr := recordInstanceID(rec) |
| + if ierr != nil { |
| + return false, fmt.Errorf("this box's own identity: %v", ierr) |
| + } |
| + if theirs != mine { |
| + return true, fmt.Errorf("the record already under %s is signed for instance %s, this box is %s", box, theirs, mine) |
| + } |
| + return true, nil // create-only: an existing record is never rewritten |
| } |
| err = boxid.Publish(ctx, s, rec) |
| if errors.Is(err, release.ErrCASConflict) { |
diff --git a/internal/host/boxrecord_test.go b/internal/host/boxrecord_test.go| index 8d55ce309..d3eb1de40 100644 |
| --- a/internal/host/boxrecord_test.go |
| +++ b/internal/host/boxrecord_test.go |
| @@ -83,21 +83,59 @@ func TestReadIdentityRecordRefusesWhatIsNotAnAnswer(t *testing.T) { |
| } |
| } |
| +// identityRecord is a record whose document names instance; the |
| +// signature is not checked on this path. |
| +func identityRecord(instance, writtenAt string) boxid.Record { |
| + doc := base64.StdEncoding.EncodeToString([]byte(`{"accountId":"111122223333","region":"eu-west-1","instanceId":"` + instance + `"}`)) |
| + return boxid.Record{V: 1, Box: "annie-welding", IdentityDocument: doc, IdentitySignature: "c2ln", WrittenAt: writtenAt} |
| +} |
| + |
| func TestPublishBoxRecordWritesOnceAndNeverRewrites(t *testing.T) { |
| ctx, st := context.Background(), release.NewDirStore(t.TempDir()) |
| - reads := 0 |
| - read := func() (boxid.Record, error) { |
| - reads++ |
| - return boxid.Record{V: 1, Box: "annie-welding", IdentityDocument: "ZG9j", IdentitySignature: "c2ln", WrittenAt: "2026-10-06T09:00:00Z"}, nil |
| - } |
| + writtenAt := "2026-10-06T09:00:00Z" |
| + read := func() (boxid.Record, error) { return identityRecord("i-0abc", writtenAt), nil } |
| if done, err := publishBoxRecord(ctx, st, "annie-welding", read); !done || err != nil { |
| t.Fatalf("first publish: done=%v err=%v", done, err) |
| } |
| + first, err := boxid.Load(ctx, st, "annie-welding") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + writtenAt = "2026-10-06T10:00:00Z" // a later run reads a fresh record |
| if done, err := publishBoxRecord(ctx, st, "annie-welding", read); !done || err != nil { |
| t.Fatalf("second publish: done=%v err=%v", done, err) |
| } |
| - if reads != 1 { |
| - t.Fatalf("IMDS read %d times; an existing record must not be re-read or rewritten", reads) |
| + if again, _ := boxid.Load(ctx, st, "annie-welding"); string(again) != string(first) { |
| + t.Fatalf("an existing record was rewritten:\n%s\n%s", first, again) |
| + } |
| +} |
| + |
| +// Final review 3: a record already under this box's label that is signed |
| +// for another instance is someone else's claim on the reference. It is |
| +// never rewritten, and it is reported once, as done with an error, so the |
| +// loop logs it loudly. |
| +func TestPublishBoxRecordReportsARecordForAnotherInstance(t *testing.T) { |
| + ctx, st := context.Background(), release.NewDirStore(t.TempDir()) |
| + if err := boxid.Publish(ctx, st, identityRecord("i-0zzz", "2026-10-06T08:00:00Z")); err != nil { |
| + t.Fatal(err) |
| + } |
| + read := func() (boxid.Record, error) { return identityRecord("i-0abc", "2026-10-06T09:00:00Z"), nil } |
| + done, err := publishBoxRecord(ctx, st, "annie-welding", read) |
| + if !done || err == nil || !strings.Contains(err.Error(), "i-0zzz") || !strings.Contains(err.Error(), "i-0abc") { |
| + t.Fatalf("done=%v err=%v; want done with an error naming both instances", done, err) |
| + } |
| + // An existing record this box cannot decode is reported the same way. |
| + st2 := release.NewDirStore(t.TempDir()) |
| + if _, err := st2.Put(ctx, boxid.Key("annie-welding"), []byte(`{"v":1}`), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if done, err := publishBoxRecord(ctx, st2, "annie-welding", read); !done || err == nil { |
| + t.Fatalf("an unreadable existing record: done=%v err=%v", done, err) |
| + } |
| + // Until this box can read its own identity, it retries. |
| + failing := func() (boxid.Record, error) { return boxid.Record{}, errors.New("IMDS down") } |
| + if done, err := publishBoxRecord(ctx, st, "annie-welding", failing); done || err == nil { |
| + t.Fatalf("own identity unreadable: done=%v err=%v; want a retry", done, err) |
| } |
| } |
| @@ -133,7 +171,11 @@ func TestPublishBoxRecordDoesNotPublishWhenItCannotRead(t *testing.T) { |
| } |
| func TestRegionHAEnabled(t *testing.T) { |
| - for v, want := range map[string]bool{"": false, "0": false, " 0 ": false, "1": true, "yes": true} { |
| + // Final review 5: only what strconv.ParseBool reads as true turns it on. |
| + for v, want := range map[string]bool{ |
| + "": false, "0": false, " 0 ": false, "false": false, "FALSE": false, "off": false, "no": false, "yes": false, "garbage": false, |
| + "1": true, " 1 ": true, "true": true, "TRUE": true, "t": true, |
| + } { |
| if got := regionHAEnabled(func(string) string { return v }); got != want { |
| t.Errorf("CARLOS_REGION_HA=%q: %v, want %v", v, got, want) |
| } |