carlos / platform Public

region HA 2b: bound the membership read; status overrides and the alert webhook off the host.env allowlist

Final-review fix wave (Astra, CODEX-FINAL-REVIEW-REGION-HA-2B.md).

F1 (P2): confirmMembership gives each Member call its own
storeCallTimeout (15 s) context. A read cut off by the timeout or by
the pass's context is ErrMembershipUnknown whatever it returned: the
attempt stands, its wait does not grow, and the pass goes on to the
next box. TestAStalledMembershipReadIsCutOffAndThePassGoesOn (fails
on the old code: the pass never returns).

F2 (P2): CARLOS_STATUS_BUCKET, CARLOS_STATUS_PREFIX and
CARLOS_STATUS_REGION leave Optional. A group box reports to the
deployment bucket, unprefixed, where the console reads, while joining
and once hosting.

F3 (P3): CARLOS_ALERT_WEBHOOK leaves Optional: a webhook URL can carry
a credential, the DNSimple token's rule.

TestStatusOverridesAndTheAlertWebhookAreRefusedByNameNotValue: Assemble
and Seed refuse each by name, never printing its value, and Seed writes
nothing. The fixtures that used CARLOS_STATUS_PREFIX as their sample
optional now use CARLOS_HIBERNATE_PREFIX.

Census: @coverage internal/hostenv and internal/hostactors re-pinned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by claude 1 week ago 18ed4a9acd89b811a674d79c9a2678beb202be5c parent 4433a78

5 files changed, +151 −16

internal/hostactors/autoenroll_test.go +83 −0
internal/hostactors/membershipread.go +16 −2
internal/hostenv/hostenv.go +11 −10
internal/hostenv/hostenv_test.go +39 −2
internal/provision/testdata/plan14-root-reads-region-ha-2b.tsv +2 −2

diff --git a/internal/hostactors/autoenroll_test.go b/internal/hostactors/autoenroll_test.go

index 7789ad3ff..586bafe47 100644

--- a/internal/hostactors/autoenroll_test.go

+++ b/internal/hostactors/autoenroll_test.go

@@ -231,6 +231,89 @@ func TestAMembershipReadThatFailsKeepsTheAttempt(t *testing.T) {

}

+// stalledMembership is a describe whose connection stalls for one

+// instance: Member returns only when its context ends. Every other

+// instance is answered by the cloud underneath.

