diff --git a/internal/dns/resolver.go b/internal/dns/resolver.go index e13a3aa6..f57acc3a 100644 --- a/internal/dns/resolver.go +++ b/internal/dns/resolver.go @@ -18,6 +18,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "runtime" "strings" "time" @@ -120,6 +121,19 @@ const ( hostsFile = "/etc/hosts" ) +// validHostnameRegex matches DNS-safe hostnames: labels of lowercase/uppercase +// alphanumerics and hyphens, joined by dots. No spaces, newlines, or other +// characters that could inject extra lines into /etc/hosts. +var validHostnameRegex = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`) + +// isValidHostname reports whether h is safe to write as an /etc/hosts entry. +// Belt-and-suspenders check: callers are expected to validate ids before +// they ever become a hostname, but this guards the file write regardless of +// how a malformed hostname got here. +func isValidHostname(h string) bool { + return validHostnameRegex.MatchString(h) +} + // EnsureHostsEntries adds /etc/hosts entries for the given hostnames. // Always includes "obol.stack" plus any additional hostnames (e.g. openclaw subdomains). // Entries are idempotent — existing managed block is replaced. @@ -129,7 +143,7 @@ func EnsureHostsEntries(hostnames []string) error { seen := map[string]bool{domain: true} for _, h := range hostnames { - if h != "" && !seen[h] { + if h != "" && !seen[h] && isValidHostname(h) { all = append(all, h) seen[h] = true } diff --git a/internal/dns/resolver_test.go b/internal/dns/resolver_test.go index 7c0024b2..16377d08 100644 --- a/internal/dns/resolver_test.go +++ b/internal/dns/resolver_test.go @@ -40,6 +40,30 @@ func TestGetNMDNSMode(t *testing.T) { _ = mode } +func TestIsValidHostname(t *testing.T) { + valid := []string{"obol.stack", "hermes-abc.obol.stack", "openclaw-my-agent.obol.stack", "a"} + for _, h := range valid { + if !isValidHostname(h) { + t.Errorf("isValidHostname(%q) = false, want true", h) + } + } + + // Belt-and-suspenders guard for the Canary402 audit finding: a hostname + // carrying a newline (e.g. from an unsanitized agent --id) must never be + // written to /etc/hosts, even if it slipped past upstream validation. + invalid := []string{ + "", + "evil\n127.0.0.1 attacker.com", + "has space", + "has/slash", + } + for _, h := range invalid { + if isValidHostname(h) { + t.Errorf("isValidHostname(%q) = true, want false", h) + } + } +} + func TestHasNMDnsmasqConfig(t *testing.T) { // On a clean system without obol config, this should return false // unless the test system has it installed diff --git a/internal/hermes/hermes.go b/internal/hermes/hermes.go index ca300253..b208c852 100644 --- a/internal/hermes/hermes.go +++ b/internal/hermes/hermes.go @@ -22,6 +22,7 @@ import ( "github.com/ObolNetwork/obol-stack/internal/model" "github.com/ObolNetwork/obol-stack/internal/tunnel" "github.com/ObolNetwork/obol-stack/internal/ui" + "github.com/ObolNetwork/obol-stack/internal/validate" petname "github.com/dustinkirkland/golang-petname" "gopkg.in/yaml.v3" ) @@ -106,6 +107,13 @@ func Onboard(cfg *config.Config, opts OnboardOptions, u *ui.UI) error { u.Infof("Using deployment ID: %s", id) } + // id becomes a DNS label (hostname, namespace) below, so it must be + // restricted to a safe charset — an unsanitized id (e.g. containing a + // newline) could otherwise inject arbitrary /etc/hosts entries. + if err := validate.Name(id); err != nil { + return fmt.Errorf("invalid agent id: %w", err) + } + deploymentDir := DeploymentPath(cfg, id) namespace := agentruntime.Namespace(agentruntime.Hermes, id) hostname := agentruntime.Hostname(agentruntime.Hermes, id) diff --git a/internal/hermes/hermes_test.go b/internal/hermes/hermes_test.go index 8ad58cc7..41a460b8 100644 --- a/internal/hermes/hermes_test.go +++ b/internal/hermes/hermes_test.go @@ -117,6 +117,27 @@ func TestOnboardDefaultAlreadyInstalledIsIdempotent(t *testing.T) { } } +// TestOnboardRejectsUnsafeID guards against the Canary402 full-surface audit +// finding: an unsanitized --id becomes a DNS label (hostname, namespace) and +// was written verbatim into /etc/hosts, so a newline in --id could inject +// arbitrary /etc/hosts lines via "sudo tee". Onboard must reject any id that +// isn't a safe DNS label before it reaches deployment/hostname construction. +func TestOnboardRejectsUnsafeID(t *testing.T) { + invalid := []string{ + "evil\n127.0.0.1 attacker.com", // /etc/hosts line injection + "has space", + "has/slash", + "-leading-dash", + } + for _, id := range invalid { + cfg := testConfig(t) + err := Onboard(cfg, OnboardOptions{ID: id}, newTestUI()) + if err == nil { + t.Errorf("Onboard(ID=%q) = nil, want error", id) + } + } +} + // TestGenerateConfig_PrimaryIsRoundTrippable guards the LiteLLM model_name // contract end-to-end: whatever string the agent's `model.default` is set to // MUST match a `model_name` entry in the LiteLLM ConfigMap byte-for-byte, diff --git a/internal/openclaw/onboard_test.go b/internal/openclaw/onboard_test.go new file mode 100644 index 00000000..d54b6679 --- /dev/null +++ b/internal/openclaw/onboard_test.go @@ -0,0 +1,28 @@ +package openclaw + +import ( + "testing" + + "github.com/ObolNetwork/obol-stack/internal/ui" +) + +// TestOnboardRejectsUnsafeID guards against the Canary402 full-surface audit +// finding: an unsanitized --id becomes a DNS label (hostname, namespace) and +// was written verbatim into /etc/hosts, so a newline in --id could inject +// arbitrary /etc/hosts lines via "sudo tee". Onboard must reject any id that +// isn't a safe DNS label before it reaches deployment/hostname construction. +func TestOnboardRejectsUnsafeID(t *testing.T) { + invalid := []string{ + "evil\n127.0.0.1 attacker.com", // /etc/hosts line injection + "has space", + "has/slash", + "-leading-dash", + } + for _, id := range invalid { + cfg := testConfig(t) + err := Onboard(cfg, OnboardOptions{ID: id}, ui.New(false)) + if err == nil { + t.Errorf("Onboard(ID=%q) = nil, want error", id) + } + } +} diff --git a/internal/openclaw/openclaw.go b/internal/openclaw/openclaw.go index 3fba946c..1f03338f 100644 --- a/internal/openclaw/openclaw.go +++ b/internal/openclaw/openclaw.go @@ -28,6 +28,7 @@ import ( "github.com/ObolNetwork/obol-stack/internal/model" "github.com/ObolNetwork/obol-stack/internal/tunnel" "github.com/ObolNetwork/obol-stack/internal/ui" + "github.com/ObolNetwork/obol-stack/internal/validate" petname "github.com/dustinkirkland/golang-petname" ) @@ -168,6 +169,13 @@ func Onboard(cfg *config.Config, opts OnboardOptions, u *ui.UI) error { u.Infof("Using deployment ID: %s", id) } + // id becomes a DNS label (hostname, namespace) below, so it must be + // restricted to a safe charset — an unsanitized id (e.g. containing a + // newline) could otherwise inject arbitrary /etc/hosts entries. + if err := validate.Name(id); err != nil { + return fmt.Errorf("invalid agent id: %w", err) + } + deploymentDir := DeploymentPath(cfg, id) // Idempotent re-run for default deployment: just re-sync diff --git a/internal/validate/validate_test.go b/internal/validate/validate_test.go index 5421206a..744eec41 100644 --- a/internal/validate/validate_test.go +++ b/internal/validate/validate_test.go @@ -18,8 +18,10 @@ func TestName(t *testing.T) { "-leading-hyphen", // starts with hyphen "has spaces", "has_underscore", - "../etc/passwd", // path traversal + "../etc/passwd", // path traversal "a" + string(make([]byte, 63)), // too long (64 chars) + "has\nnewline", // /etc/hosts injection (Canary402 agent --id finding) + "has/slash", } for _, s := range invalid { if err := Name(s); err == nil {