amadan

carlos / platform Public

Sidecar isolation Task 10 fix round 1: one host.env parser for the claim unit and the workers floor

parseHostEnvKeys is readClaimUnitSettings' line parsing, moved verbatim
and side-effect free: selected keys, systemd quoting, last wins, and the
same two refusals. readClaimUnitSettings keeps the read, the missing-file
default and the root-retry directory preparation; install's
nestedWorkersOptedIn now reads through the same parser (missing = off,
unreadable or refused = on). Census claim for readClaimUnitSettings.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude b9f6256d9cbaefddeccdb001017f212b4852af17 parent 1ce6745
4 files changed, +86 −67
  • implementation/census-claims-sidecar-isolation.txt +1 −0
  • internal/host/claimunit.go +55 −34
  • internal/host/claimunit_test.go +22 −0
  • internal/host/systemapply.go +8 −33
diff --git a/implementation/census-claims-sidecar-isolation.txt b/implementation/census-claims-sidecar-isolation.txt
index a493f4bae..3c85d07eb 100644
--- a/implementation/census-claims-sidecar-isolation.txt
+++ b/implementation/census-claims-sidecar-isolation.txt
@@ -32,3 +32,4 @@ internal/status/snapshot.go:CollectBox carries Probes.FlatExec into BoxMetrics.F
internal/host/systemapply.go:systemApplier.install beside R9 and before the journal, refuses a pre-worker candidate (version below release.WorkerLayoutFloor, or unparseable and no carlos-instance-worker@.service in its staged units) while host.env sets CARLOS_NESTED_WORKERS=on (unreadable counts as on) or the row test names a worker host (D11 rev 4, R3-1); no new read
internal/host/systemapply.go:layoutReversalIncomplete returns the named hosts and the failed reads apart (hosts, problems) so the workers floor can tell a worker host from an error; the same reads, the same hosts named
internal/host/systemapply.go:systemApplier.layoutReversalReasons opens the registry through guardRegistry (the seam it had inline) and joins layoutReversalIncomplete's hosts and problems into the same list as before; no new read
+internal/host/claimunit.go:readClaimUnitSettings the line parsing moved verbatim into parseHostEnvKeys (side-effect free, selected keys, same error texts), shared with install's workers floor (nestedWorkersOptedIn); the file read, the missing-file default and prepareRootRetryUnitDirectory stay here; no new read
diff --git a/internal/host/claimunit.go b/internal/host/claimunit.go
index 5ad6a66df..d67487cf9 100644
--- a/internal/host/claimunit.go
+++ b/internal/host/claimunit.go
@@ -27,46 +27,17 @@ type claimUnitSettings struct {
}
func readClaimUnitSettings(path string) (claimUnitSettings, error) {
- values := map[string]string{"CARLOS_HOST_CLAIM": "", "CARLOS_DATA_DIR": "", "CARLOS_OPT_DIR": ""}
+ keys := []string{"CARLOS_HOST_CLAIM", "CARLOS_DATA_DIR", "CARLOS_OPT_DIR"}
+ values := map[string]string{}
raw, err := readClaimUnitFile(path)
if err != nil && !errors.Is(err, os.ErrNotExist) {
return claimUnitSettings{}, err
}
if err == nil {
// The installed EnvironmentFile is authoritative, including absent keys.
- // Never execute it as shell code. Refuse unsupported syntax for these
- // three settings rather than render permissions for a different value.
- for key := range values {
- values[key] = ""
- }
- scan := bufio.NewScanner(bytes.NewReader(raw))
- for scan.Scan() {
- line := strings.TrimSpace(scan.Text())
- if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, ";") {
- continue
- }
- key, v, ok := strings.Cut(line, "=")
- key = strings.TrimSpace(key)
- v = strings.TrimSpace(v)
- if !ok {
- continue
- }
- if strings.HasSuffix(v, `\`) || (len(v) > 0 && (v[0] == '"' || v[0] == '\'') && (len(v) < 2 || v[len(v)-1] != v[0])) {
- return claimUnitSettings{}, errors.New("unsupported multiline host unit setting")
- }
- if _, known := values[key]; !known {
- continue
- }
- v = strings.TrimSpace(v)
- if len(v) >= 2 && ((v[0] == '"' && v[len(v)-1] == '"') || (v[0] == '\'' && v[len(v)-1] == '\'')) {
- v = v[1 : len(v)-1]
- }
- if strings.ContainsAny(v, "\\\"'\x00\r\n") {
- return claimUnitSettings{}, fmt.Errorf("unsupported host unit setting %s", key)
- }
- values[key] = v
- }
- if err := scan.Err(); err != nil {
+ // Unsupported syntax for these three settings is refused rather than
+ // render permissions for a different value.
+ if values, err = parseHostEnvKeys(raw, keys...); err != nil {
return claimUnitSettings{}, err
}
}
@@ -93,6 +64,56 @@ func readClaimUnitSettings(path string) (claimUnitSettings, error) {
return cfg, nil
}
+// parseHostEnvKeys reads the named keys out of host.env's bytes, the way
+// the units' EnvironmentFile= reads them, for root's readers that decide
+// on them (readClaimUnitSettings, nestedWorkersOptedIn). It has no side
+// effects: it reads only raw and never executes it as shell code.
+//
+// Comments (# or ;) and blank lines are skipped, a line with no "=" is
+// ignored, one pair of matching quotes around a value is stripped, and the
+// last assignment of a key wins. Every key is in the result, "" when the
+// file does not set it. Syntax this reader cannot place is an error, not
+// a guess: a continuation line or an unterminated quote on any assignment
+// ("unsupported multiline host unit setting"), and a selected key whose
+// value still holds a quote, a backslash, NUL, CR or LF ("unsupported
+// host unit setting <key>").
+func parseHostEnvKeys(raw []byte, keys ...string) (map[string]string, error) {
+ values := make(map[string]string, len(keys))
+ for _, k := range keys {
+ values[k] = ""
+ }
+ scan := bufio.NewScanner(bytes.NewReader(raw))
+ for scan.Scan() {
+ line := strings.TrimSpace(scan.Text())
+ if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, ";") {
+ continue
+ }
+ key, v, ok := strings.Cut(line, "=")
+ key = strings.TrimSpace(key)
+ v = strings.TrimSpace(v)
+ if !ok {
+ continue
+ }
+ if strings.HasSuffix(v, `\`) || (len(v) > 0 && (v[0] == '"' || v[0] == '\'') && (len(v) < 2 || v[len(v)-1] != v[0])) {
+ return nil, errors.New("unsupported multiline host unit setting")
+ }
+ if _, known := values[key]; !known {
+ continue
+ }
+ if len(v) >= 2 && ((v[0] == '"' && v[len(v)-1] == '"') || (v[0] == '\'' && v[len(v)-1] == '\'')) {
+ v = v[1 : len(v)-1]
+ }
+ if strings.ContainsAny(v, "\\\"'\x00\r\n") {
+ return nil, fmt.Errorf("unsupported host unit setting %s", key)
+ }
+ values[key] = v
+ }
+ if err := scan.Err(); err != nil {
+ return nil, err
+ }
+ return values, nil
+}
+
func claimUnitContent(name string, content []byte, cfg claimUnitSettings) []byte {
if name != claimControlUnit || !cfg.enabled {
return content
diff --git a/internal/host/claimunit_test.go b/internal/host/claimunit_test.go
index 9a2190b5f..fade1d27c 100644
--- a/internal/host/claimunit_test.go
+++ b/internal/host/claimunit_test.go
@@ -305,3 +305,25 @@ func TestPlan14RootRetryUnitDirectoryScope(t *testing.T) {
t.Fatal("disabled claims changed the unit")
}
}
+
+// parseHostEnvKeys is the one host.env reader readClaimUnitSettings and
+// the workers floor share: selected keys only, every one present, systemd's
+// quoting and last-wins, and the two refusals with their exact texts.
+func TestParseHostEnvKeys(t *testing.T) {
+ t.Parallel()
+ got, err := parseHostEnvKeys([]byte("# CARLOS_A=no\n; CARLOS_A=no\n\nnot an assignment \\\nCARLOS_A = \"one\"\nCARLOS_B='two'\nCARLOS_OTHER=x\"y\nCARLOS_A=three\n"), "CARLOS_A", "CARLOS_B", "CARLOS_C")
+ want := map[string]string{"CARLOS_A": "three", "CARLOS_B": "two", "CARLOS_C": ""}
+ if err != nil || !reflect.DeepEqual(got, want) {
+ t.Fatalf("parse = %v, %v; want %v", got, err, want)
+ }
+ for _, tc := range []struct{ raw, want string }{
+ {"CARLOS_OTHER=a \\\n", "unsupported multiline host unit setting"},
+ {"CARLOS_OTHER=\"open\n", "unsupported multiline host unit setting"},
+ {"CARLOS_A=a\\b\n", "unsupported host unit setting CARLOS_A"},
+ {"CARLOS_A=\"a\"b\"\n", "unsupported host unit setting CARLOS_A"},
+ } {
+ if got, err := parseHostEnvKeys([]byte(tc.raw), "CARLOS_A"); err == nil || err.Error() != tc.want || got != nil {
+ t.Errorf("%q: %v, %v; want error %q", tc.raw, got, err, tc.want)
+ }
+ }
+}
diff --git a/internal/host/systemapply.go b/internal/host/systemapply.go
index 3b7426632..c1bad3cba 100644
--- a/internal/host/systemapply.go
+++ b/internal/host/systemapply.go
@@ -1,7 +1,6 @@
package host
import (
- "bufio"
"bytes"
"context"
"crypto/ed25519"
@@ -1300,13 +1299,11 @@ func (a *systemApplier) workerDowngradeReasons() []string {
}
// nestedWorkersOptedIn reports whether the host.env at path sets
-// CARLOS_NESTED_WORKERS to exactly "on", read the way the agent's
-// EnvironmentFile=- reads it: comments and blank lines skipped, one pair of
-// matching quotes stripped, the last assignment winning. A missing file is
-// off (the agent then runs with the setting unset). A file that cannot be
-// read, or that uses syntax this reader cannot place (a continuation line,
-// an unterminated quote, an escape in the value), is an error: the caller
-// must not take it for off.
+// CARLOS_NESTED_WORKERS to exactly "on", read by parseHostEnvKeys, the
+// parser readClaimUnitSettings uses. A missing file is off (the agent's
+// EnvironmentFile=- then runs with the setting unset). A file that cannot
+// be read, or that the parser refuses, is an error: the caller must not
+// take it for off.
func nestedWorkersOptedIn(path string) (bool, error) {
raw, err := readClaimUnitFile(path)
if errors.Is(err, os.ErrNotExist) {
@@ -1315,33 +1312,11 @@ func nestedWorkersOptedIn(path string) (bool, error) {
if err != nil {
return false, err
}
- value := ""
- scan := bufio.NewScanner(bytes.NewReader(raw))
- for scan.Scan() {
- line := strings.TrimSpace(scan.Text())
- if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, ";") {
- continue
- }
- key, v, ok := strings.Cut(line, "=")
- key, v = strings.TrimSpace(key), strings.TrimSpace(v)
- if strings.HasSuffix(line, `\`) || (len(v) > 0 && (v[0] == '"' || v[0] == '\'') && (len(v) < 2 || v[len(v)-1] != v[0])) {
- return false, errors.New("unsupported multiline or unterminated host.env value")
- }
- if !ok || key != nestedWorkersEnv {
- continue
- }
- if len(v) >= 2 && ((v[0] == '"' && v[len(v)-1] == '"') || (v[0] == '\'' && v[len(v)-1] == '\'')) {
- v = v[1 : len(v)-1]
- }
- if strings.ContainsAny(v, "\\\"'\x00\r\n") {
- return false, errors.New("unsupported value for " + nestedWorkersEnv)
- }
- value = v
- }
- if err := scan.Err(); err != nil {
+ values, err := parseHostEnvKeys(raw, nestedWorkersEnv)
+ if err != nil {
return false, err
}
- return value == "on", nil
+ return values[nestedWorkersEnv] == "on", nil
}
// systemApplyLayoutRefusal takes the release and the rows in its way.