Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion internal/dns/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"os"
"os/exec"
"path/filepath"
"regexp"
"runtime"
"strings"
"time"
Expand Down Expand Up @@ -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.
Expand All @@ -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
}
Expand Down
24 changes: 24 additions & 0 deletions internal/dns/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions internal/hermes/hermes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions internal/hermes/hermes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 28 additions & 0 deletions internal/openclaw/onboard_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
8 changes: 8 additions & 0 deletions internal/openclaw/openclaw.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion internal/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down