instancekey: accept only the canonical adoption record, pin ParseEnv's behaviour and name the empty-key exception (task 1 review minors 1-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+46
−7
internal/activator/backend_exec.go+7 −3internal/activator/backend_exec_test.go+2 −2internal/instancekey/instancekey.go+8 −2internal/instancekey/instancekey_test.go+29 −0
CI passed — run details
CI log
=== gate ===
cd launchctx && go vet ./... && go test ./... -count=1
ok amadan.net/carlos/platform.git/launchctx 0.013s
? amadan.net/carlos/platform.git/launchctx/controlname [no test files]
? amadan.net/carlos/platform.git/launchctx/wireid [no test files]
go vet ./...
gofmt -l .: clean
go test ./... -count=1 -timeout 30m
ok github.com/carlosframework/platform 325.074s
? github.com/carlosframework/platform/cmd/changelog [no test files]
ok github.com/carlosframework/platform/cmd/console 3.894s
ok github.com/carlosframework/platform/internal/activator 140.177s
ok github.com/carlosframework/platform/internal/agentsock 0.020s
ok github.com/carlosframework/platform/internal/alerting 0.003s
ok github.com/carlosframework/platform/internal/analytics 0.028s
ok github.com/carlosframework/platform/internal/appconfig 2.386s
ok github.com/carlosframework/platform/internal/apperr 0.108s
ok github.com/carlosframework/platform/internal/applog 0.168s
ok github.com/carlosframework/platform/internal/awsinv 0.004s
ok github.com/carlosframework/platform/internal/boxchoice 0.004s
ok github.com/carlosframework/platform/internal/boxid 0.641s
? github.com/carlosframework/platform/internal/boxid/boxidtest [no test files]
? github.com/carlosframework/platform/internal/boxid/internal/signtest [no test files]
ok github.com/carlosframework/platform/internal/boxvolumes 0.008s
ok github.com/carlosframework/platform/internal/caps 0.015s
ok github.com/carlosframework/platform/internal/certwarm 0.003s
ok github.com/carlosframework/platform/internal/cgroup 0.195s
ok github.com/carlosframework/platform/internal/changelog 0.295s
ok github.com/carlosframework/platform/internal/clamd 0.072s
? github.com/carlosframework/platform/internal/cliflag [no test files]
ok github.com/carlosframework/platform/internal/console 321.539s
? github.com/carlosframework/platform/internal/controlname [no test files]
? github.com/carlosframework/platform/internal/controlsock [no test files]
ok github.com/carlosframework/platform/internal/dbdoor 5.922s
ok github.com/carlosframework/platform/internal/dbdoor/dbdoortest 0.024s
ok github.com/carlosframework/platform/internal/dbverify 0.005s
ok github.com/carlosframework/platform/internal/dnsrec 0.003s
ok github.com/carlosframework/platform/internal/docsite 0.009s
ok github.com/carlosframework/platform/internal/domains 2.414s
ok github.com/carlosframework/platform/internal/doorkey 0.015s
ok github.com/carlosframework/platform/internal/economics 0.008s
ok github.com/carlosframework/platform/internal/edge 26.401s
? github.com/carlosframework/platform/internal/edgeproof [no test files]
ok github.com/carlosframework/platform/internal/edgesfile 0.007s
ok github.com/carlosframework/platform/internal/fleetapi 0.192s
ok github.com/carlosframework/platform/internal/gate 1.875s
ok github.com/carlosframework/platform/internal/genselect 0.030s
ok github.com/carlosframework/platform/internal/geo 0.027s
ok github.com/carlosframework/platform/internal/geoip 0.037s
ok github.com/carlosframework/platform/internal/host 46.315s
ok github.com/carlosframework/platform/internal/hostactors 0.604s
ok github.com/carlosframework/platform/internal/hostbox 1.459s
ok github.com/carlosframework/platform/internal/hostclaim 0.407s
ok github.com/carlosframework/platform/internal/hostevidence 0.011s
ok github.com/carlosframework/platform/internal/hostexec 0.342s
ok github.com/carlosframework/platform/internal/hostfence 0.169s
ok github.com/carlosframework/platform/internal/hostfiles 4.087s
ok github.com/carlosframework/platform/internal/hostfloor 0.013s
ok github.com/carlosframework/platform/internal/hostguard 0.181s
ok github.com/carlosframework/platform/internal/hostops 1.956s
ok github.com/carlosframework/platform/internal/hostquarantine 0.003s
ok github.com/carlosframework/platform/internal/hostresolve 0.246s
ok github.com/carlosframework/platform/internal/hostwrite 0.003s
ok github.com/carlosframework/platform/internal/infraview 0.005s
ok github.com/carlosframework/platform/internal/instancectl 1.711s
ok github.com/carlosframework/platform/internal/instanceexec 2.903s
ok github.com/carlosframework/platform/internal/instancefs 0.008s
ok github.com/carlosframework/platform/internal/instancekey 0.003s
ok github.com/carlosframework/platform/internal/layout 1.764s
ok github.com/carlosframework/platform/internal/lease 9.514s
ok github.com/carlosframework/platform/internal/lease/leasetest 0.007s
ok github.com/carlosframework/platform/internal/ledger 0.017s
ok github.com/carlosframework/platform/internal/mail 0.399s
ok github.com/carlosframework/platform/internal/maintenance 0.162s
ok github.com/carlosframework/platform/internal/maintenanceexec 5.800s
ok github.com/carlosframework/platform/internal/maintenanceguard 0.155s
ok github.com/carlosframework/platform/internal/metrics 0.014s
ok github.com/carlosframework/platform/internal/moves 21.891s
ok github.com/carlosframework/platform/internal/moves/archive 1.159s
ok github.com/carlosframework/platform/internal/objstore 0.420s
ok github.com/carlosframework/platform/internal/ops 0.315s
ok github.com/carlosframework/platform/internal/park 0.391s
ok github.com/carlosframework/platform/internal/privatefile 0.003s
ok github.com/carlosframework/platform/internal/provision 3.627s
ok github.com/carlosframework/platform/internal/publicrelease 0.006s
ok github.com/carlosframework/platform/internal/registry 3.817s
ok github.com/carlosframework/platform/internal/relay 7.200s
ok github.com/carlosframework/platform/internal/release 18.807s
ok github.com/carlosframework/platform/internal/rollout 1.852s
ok github.com/carlosframework/platform/internal/rootaws 0.009s
ok github.com/carlosframework/platform/internal/s3compat 0.012s
ok github.com/carlosframework/platform/internal/safefile 0.018s
ok github.com/carlosframework/platform/internal/schedule 2.348s
ok github.com/carlosframework/platform/internal/sim/hostlife 4.742s
ok github.com/carlosframework/platform/internal/sim/litestreamfacts 4.754s
ok github.com/carlosframework/platform/internal/status 0.081s
ok github.com/carlosframework/platform/internal/steering 0.006s
ok github.com/carlosframework/platform/internal/stopevidence 0.253s
ok github.com/carlosframework/platform/internal/svcrole 0.003s
ok github.com/carlosframework/platform/internal/telemetry 0.006s
ok github.com/carlosframework/platform/internal/termfence 0.004s
ok github.com/carlosframework/platform/internal/tunnelclient 0.005s
ok github.com/carlosframework/platform/internal/unixdial 0.002s
ok github.com/carlosframework/platform/internal/updatewords 0.002s
ok github.com/carlosframework/platform/internal/usage 0.005s
ok github.com/carlosframework/platform/internal/version 0.004s
ok github.com/carlosframework/platform/internal/vet 1.720s
=== cross ===
--- shard 0 of 5 ---
cross-compiling darwin/arm64
--- shard 1 of 5 ---
cross-compiling darwin/amd64
--- shard 2 of 5 ---
cross-compiling linux/arm64
--- shard 3 of 5 ---
cross-compiling windows/amd64
--- shard 4 of 5 ---
cross-compiling windows/arm64
=== minio-contract ===
minio contract: arm64, minio RELEASE.2025-09-07T16-13-09Z, mc RELEASE.2025-08-13T08-35-41Z
minio: cached, digest verified
mc: cached, digest verified
minio healthy on 127.0.0.1:20999
Added `local` successfully.
Bucket created successfully `local/contract-bucket`.
Bucket created successfully `local/denied-bucket`.
Created policy `contract-only` successfully.
Added user `scopeduser` successfully.
Attached Policies: [contract-only]
To User: scopeduser
buckets and scoped user ready
=== RUN TestDeliveredContractMinio
=== RUN TestDeliveredContractMinio/put
=== RUN TestDeliveredContractMinio/get_round-trips_the_body
=== RUN TestDeliveredContractMinio/list_shows_the_key
=== RUN TestDeliveredContractMinio/head_and_list_agree_on_the_version
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/not_by_PutObjectTagging
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/and_not_by_x-amz-tagging_on_upload
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CreateMultipartUpload
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CopyObject_with_a_REPLACE_tagging_directive
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/but_multipart_itself_is_allowed
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_copy_itself_is_allowed
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/a_POST-policy_forged_tag_with_a_matching_version_is_still_not_a_verdict
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_an_ordinary_tagged-with-something-else_upload_is_allowed
=== RUN TestDeliveredContractMinio/delete_removes_it
=== RUN TestDeliveredContractMinio/cross-bucket_denial
--- PASS: TestDeliveredContractMinio (0.02s)
--- PASS: TestDeliveredContractMinio/put (0.00s)
--- PASS: TestDeliveredContractMinio/get_round-trips_the_body (0.00s)
--- PASS: TestDeliveredContractMinio/list_shows_the_key (0.00s)
--- PASS: TestDeliveredContractMinio/head_and_list_agree_on_the_version (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/not_by_PutObjectTagging (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/and_not_by_x-amz-tagging_on_upload (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route (0.01s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CreateMultipartUpload (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CopyObject_with_a_REPLACE_tagging_directive (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/but_multipart_itself_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_copy_itself_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/a_POST-policy_forged_tag_with_a_matching_version_is_still_not_a_verdict (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_an_ordinary_tagged-with-something-else_upload_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/delete_removes_it (0.00s)
--- PASS: TestDeliveredContractMinio/cross-bucket_denial (0.00s)
PASS
ok github.com/carlosframework/platform/internal/objstore 0.023s
=== RUN TestT4StorageProbeMinio
2026/10/11 13:41:00 moves: T4: STORE: list: operation error S3: ListObjectsV2, https response error StatusCode: 403, RequestID: 18DD7D5BE2659AAB, HostID: 7c420053e0fbcd7f41653fdd92ffa2802c903d05303b1aa15adc2e8bb65d183f, api error SignatureDoesNotMatch: The request signature we calculated does not match the signature you provided. Check your key and signing method.
--- PASS: TestT4StorageProbeMinio (0.01s)
PASS
ok github.com/carlosframework/platform/internal/host 0.015s
minio contract: PASS
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go| index 39491ef4b..aac499383 100644 |
| --- a/internal/activator/backend_exec.go |
| +++ b/internal/activator/backend_exec.go |
| @@ -1873,7 +1873,9 @@ func (b *ExecBackend) instanceEnvParts(rt registry.Route) (cfgEnv, ownEnv map[st |
| // shadowed is the sorted names present in both the rendered configuration |
| // file and the instance's own <host>.env: the override mergeEnv applies |
| // (moves spec §7.3 T3 reports it), collected here because this is the only |
| -// layer that sees both files. The move's launch report (§7.3, codex R2-8) |
| +// layer that sees both files. The one exception is mergeEnv's: an empty |
| +// CARLOS_INSTANCE_KEY in the own file overrides nothing, so it is not |
| +// listed. The move's launch report (§7.3, codex R2-8) |
| // is computed over exactly this env, so a target compares what would run, |
| // not a reconstruction of it. The values are raw: the caller HMACs them |
| // and must never log or return them. |
| @@ -1977,10 +1979,12 @@ func withoutVars(env map[string]string, keys ...string) map[string]string { |
| // mergeEnv composes the child's env-file layers: base (the sync-owned |
| // <host>.config.env, bucket-derived) under overlay (the instance's own |
| -// <host>.env). The overlay wins every conflict — CARLOS_ADMIN_TOKEN |
| +// <host>.env). The overlay wins every conflict but one — CARLOS_ADMIN_TOKEN |
| // lives only in the instance file and must never be clobberable from |
| // the bucket, and a box-local override deliberately beats central |
| -// config. Returns a fresh map; inputs are never mutated. |
| +// config. The exception: an empty CARLOS_INSTANCE_KEY in the overlay |
| +// supplies no key, so a non-empty one in base stands (instancekey.Effective). |
| +// Returns a fresh map; inputs are never mutated. |
| func mergeEnv(base, overlay map[string]string) map[string]string { |
| out := make(map[string]string, len(base)+len(overlay)) |
| for k, v := range base { |
diff --git a/internal/activator/backend_exec_test.go b/internal/activator/backend_exec_test.go| index 37bd8d8e0..d6163ccd0 100644 |
| --- a/internal/activator/backend_exec_test.go |
| +++ b/internal/activator/backend_exec_test.go |
| @@ -427,8 +427,8 @@ func TestEmptyOwnInstanceKeyShadowsNothing(t *testing.T) { |
| t.Fatalf("empty own key: CARLOS_INSTANCE_KEY is the configured key: %v; shadowed %v, want [MINE]", env["CARLOS_INSTANCE_KEY"] == "cfg", shadowed) |
| } |
| write(".env", "CARLOS_INSTANCE_KEY=own\nMINE=1\n") |
| - if env, shadowed, err = b.LaunchEnvironment(rt); err != nil || env["CARLOS_INSTANCE_KEY"] != "own" || len(shadowed) != 2 { |
| - t.Fatalf("non-empty own key: CARLOS_INSTANCE_KEY is the own key: %v; shadowed %v; err %v", env["CARLOS_INSTANCE_KEY"] == "own", shadowed, err) |
| + if env, shadowed, err = b.LaunchEnvironment(rt); err != nil || env["CARLOS_INSTANCE_KEY"] != "own" || strings.Join(shadowed, ",") != "CARLOS_INSTANCE_KEY,MINE" { |
| + t.Fatalf("non-empty own key: CARLOS_INSTANCE_KEY is the own key: %v; shadowed %v, want [CARLOS_INSTANCE_KEY MINE]; err %v", env["CARLOS_INSTANCE_KEY"] == "own", shadowed, err) |
| } |
| } |
diff --git a/internal/instancekey/instancekey.go b/internal/instancekey/instancekey.go| index 57788d5e1..f99491301 100644 |
| --- a/internal/instancekey/instancekey.go |
| +++ b/internal/instancekey/instancekey.go |
| @@ -145,8 +145,11 @@ func AdoptionRecord(b Binding, source, sha string) []byte { |
| } |
| // ParseAdoption reads a record strictly: version 2, this variable, a |
| -// complete binding, a known source and a digest. Anything else is an |
| -// error, never a record. |
| +// complete binding, a known source and a digest, in exactly the bytes |
| +// AdoptionRecord writes for them. Anything else is an error, never a |
| +// record: trailing data, a key in another case, a key given twice, or any |
| +// other spelling of the same fields (encoding/json alone accepts the |
| +// last three). |
| func ParseAdoption(record []byte) (Adoption, error) { |
| var a Adoption |
| dec := json.NewDecoder(bytes.NewReader(record)) |
| @@ -157,6 +160,9 @@ func ParseAdoption(record []byte) (Adoption, error) { |
| if dec.More() || a.V != 2 || a.Var != Var || !a.Binding.valid() || (a.Source != SourcePlatform && a.Source != SourceConfig) || !ValidDigest(a.SHA256) { |
| return Adoption{}, errors.New("instancekey: adoption record: not a version 2 record for " + Var) |
| } |
| + if !bytes.Equal(record, AdoptionRecord(a.Binding, a.Source, a.SHA256)) { |
| + return Adoption{}, errors.New("instancekey: adoption record: not in the canonical form AdoptionRecord writes") |
| + } |
| return a, nil |
| } |
diff --git a/internal/instancekey/instancekey_test.go b/internal/instancekey/instancekey_test.go| index 39b3eb89b..7e13067a0 100644 |
| --- a/internal/instancekey/instancekey_test.go |
| +++ b/internal/instancekey/instancekey_test.go |
| @@ -64,13 +64,42 @@ func TestAdoptionRecordBindsOneWriter(t *testing.T) { |
| "an unknown field": `{"v":2,"var":"CARLOS_INSTANCE_KEY","host":"h","incarnation":"i","session":1,"generation":"g","source":"config","sha256":"` + Digest(key) + `","key":"x"}`, |
| "trailing data": string(rec) + `{}`, |
| "garbage": "not json", |
| + // The canonical bytes are the only record (review minor 1): |
| + // encoding/json alone accepts each of these. |
| + "a trailing }": string(rec) + `}`, |
| + "a trailing ]": string(rec) + `]`, |
| + "an upper-case key": strings.Replace(string(rec), `"sha256"`, `"SHA256"`, 1), |
| + "a key given twice": strings.Replace(string(rec), `"source":"platform"`, `"source":"config","source":"platform"`, 1), |
| + "reordered fields": strings.Replace(string(rec), `{"v":2,"var":"CARLOS_INSTANCE_KEY",`, `{"var":"CARLOS_INSTANCE_KEY","v":2,`, 1), |
| + "an upper-case digest": strings.Replace(string(rec), Digest(key), strings.ToUpper(Digest(key)), 1), |
| + "a short digest": strings.Replace(string(rec), Digest(key), Digest(key)[:63], 1), |
| + "another variable": strings.Replace(string(rec), `"var":"CARLOS_INSTANCE_KEY"`, `"var":"OTHER"`, 1), |
| } { |
| + if raw == string(rec) { |
| + t.Fatalf("%s: the case did not change the record", name) |
| + } |
| if _, err := ParseAdoption([]byte(raw)); err == nil { |
| t.Errorf("%s: parsed", name) |
| } |
| } |
| } |
| +// ParseEnv is exactly the activator's old parseEnvBytes, so what a reader |
| +// parses is what a child received: quotes kept, whole-line comments only, |
| +// the last duplicate winning, CRLF and surrounding whitespace trimmed. |
| +func TestParseEnvMatchesWhatAChildReceives(t *testing.T) { |
| + got := ParseEnv([]byte("A=\"quoted\"\nB='single'\n# C=1\n # D=1\nE=1 # kept\nF=1\nF=2\n G = spaced \nH=a=b\nI=1\r\nnoequals\n\n")) |
| + want := map[string]string{"A": `"quoted"`, "B": "'single'", "E": "1 # kept", "F": "2", "G": "spaced", "H": "a=b", "I": "1"} |
| + if len(got) != len(want) { |
| + t.Fatalf("ParseEnv = %q, want %q", got, want) |
| + } |
| + for k, v := range want { |
| + if got[k] != v { |
| + t.Fatalf("ParseEnv = %q, want %q", got, want) |
| + } |
| + } |
| +} |
| + |
| // O3': the install refuses env files that would deliver any key but the |
| // pinned one, including a configuration changed since the pin. |
| func TestVerifyDeliveredRefusesAnyOtherKey(t *testing.T) { |