+type stalledMembership struct {

+ stalled string

+ cloud *fakeMembership

+ deadline bool

+func (s *stalledMembership) Member(ctx context.Context, awsRegion, instance string) (Membership, error) {

+ if instance != s.stalled {

+ return s.cloud.Member(ctx, awsRegion, instance)

+ }

+ _, s.deadline = ctx.Deadline()

+ <-ctx.Done()

+ return Membership{}, ctx.Err()

+// A membership read that stalls is cut off at storeCallTimeout and is

+// unknown: the pass returns within the bound, the stalled box's attempt

+// stands with its wait unchanged, and the box after it in the same pass is

+// still invited.

+func TestAStalledMembershipReadIsCutOffAndThePassGoesOn(t *testing.T) {

+ was := storeCallTimeout

+ storeCallTimeout = 100 * time.Millisecond

+ t.Cleanup(func() { storeCallTimeout = was })

+ w := newEnrollWorld(t, "")

+ // A second box the group launched, whose reference sorts after

+ // newRef's, so it waits in the pass behind the stalled read.

+ twoRef := GroupBoxReference("123", "i-two")

+ if twoRef <= newRef {

+ t.Fatalf("%s does not sort after %s", twoRef, newRef)

+ }

+ _, raw := w.signer.Record(t, twoRef, "123", "eu-west-1", "i-two")

+ if _, err := w.d.Store.Put(context.Background(), boxid.Key(twoRef), raw, ""); err != nil {

+ t.Fatal(err)

+ }

+ w.cloud.members["i-two"] = member("routers-ie")

+ stalled := &stalledMembership{stalled: "i-new", cloud: w.cloud}

+ w.e.Members = stalled

+ steps, err := w.e.EnrollPass(context.Background())

+ if err != nil || len(steps) != 2 || steps[0].Step != "began an attempt" || steps[1].Step != "began an attempt" {

+ t.Fatalf("first pass: %v, %v", steps, err)

+ }

+ first := w.attempt(t).AttemptID

+ done := make(chan []EnrollStep, 1)

+ start := time.Now()

+ go func() {

+ steps, err := w.e.EnrollPass(context.Background())

+ if err != nil {

+ t.Error(err)

+ }

+ done <- steps

+ }()

+ select {

+ case steps = <-done:

+ case <-time.After(10 * time.Second):

+ t.Fatal("the pass is still waiting on the stalled read")

+ }

+ if took := time.Since(start); took > storeCallTimeout+5*time.Second {

+ t.Fatalf("the pass took %s", took)

+ }

+ if !stalled.deadline {

+ t.Fatal("the read was given no deadline")

+ }

+ if len(steps) != 2 || steps[0].Box != newRef || !strings.Contains(steps[0].Step, "could not be read") || !strings.Contains(steps[0].Step, "did not finish in time") {

+ t.Fatalf("stalled box: %v", steps)

+ }

+ if got := w.attempt(t).AttemptID; got != first || w.e.attempts[newRef].tries != 1 || w.e.attempts[newRef].refused != "" {

+ t.Fatalf("the stalled read superseded the attempt (%s, was %s), grew its wait (%d) or refused it (%q)", got, first, w.e.attempts[newRef].tries, w.e.attempts[newRef].refused)

+ }

+ if _, ok := w.invitation(t); ok {

+ t.Fatal("the stalled box was invited")

+ }

+ if steps[1].Box != twoRef || steps[1].Step != "invited" {

+ t.Fatalf("the box behind the stalled read: %v", steps[1])

+ }

+ if _, _, err := w.d.Store.Get(context.Background(), "control/host-actor-invitations/"+twoRef+".json"); err != nil {

+ t.Fatalf("the box behind the stalled read has no invitation: %v", err)

+ }

// C2: the invitation is written only while the console's read is under a

// minute old. Here every look at the clock takes 31 seconds: the read is

// fresh when checkConfirmed has it, and over a minute old by the time the

diff --git a/internal/hostactors/membershipread.go b/internal/hostactors/membershipread.go

index 523638cf9..5e924775b 100644

--- a/internal/hostactors/membershipread.go

+++ b/internal/hostactors/membershipread.go

@@ -98,15 +98,28 @@ type membershipConfirmation struct {

// region's account, in its AWS region, with a request id for each call.

// The read's anchor, its time, is taken before the call: never later than

// the describe's completion, so its age is never understated.

+//

+// The call gets at most storeCallTimeout, as each bound store call does:

+// neither describe sets a deadline of its own, and a connection that

+// stalls would otherwise hold the serial pass, and every box behind this

+// one, for as long as it stalls. A call cut off, by the timeout or by the

+// pass's own context, is unknown whatever it returned: the attempt stands

+// and its wait does not grow.

func confirmMembership(ctx context.Context, r MembershipReader, region Region, instance string, now func() time.Time) (membershipConfirmation, error) {

if r == nil || now == nil {

return membershipConfirmation{}, fmt.Errorf("%w: no reader", ErrMembershipUnknown)

}

at := now().UTC()

- m, err := r.Member(ctx, region.AWSRegion, instance)

+ rctx, cancel := context.WithTimeout(ctx, storeCallTimeout)

+ m, err := r.Member(rctx, region.AWSRegion, instance)

+ cut := rctx.Err()

+ cancel()

// The ids the cloud gave are kept whatever came of the read, failed

// calls included, for the console's log.

c := membershipConfirmation{requestIDs: append([]string(nil), m.RequestIDs...), observedAt: at}

+ if cut != nil {

+ return c, fmt.Errorf("%w: the read did not finish in time: %v", ErrMembershipUnknown, cut)

+ }

if err != nil {

return c, fmt.Errorf("%w: %v", ErrMembershipUnknown, err)

}

@@ -189,7 +202,8 @@ func writeBound(ctx context.Context, d Directory, c membershipConfirmation, now

// context is cancelled, so an S3 request is abandoned at the HTTP layer.

// It bounds how long the console waits, not when a write commits: a

// call cut off may still commit later, and is settled from the roster on

-// a later pass. A var so tests can shorten it.

+// a later pass. confirmMembership gives each membership read the same

+// limit. A var so tests can shorten it.

var storeCallTimeout = 15 * time.Second

// deadlineStore refuses every call once check says the deadline has been

diff --git a/internal/hostenv/hostenv.go b/internal/hostenv/hostenv.go

index 6be2a2fc6..26458e0ee 100644

--- a/internal/hostenv/hostenv.go

+++ b/internal/hostenv/hostenv.go

@@ -56,19 +56,23 @@ var Required = []string{

// an address override, a pool, the data, opt and deployment directories,

// the relay's settings) and every token or API credential, except the

// deployment's own required secrets, CARLOS_SECRETS_KEY and

-// CARLOS_RELEASE_PUBKEY. An endpoint override such as CARLOS_S3_ENDPOINT

-// stays, because it selects no box authority and only operators can write

-// the parameter paths. CARLOS_REGION and the group marker, which do select, are

-// cross-checked against the signed roster where the agent uses them

-// (internal/host's mode selection). Adding a variable is a code change,

-// reviewed.

+// CARLOS_RELEASE_PUBKEY. CARLOS_ALERT_WEBHOOK is left out on the same

+// rule: a webhook URL can carry a credential. So are the status

+// destination overrides, CARLOS_STATUS_BUCKET, CARLOS_STATUS_PREFIX and

+// CARLOS_STATUS_REGION: a group box reports to the deployment bucket,

+// unprefixed, where the console reads its guard-ready report, while it is

+// joining and once it is hosting. An endpoint override such as

+// CARLOS_S3_ENDPOINT stays, because it selects no box authority and only

+// operators can write the parameter paths. CARLOS_REGION and the group

+// marker, which do select, are cross-checked against the signed roster

+// where the agent uses them (internal/host's mode selection). Adding a

+// variable is a code change, reviewed.

var Optional = []string{

"CARLOS_ACME_DELEGATION_ZONE",

"CARLOS_ACME_DIRECTORY",

"CARLOS_ALERT_CPU_PCT",

"CARLOS_ALERT_DISK_PCT",

"CARLOS_ALERT_MEM_PCT",

- "CARLOS_ALERT_WEBHOOK",

"CARLOS_AWS_REGION",

"CARLOS_BACKHAUL_MAX_INFLIGHT",

"CARLOS_DNS_PROVIDER",

@@ -83,9 +87,6 @@ var Optional = []string{

"CARLOS_REQUIRE_POINTER_DIGEST",

"CARLOS_S3_ENDPOINT",

"CARLOS_S3_QUIRKS",

- "CARLOS_STATUS_BUCKET",

- "CARLOS_STATUS_PREFIX",

- "CARLOS_STATUS_REGION",

"CARLOS_STEERING_ZONE_ID",

"CARLOS_SWAP",

"CARLOS_UNIT_SETTLE",

diff --git a/internal/hostenv/hostenv_test.go b/internal/hostenv/hostenv_test.go

index fc539990d..7b61ece70 100644

--- a/internal/hostenv/hostenv_test.go

+++ b/internal/hostenv/hostenv_test.go

@@ -71,7 +71,7 @@ func today() map[string]string {

"CARLOS_RELEASE_PUBKEY": "q3J0bXlwdWJsaWNrZXk+/=",

"CARLOS_SECRETS_KEY": "c2VjcmV0c2tleQ==",

"CARLOS_SYSTEM_CHANNEL": "stable",

- "CARLOS_STATUS_PREFIX": "fleet",

+ "CARLOS_HIBERNATE_PREFIX": "fleet",

}

@@ -340,7 +340,7 @@ func TestWriteTakesOnlyAllowedNamesAndSafeValues(t *testing.T) {

// Values that would break a line, end the file early or change how

// systemd reads it.

for _, v := range []string{"a\nCARLOS_REGION=uk", "a\x00b", "a\rb", "$(id)", "a b", "\"quoted\""} {

- p.vals["/carlos/flagship/ie/CARLOS_STATUS_PREFIX"] = v

+ p.vals["/carlos/flagship/ie/CARLOS_HIBERNATE_PREFIX"] = v

if _, err := Fetch(ctx, p, "flagship", "ie"); !errors.Is(err, ErrValue) || strings.Contains(err.Error(), v) {

t.Fatalf("%q: %v", v, err)

}

@@ -389,3 +389,40 @@ func TestDNSProviderTokenIsRefusedByNameNotValue(t *testing.T) {

t.Fatalf("Seed wrote %v", p.sets)

}

+// The status destination overrides are off the allowlist: a group box

+// reports to the deployment bucket, unprefixed, where the console reads,

+// while joining and once hosting. So is the alert webhook, whose URL can

+// carry a credential. Assemble and Seed refuse each by name, and the

+// refusal never carries its value.

+func TestStatusOverridesAndTheAlertWebhookAreRefusedByNameNotValue(t *testing.T) {

+ for name, value := range map[string]string{

+ "CARLOS_STATUS_BUCKET": "other-status-bucket-5c1d",

+ "CARLOS_STATUS_PREFIX": "fleets-prefix-7a2b",

+ "CARLOS_STATUS_REGION": "ap-southeast-9z",

+ "CARLOS_ALERT_WEBHOOK": "https://hooks.example.com/services/T0/B0/s3cr3t-4e5f",

+ } {

+ t.Run(name, func(t *testing.T) {

+ if Allowed(name) {

+ t.Fatalf("%s is on the allowlist", name)

+ }

+ check := func(what string, err error) {

+ t.Helper()

+ if !errors.Is(err, ErrValue) || !strings.Contains(err.Error(), name) || strings.Contains(err.Error(), value) {

+ t.Fatalf("%s: %v", what, err)

+ }

+ env := today()

+ env[name] = value

+ shared, regional := Split(env)

+ _, err := Assemble(shared, regional)

+ check("Assemble", err)

+ p := &fakeParams{vals: map[string]string{}}

+ _, err = Seed(context.Background(), p, "flagship", "ie", env)

+ check("Seed", err)

+ if len(p.sets) != 0 {

+ t.Fatalf("Seed wrote %v", p.sets)

+ }

+ })

+ }

diff --git a/internal/provision/testdata/plan14-root-reads-region-ha-2b.tsv b/internal/provision/testdata/plan14-root-reads-region-ha-2b.tsv

index e727fef76..59786c211 100644

--- a/internal/provision/testdata/plan14-root-reads-region-ha-2b.tsv

+++ b/internal/provision/testdata/plan14-root-reads-region-ha-2b.tsv

@@ -8,7 +8,7 @@

# Task 8: joining mode and the gate (internal/host joining.go; RunAgent calls hostingGate before runAgent; ClaimWiring.startRegional sets the health answer's hosting condition from the gate's decision and clears it when the worker ends). runAgent, authority B,C,D,F,J4, is not changed. No root-read unit changed or was added. Envelope-only, and the seal stands. Before the gate, RunAgent has a box carrying the group marker resolve its group reference (agentEntry, through Task 6's resolveGroupBoxReference, with backoff), whatever the switches say, so runAgent's loops never keep an empty label; joining touches the join marker again once a minute while the box stays invited, so a failed carlos system join is retried.

# Task 9 (system.go): the install-channel, join and host-env verbs join systemVerb's list and runSystem's dispatch, and the usage gains their lines. The two seed-overlay authority declarations are re-digested, class, task, basis and (empty) kinds unchanged: no root read is added. Merged onto main 26065378, whose activation-tools overlay (census-ruling-activation-tools) re-ruled both declarations for the claims verb: the two rows carry that overlay's class, basis (B,C,D,F,G,J4) and text, re-digested over the merged code with both sets of verbs.

@coverage . 9949422c526be724b50a4614fc79623b9823a1aa0960d1472fd9050f8000c3b0

-@coverage internal/hostenv 45d9d4b90a5d441020f5ba4573821ca92701246c80b32d6f1823665fb0f65208

+@coverage internal/hostenv 6b65e7987026a4c81f410c0afcfaca13bc2b252d7382c45564f670e36a045bcb

internal/hostenv/cli.go runHostEnv authority A,J4 rev3=census-ruling-region-ha-2b;basis=carlos system host-env: write runs as root at a box's start-up and writes /etc/carlos/host.env, 0600, whole (temporary file and rename), from allowlisted parameters with safe values only, refusing a missing required value; its one read site is seed's --from file, an operator's host.env whose values go only to the parameter store; write's path is fixed and reached only through writeHostEnv file:ReadFile@1500 df4470176b229b311927ff0ac4f268d354c3a2357cac4300ac0992d2ebb90899

internal/hostenv/write_linux.go checkedDir authority A,J4 rev3=census-ruling-region-ha-2b;basis=carlos system host-env write (root, Linux): opens one directory under one already held, O_NOFOLLOW|O_DIRECTORY, creating it 0755 first only when asked and missing, and refuses it unless fstat shows a directory owned by this process's user (root, as write requires) that no group or other can write file:Fstat@576,file:Openat@168,file:Openat@372 012257ac0866f3385044785d8abae9c31e61134928c394e22a2aeaef39ec9f3c

internal/hostenv/write_linux.go writeHostEnv authority A,J4 rev3=census-ruling-region-ha-2b;basis=carlos system host-env write (root, Linux): writes /etc/carlos/host.env only through /etc and /etc/carlos held by descriptor and checked by checkedDir; the new file is created 0600, O_EXCL|O_NOFOLLOW, in that directory, synced, renamed over host.env within it, and the directory synced; its read sites open the root and the temporary file file:Open@68,file:Openat@509 1245fc0f819ba73fb45545ffdcf92d5d4941802175e73bac209ab4d4238adff1

@@ -17,7 +17,7 @@ internal/host/installchannel.go channelInstall.run authority F,J4 rev3=census-ru

internal/host/installchannel_linux.go slotState authority F,J4 rev3=census-ruling-region-ha-2b;basis=carlos system install-channel (root, Linux): opens one protected slot with O_NOFOLLOW (a symlink is never followed), fstats the descriptor and hashes from it, to compare with the sha256 of the signed, verified artifact; a slot holding anything else, a symlink or a non-regular file is refused and left as it is, and nothing in it is executed file:Open@66,file:Stat@388 bc533a29f92ae95d31123b551354ab9f8960576e5ffef1bd6a74a06df0b57a24

internal/host/installchannel_linux.go var openat2 authority F,J4 rev3=census-ruling-region-ha-2b;basis=carlos system install-channel (root, Linux): unix.Openat2 held in a variable so a test can make it fail; removeBeneath is its only user and has no path-based fallback when it fails file:Openat2@14 6081e86f758f4ae3948ec2d4f4c037888e1362f122a410c110e31290ba914816

internal/host/installchannel_linux.go removeBeneath authority F,J4 rev3=census-ruling-region-ha-2b;basis=carlos system install-channel (root, Linux): removes the agent's cache entry for the version, after a run that made it, only through the router directory held by descriptor (opened with no symlink anywhere on its path); its version directory is opened beneath it with RESOLVE_BENEATH|RESOLVE_NO_SYMLINKS and everything under it is unlinked by descriptor; a symlink is unlinked, never entered; when openat2 is unavailable or refuses, the removal refuses and leaves the tree, with no path-based fallback; its one read site lists the held directory's names file:Readdirnames@381 42732bb7b400abdb7f18ddbf5aacdfadc9496d8941e774dc01a471bf15c35f69

-@coverage internal/hostactors 8777d00e51be1d185bb9f16018c7a26cd7e1d8887044e9bdc9370613be372545

+@coverage internal/hostactors 4abc791e6450616aa877bc09f806242186e6d8a53f1e52600a66ab7cab40920b

@coverage internal/hostactors/awsmembership ff967730ddb8f17acb73e3351590ba11695e82341b195539da6fbd856ac39175

@coverage internal/console e891098191a724430f78e9aa0c5d1307b9343283e0cb7506163383c4dee0df7c

@legacy 2f4ca2a51661d8aae1e0f38a41efdcc9df9f1dd75411ca6742d6ee3ae71f1756 internal/hostactors/autoenroll.go AutoEnroller.inputs legacy accepted-legacy accepted-legacy;rev3=J4;basis=root-input;census-ruling-region-ha-2b file:Read@102,file:Read@305 c0ae921ee9d8df9de403d488d9708c565d659a7bf880199313f50bdc7c8f8127