refactor(security): guard binds in resolveServiceBinds and add childenv.CommandContext
Apply the runtime guard inside resolveServiceBinds so every caller, including createAndStart, is guarded; drop resolveGuardedBinds, sweepAppSecretFiles, and homePath. Route every daemon exec site, including pass/sops IsAvailable probes, through childenv.CommandContext. Make the protected-path test host-agnostic, test readFileBeneath under PR_SET_DUMPABLE=0, and merge or tighten low-value tests.
- commit
- a320e99f9cc888fa4f1549561e363e9af4ba8893
- author
- brice · · 2026-10-10 01:11 UTC
- parent
- 01ec9bd
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
20 files · +95 -115
M internal/adapters/out/appsecrets/store.go +2 -5
| @@ -7,7 +7,6 @@ package appsecrets | |||
| 7 | 7 | import ( | |
| 8 | 8 | "context" | |
| 9 | 9 | "fmt" | |
| 10 | - | "os/exec" | |
| 11 | 10 | "strings" | |
| 12 | 11 | "time" | |
| 13 | 12 | ||
| @@ -41,8 +40,7 @@ func (s *Store) SetSecret(ctx context.Context, path, value string) error { | |||
| 41 | 40 | } | |
| 42 | 41 | timeoutCtx, cancel := context.WithTimeout(ctx, s.timeout) | |
| 43 | 42 | defer cancel() | |
| 44 | - | cmd := exec.CommandContext(timeoutCtx, "pass", "insert", "-m", "-f", path) //nolint:gosec // binary is constant ("pass"); path validated below | |
| 45 | - | cmd.Env = childenv.Env() | |
| 43 | + | cmd := childenv.CommandContext(timeoutCtx, "pass", "insert", "-m", "-f", path) | |
| 46 | 44 | cmd.Stdin = strings.NewReader(value) | |
| 47 | 45 | output, err := cmd.CombinedOutput() | |
| 48 | 46 | if err != nil { | |
| @@ -59,8 +57,7 @@ func (s *Store) DeleteSecret(ctx context.Context, path string) error { | |||
| 59 | 57 | } | |
| 60 | 58 | timeoutCtx, cancel := context.WithTimeout(ctx, s.timeout) | |
| 61 | 59 | defer cancel() | |
| 62 | - | cmd := exec.CommandContext(timeoutCtx, "pass", "rm", "-f", path) //nolint:gosec // binary is constant ("pass"); path validated below | |
| 63 | - | cmd.Env = childenv.Env() | |
| 60 | + | cmd := childenv.CommandContext(timeoutCtx, "pass", "rm", "-f", path) | |
| 64 | 61 | output, err := cmd.CombinedOutput() | |
| 65 | 62 | if err != nil { | |
| 66 | 63 | return fmt.Errorf("appsecrets: pass rm failed: %s: %w", strings.TrimSpace(string(output)), err) | |
| @@ -7,7 +7,6 @@ package appsecrets | |||||
| 7 | import ( | 7 | import ( | ||
| 8 | "context" | 8 | "context" | ||
| 9 | "fmt" | 9 | "fmt" | ||
| 10 | - | "os/exec" | |||
| 11 | "strings" | 10 | "strings" | ||
| 12 | "time" | 11 | "time" | ||
| 13 | 12 | ||||
| @@ -41,8 +40,7 @@ func (s *Store) SetSecret(ctx context.Context, path, value string) error { | |||||
| 41 | } | 40 | } | ||
| 42 | timeoutCtx, cancel := context.WithTimeout(ctx, s.timeout) | 41 | timeoutCtx, cancel := context.WithTimeout(ctx, s.timeout) | ||
| 43 | defer cancel() | 42 | defer cancel() | ||
| 44 | - | cmd := exec.CommandContext(timeoutCtx, "pass", "insert", "-m", "-f", path) //nolint:gosec // binary is constant ("pass"); path validated below | 43 | + | cmd := childenv.CommandContext(timeoutCtx, "pass", "insert", "-m", "-f", path) |
| 45 | - | cmd.Env = childenv.Env() | |||
| 46 | cmd.Stdin = strings.NewReader(value) | 44 | cmd.Stdin = strings.NewReader(value) | ||
| 47 | output, err := cmd.CombinedOutput() | 45 | output, err := cmd.CombinedOutput() | ||
| 48 | if err != nil { | 46 | if err != nil { | ||
| @@ -59,8 +57,7 @@ func (s *Store) DeleteSecret(ctx context.Context, path string) error { | |||||
| 59 | } | 57 | } | ||
| 60 | timeoutCtx, cancel := context.WithTimeout(ctx, s.timeout) | 58 | timeoutCtx, cancel := context.WithTimeout(ctx, s.timeout) | ||
| 61 | defer cancel() | 59 | defer cancel() | ||
| 62 | - | cmd := exec.CommandContext(timeoutCtx, "pass", "rm", "-f", path) //nolint:gosec // binary is constant ("pass"); path validated below | 60 | + | cmd := childenv.CommandContext(timeoutCtx, "pass", "rm", "-f", path) |
| 63 | - | cmd.Env = childenv.Env() | |||
| 64 | output, err := cmd.CombinedOutput() | 61 | output, err := cmd.CombinedOutput() | ||
| 65 | if err != nil { | 62 | if err != nil { | ||
| 66 | return fmt.Errorf("appsecrets: pass rm failed: %s: %w", strings.TrimSpace(string(output)), err) | 63 | return fmt.Errorf("appsecrets: pass rm failed: %s: %w", strings.TrimSpace(string(output)), err) | ||
M internal/adapters/out/secrets/pass.go +2 -4
| @@ -81,8 +81,7 @@ func (p *PassProvider) GetSecret(ctx context.Context, path string) (string, erro | |||
| 81 | 81 | ctx, cancel := context.WithTimeout(ctx, p.timeout) | |
| 82 | 82 | defer cancel() | |
| 83 | 83 | ||
| 84 | - | cmd := exec.CommandContext(ctx, "pass", "show", path) //nolint:gosec // binary is constant ("pass"); arguments are validated secret paths | |
| 85 | - | cmd.Env = childenv.Env() | |
| 84 | + | cmd := childenv.CommandContext(ctx, "pass", "show", path) | |
| 86 | 85 | output, err := cmd.Output() | |
| 87 | 86 | if err != nil { | |
| 88 | 87 | if exitError, ok := err.(*exec.ExitError); ok { | |
| @@ -111,7 +110,6 @@ func (p *PassProvider) GetSecret(ctx context.Context, path string) (string, erro | |||
| 111 | 110 | func (p *PassProvider) IsAvailable() bool { | |
| 112 | 111 | ctx, cancel := context.WithTimeout(context.Background(), p.timeout) | |
| 113 | 112 | defer cancel() | |
| 114 | - | cmd := exec.CommandContext(ctx, "pass", "version") //nolint:gosec // binary is a constant ("pass"), no user input | |
| 115 | - | cmd.Env = childenv.Env() | |
| 113 | + | cmd := childenv.CommandContext(ctx, "pass", "version") | |
| 116 | 114 | return cmd.Run() == nil | |
| 117 | 115 | } | |
| @@ -81,8 +81,7 @@ func (p *PassProvider) GetSecret(ctx context.Context, path string) (string, erro | |||||
| 81 | ctx, cancel := context.WithTimeout(ctx, p.timeout) | 81 | ctx, cancel := context.WithTimeout(ctx, p.timeout) | ||
| 82 | defer cancel() | 82 | defer cancel() | ||
| 83 | 83 | ||||
| 84 | - | cmd := exec.CommandContext(ctx, "pass", "show", path) //nolint:gosec // binary is constant ("pass"); arguments are validated secret paths | 84 | + | cmd := childenv.CommandContext(ctx, "pass", "show", path) |
| 85 | - | cmd.Env = childenv.Env() | |||
| 86 | output, err := cmd.Output() | 85 | output, err := cmd.Output() | ||
| 87 | if err != nil { | 86 | if err != nil { | ||
| 88 | if exitError, ok := err.(*exec.ExitError); ok { | 87 | if exitError, ok := err.(*exec.ExitError); ok { | ||
| @@ -111,7 +110,6 @@ func (p *PassProvider) GetSecret(ctx context.Context, path string) (string, erro | |||||
| 111 | func (p *PassProvider) IsAvailable() bool { | 110 | func (p *PassProvider) IsAvailable() bool { | ||
| 112 | ctx, cancel := context.WithTimeout(context.Background(), p.timeout) | 111 | ctx, cancel := context.WithTimeout(context.Background(), p.timeout) | ||
| 113 | defer cancel() | 112 | defer cancel() | ||
| 114 | - | cmd := exec.CommandContext(ctx, "pass", "version") //nolint:gosec // binary is a constant ("pass"), no user input | 113 | + | cmd := childenv.CommandContext(ctx, "pass", "version") |
| 115 | - | cmd.Env = childenv.Env() | |||
| 116 | return cmd.Run() == nil | 114 | return cmd.Run() == nil | ||
| 117 | } | 115 | } | ||
M internal/adapters/out/secrets/sops.go +2 -3
| @@ -110,8 +110,7 @@ func (s *SopsProvider) GetSecret(ctx context.Context, path string) (string, erro | |||
| 110 | 110 | ||
| 111 | 111 | // Use sops -d --extract to get specific key | |
| 112 | 112 | // #nosec G204 - filePath and keyPath are validated above | |
| 113 | - | cmd := exec.CommandContext(ctx, "sops", "-d", "--extract", extractPath, cleanPath) | |
| 114 | - | cmd.Env = childenv.Env() | |
| 113 | + | cmd := childenv.CommandContext(ctx, "sops", "-d", "--extract", extractPath, cleanPath) | |
| 115 | 114 | output, err := cmd.Output() | |
| 116 | 115 | if err != nil { | |
| 117 | 116 | if exitError, ok := err.(*exec.ExitError); ok { | |
| @@ -137,7 +136,7 @@ func (s *SopsProvider) GetSecret(ctx context.Context, path string) (string, erro | |||
| 137 | 136 | ||
| 138 | 137 | // IsAvailable checks if sops is available in the system. | |
| 139 | 138 | func (s *SopsProvider) IsAvailable() bool { | |
| 140 | - | cmd := exec.Command("sops", "--version") | |
| 139 | + | cmd := childenv.CommandContext(context.Background(), "sops", "--version") | |
| 141 | 140 | err := cmd.Run() | |
| 142 | 141 | return err == nil | |
| 143 | 142 | } | |
| @@ -110,8 +110,7 @@ func (s *SopsProvider) GetSecret(ctx context.Context, path string) (string, erro | |||||
| 110 | 110 | ||||
| 111 | // Use sops -d --extract to get specific key | 111 | // Use sops -d --extract to get specific key | ||
| 112 | // #nosec G204 - filePath and keyPath are validated above | 112 | // #nosec G204 - filePath and keyPath are validated above | ||
| 113 | - | cmd := exec.CommandContext(ctx, "sops", "-d", "--extract", extractPath, cleanPath) | 113 | + | cmd := childenv.CommandContext(ctx, "sops", "-d", "--extract", extractPath, cleanPath) |
| 114 | - | cmd.Env = childenv.Env() | |||
| 115 | output, err := cmd.Output() | 114 | output, err := cmd.Output() | ||
| 116 | if err != nil { | 115 | if err != nil { | ||
| 117 | if exitError, ok := err.(*exec.ExitError); ok { | 116 | if exitError, ok := err.(*exec.ExitError); ok { | ||
| @@ -137,7 +136,7 @@ func (s *SopsProvider) GetSecret(ctx context.Context, path string) (string, erro | |||||
| 137 | 136 | ||||
| 138 | // IsAvailable checks if sops is available in the system. | 137 | // IsAvailable checks if sops is available in the system. | ||
| 139 | func (s *SopsProvider) IsAvailable() bool { | 138 | func (s *SopsProvider) IsAvailable() bool { | ||
| 140 | - | cmd := exec.Command("sops", "--version") | 139 | + | cmd := childenv.CommandContext(context.Background(), "sops", "--version") |
| 141 | err := cmd.Run() | 140 | err := cmd.Run() | ||
| 142 | return err == nil | 141 | return err == nil | ||
| 143 | } | 142 | } | ||
M internal/adapters/out/tokenstore/pass.go +6 -12
| @@ -5,7 +5,6 @@ import ( | |||
| 5 | 5 | "context" | |
| 6 | 6 | "encoding/json" | |
| 7 | 7 | "fmt" | |
| 8 | - | "os/exec" | |
| 9 | 8 | "regexp" | |
| 10 | 9 | "strings" | |
| 11 | 10 | "sync" | |
| @@ -338,8 +337,7 @@ func (s *PassStore) ListTokens(ctx context.Context) ([]domain.Token, error) { | |||
| 338 | 337 | defer cancel() | |
| 339 | 338 | ||
| 340 | 339 | // List all entries under the token path | |
| 341 | - | cmd := exec.CommandContext(ctx, "pass", "ls", passTokenPath) //nolint:gosec // passTokenPath is a fixed constant | |
| 342 | - | cmd.Env = childenv.Env() | |
| 340 | + | cmd := childenv.CommandContext(ctx, "pass", "ls", passTokenPath) | |
| 343 | 341 | output, err := cmd.Output() | |
| 344 | 342 | if err != nil { | |
| 345 | 343 | // If the path doesn't exist, return empty list | |
| @@ -485,16 +483,14 @@ func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | |||
| 485 | 483 | ||
| 486 | 484 | // Remove JWT | |
| 487 | 485 | // #nosec G204 -- command and args are fixed; subject is treated as data only. | |
| 488 | - | cmd := exec.CommandContext(ctx, "pass", "rm", "-f", jwtPath) | |
| 489 | - | cmd.Env = childenv.Env() | |
| 486 | + | cmd := childenv.CommandContext(ctx, "pass", "rm", "-f", jwtPath) | |
| 490 | 487 | if err := cmd.Run(); err != nil { | |
| 491 | 488 | return fmt.Errorf("failed to delete token: %w", err) | |
| 492 | 489 | } | |
| 493 | 490 | ||
| 494 | 491 | // Remove metadata | |
| 495 | 492 | // #nosec G204 -- command and args are fixed; subject is treated as data only. | |
| 496 | - | cmd = exec.CommandContext(ctx, "pass", "rm", "-f", metaPath) | |
| 497 | - | cmd.Env = childenv.Env() | |
| 493 | + | cmd = childenv.CommandContext(ctx, "pass", "rm", "-f", metaPath) | |
| 498 | 494 | if err := cmd.Run(); err != nil { | |
| 499 | 495 | s.log.Warn().Err(err).Msg("failed to delete token metadata") | |
| 500 | 496 | } | |
| @@ -515,8 +511,7 @@ func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | |||
| 515 | 511 | ||
| 516 | 512 | // passInsert inserts a value into pass. | |
| 517 | 513 | func (s *PassStore) passInsert(ctx context.Context, path, value string) error { | |
| 518 | - | cmd := exec.CommandContext(ctx, "pass", "insert", "-m", "-f", path) //nolint:gosec // binary is constant ("pass"); path arguments are sanitized token subjects | |
| 519 | - | cmd.Env = childenv.Env() | |
| 514 | + | cmd := childenv.CommandContext(ctx, "pass", "insert", "-m", "-f", path) | |
| 520 | 515 | cmd.Stdin = strings.NewReader(value) | |
| 521 | 516 | output, err := cmd.CombinedOutput() | |
| 522 | 517 | if err != nil { | |
| @@ -527,8 +522,7 @@ func (s *PassStore) passInsert(ctx context.Context, path, value string) error { | |||
| 527 | 522 | ||
| 528 | 523 | // passShow retrieves a value from pass. | |
| 529 | 524 | func (s *PassStore) passShow(ctx context.Context, path string) (string, error) { | |
| 530 | - | cmd := exec.CommandContext(ctx, "pass", "show", path) //nolint:gosec // binary is constant ("pass"); path arguments are sanitized token subjects | |
| 531 | - | cmd.Env = childenv.Env() | |
| 525 | + | cmd := childenv.CommandContext(ctx, "pass", "show", path) | |
| 532 | 526 | output, err := cmd.Output() | |
| 533 | 527 | if err != nil { | |
| 534 | 528 | return "", fmt.Errorf("pass show failed: %w", err) | |
| @@ -554,6 +548,6 @@ func (s *PassStore) getRevokedList(ctx context.Context) ([]string, error) { | |||
| 554 | 548 | ||
| 555 | 549 | // IsAvailable checks if pass is available in the system. | |
| 556 | 550 | func (s *PassStore) IsAvailable() bool { | |
| 557 | - | cmd := exec.Command("pass", "version") | |
| 551 | + | cmd := childenv.CommandContext(context.Background(), "pass", "version") | |
| 558 | 552 | return cmd.Run() == nil | |
| 559 | 553 | } | |
| @@ -5,7 +5,6 @@ import ( | |||||
| 5 | "context" | 5 | "context" | ||
| 6 | "encoding/json" | 6 | "encoding/json" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | - | "os/exec" | |||
| 9 | "regexp" | 8 | "regexp" | ||
| 10 | "strings" | 9 | "strings" | ||
| 11 | "sync" | 10 | "sync" | ||
| @@ -338,8 +337,7 @@ func (s *PassStore) ListTokens(ctx context.Context) ([]domain.Token, error) { | |||||
| 338 | defer cancel() | 337 | defer cancel() | ||
| 339 | 338 | ||||
| 340 | // List all entries under the token path | 339 | // List all entries under the token path | ||
| 341 | - | cmd := exec.CommandContext(ctx, "pass", "ls", passTokenPath) //nolint:gosec // passTokenPath is a fixed constant | 340 | + | cmd := childenv.CommandContext(ctx, "pass", "ls", passTokenPath) |
| 342 | - | cmd.Env = childenv.Env() | |||
| 343 | output, err := cmd.Output() | 341 | output, err := cmd.Output() | ||
| 344 | if err != nil { | 342 | if err != nil { | ||
| 345 | // If the path doesn't exist, return empty list | 343 | // If the path doesn't exist, return empty list | ||
| @@ -485,16 +483,14 @@ func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | |||||
| 485 | 483 | ||||
| 486 | // Remove JWT | 484 | // Remove JWT | ||
| 487 | // #nosec G204 -- command and args are fixed; subject is treated as data only. | 485 | // #nosec G204 -- command and args are fixed; subject is treated as data only. | ||
| 488 | - | cmd := exec.CommandContext(ctx, "pass", "rm", "-f", jwtPath) | 486 | + | cmd := childenv.CommandContext(ctx, "pass", "rm", "-f", jwtPath) |
| 489 | - | cmd.Env = childenv.Env() | |||
| 490 | if err := cmd.Run(); err != nil { | 487 | if err := cmd.Run(); err != nil { | ||
| 491 | return fmt.Errorf("failed to delete token: %w", err) | 488 | return fmt.Errorf("failed to delete token: %w", err) | ||
| 492 | } | 489 | } | ||
| 493 | 490 | ||||
| 494 | // Remove metadata | 491 | // Remove metadata | ||
| 495 | // #nosec G204 -- command and args are fixed; subject is treated as data only. | 492 | // #nosec G204 -- command and args are fixed; subject is treated as data only. | ||
| 496 | - | cmd = exec.CommandContext(ctx, "pass", "rm", "-f", metaPath) | 493 | + | cmd = childenv.CommandContext(ctx, "pass", "rm", "-f", metaPath) |
| 497 | - | cmd.Env = childenv.Env() | |||
| 498 | if err := cmd.Run(); err != nil { | 494 | if err := cmd.Run(); err != nil { | ||
| 499 | s.log.Warn().Err(err).Msg("failed to delete token metadata") | 495 | s.log.Warn().Err(err).Msg("failed to delete token metadata") | ||
| 500 | } | 496 | } | ||
| @@ -515,8 +511,7 @@ func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | |||||
| 515 | 511 | ||||
| 516 | // passInsert inserts a value into pass. | 512 | // passInsert inserts a value into pass. | ||
| 517 | func (s *PassStore) passInsert(ctx context.Context, path, value string) error { | 513 | func (s *PassStore) passInsert(ctx context.Context, path, value string) error { | ||
| 518 | - | cmd := exec.CommandContext(ctx, "pass", "insert", "-m", "-f", path) //nolint:gosec // binary is constant ("pass"); path arguments are sanitized token subjects | 514 | + | cmd := childenv.CommandContext(ctx, "pass", "insert", "-m", "-f", path) |
| 519 | - | cmd.Env = childenv.Env() | |||
| 520 | cmd.Stdin = strings.NewReader(value) | 515 | cmd.Stdin = strings.NewReader(value) | ||
| 521 | output, err := cmd.CombinedOutput() | 516 | output, err := cmd.CombinedOutput() | ||
| 522 | if err != nil { | 517 | if err != nil { | ||
| @@ -527,8 +522,7 @@ func (s *PassStore) passInsert(ctx context.Context, path, value string) error { | |||||
| 527 | 522 | ||||
| 528 | // passShow retrieves a value from pass. | 523 | // passShow retrieves a value from pass. | ||
| 529 | func (s *PassStore) passShow(ctx context.Context, path string) (string, error) { | 524 | func (s *PassStore) passShow(ctx context.Context, path string) (string, error) { | ||
| 530 | - | cmd := exec.CommandContext(ctx, "pass", "show", path) //nolint:gosec // binary is constant ("pass"); path arguments are sanitized token subjects | 525 | + | cmd := childenv.CommandContext(ctx, "pass", "show", path) |
| 531 | - | cmd.Env = childenv.Env() | |||
| 532 | output, err := cmd.Output() | 526 | output, err := cmd.Output() | ||
| 533 | if err != nil { | 527 | if err != nil { | ||
| 534 | return "", fmt.Errorf("pass show failed: %w", err) | 528 | return "", fmt.Errorf("pass show failed: %w", err) | ||
| @@ -554,6 +548,6 @@ func (s *PassStore) getRevokedList(ctx context.Context) ([]string, error) { | |||||
| 554 | 548 | ||||
| 555 | // IsAvailable checks if pass is available in the system. | 549 | // IsAvailable checks if pass is available in the system. | ||
| 556 | func (s *PassStore) IsAvailable() bool { | 550 | func (s *PassStore) IsAvailable() bool { | ||
| 557 | - | cmd := exec.Command("pass", "version") | 551 | + | cmd := childenv.CommandContext(context.Background(), "pass", "version") |
| 558 | return cmd.Run() == nil | 552 | return cmd.Run() == nil | ||
| 559 | } | 553 | } | ||
M internal/app/dumpable_linux_test.go +5 -9
| @@ -3,7 +3,6 @@ | |||
| 3 | 3 | package app | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | - | "fmt" | |
| 7 | 6 | "os" | |
| 8 | 7 | "path/filepath" | |
| 9 | 8 | "testing" | |
| @@ -21,14 +20,11 @@ func TestDisableDumpable(t *testing.T) { | |||
| 21 | 20 | require.NoError(t, err) | |
| 22 | 21 | assert.Equal(t, 0, got) | |
| 23 | 22 | ||
| 24 | - | // readFileBeneath reads secrets through /proc/self/fd/N; the kernel | |
| 25 | - | // keeps that self access for a non-dumpable process. | |
| 26 | - | path := filepath.Join(t.TempDir(), "secret") | |
| 27 | - | require.NoError(t, os.WriteFile(path, []byte("value"), 0o600)) | |
| 28 | - | fd, err := unix.Open(path, unix.O_RDONLY|unix.O_CLOEXEC, 0) | |
| 29 | - | require.NoError(t, err) | |
| 30 | - | defer unix.Close(fd) | |
| 31 | - | data, err := os.ReadFile(fmt.Sprintf("/proc/self/fd/%d", fd)) | |
| 23 | + | // readFileBeneath reads secrets through /proc/self/fd/N; it must keep | |
| 24 | + | // working once the process is non-dumpable. | |
| 25 | + | root := t.TempDir() | |
| 26 | + | require.NoError(t, os.WriteFile(filepath.Join(root, "secret"), []byte("value"), 0o600)) | |
| 27 | + | data, err := readFileBeneath(root, "secret") | |
| 32 | 28 | require.NoError(t, err) | |
| 33 | 29 | assert.Equal(t, "value", string(data)) | |
| 34 | 30 | } | |
| @@ -3,7 +3,6 @@ | |||||
| 3 | package app | 3 | package app | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | - | "fmt" | |||
| 7 | "os" | 6 | "os" | ||
| 8 | "path/filepath" | 7 | "path/filepath" | ||
| 9 | "testing" | 8 | "testing" | ||
| @@ -21,14 +20,11 @@ func TestDisableDumpable(t *testing.T) { | |||||
| 21 | require.NoError(t, err) | 20 | require.NoError(t, err) | ||
| 22 | assert.Equal(t, 0, got) | 21 | assert.Equal(t, 0, got) | ||
| 23 | 22 | ||||
| 24 | - | // readFileBeneath reads secrets through /proc/self/fd/N; the kernel | 23 | + | // readFileBeneath reads secrets through /proc/self/fd/N; it must keep |
| 25 | - | // keeps that self access for a non-dumpable process. | 24 | + | // working once the process is non-dumpable. |
| 26 | - | path := filepath.Join(t.TempDir(), "secret") | 25 | + | root := t.TempDir() |
| 27 | - | require.NoError(t, os.WriteFile(path, []byte("value"), 0o600)) | 26 | + | require.NoError(t, os.WriteFile(filepath.Join(root, "secret"), []byte("value"), 0o600)) |
| 28 | - | fd, err := unix.Open(path, unix.O_RDONLY|unix.O_CLOEXEC, 0) | 27 | + | data, err := readFileBeneath(root, "secret") |
| 29 | - | require.NoError(t, err) | |||
| 30 | - | defer unix.Close(fd) | |||
| 31 | - | data, err := os.ReadFile(fmt.Sprintf("/proc/self/fd/%d", fd)) | |||
| 32 | require.NoError(t, err) | 28 | require.NoError(t, err) | ||
| 33 | assert.Equal(t, "value", string(data)) | 29 | assert.Equal(t, "value", string(data)) | ||
| 34 | } | 30 | } | ||
M internal/app/runtime_guard.go +7 -13
| @@ -20,7 +20,7 @@ func protectedRuntimePaths(dataDir string, gordonPaths []string, getenv func(str | |||
| 20 | 20 | runtimeDir := getenv("XDG_RUNTIME_DIR") | |
| 21 | 21 | configHome := getenv("XDG_CONFIG_HOME") | |
| 22 | 22 | if configHome == "" { | |
| 23 | - | configHome = homePath(home, ".config") | |
| 23 | + | configHome = subPath(home, ".config") | |
| 24 | 24 | } | |
| 25 | 25 | dockerHost, _ := strings.CutPrefix(getenv("DOCKER_HOST"), "unix://") | |
| 26 | 26 | if strings.Contains(dockerHost, "://") { | |
| @@ -34,18 +34,18 @@ func protectedRuntimePaths(dataDir string, gordonPaths []string, getenv func(str | |||
| 34 | 34 | subPath(runtimeDir, "docker.sock"), | |
| 35 | 35 | subPath(runtimeDir, "podman", "podman.sock"), | |
| 36 | 36 | subPath(runtimeDir, "gordon"), | |
| 37 | - | homePath(home, ".gordon", "run"), | |
| 37 | + | subPath(home, ".gordon", "run"), | |
| 38 | 38 | dockerHost, | |
| 39 | - | homePath(home, ".local", "share", "containers"), | |
| 40 | - | homePath(home, ".local", "share", "docker"), | |
| 39 | + | subPath(home, ".local", "share", "containers"), | |
| 40 | + | subPath(home, ".local", "share", "docker"), | |
| 41 | 41 | subPath(configHome, "containers"), | |
| 42 | - | homePath(home, ".docker"), | |
| 42 | + | subPath(home, ".docker"), | |
| 43 | 43 | subPath(configHome, "systemd", "user"), | |
| 44 | 44 | getenv("GORDON_CLOUDFLARE_API_TOKEN_FILE"), | |
| 45 | 45 | getenv("GNUPGHOME"), | |
| 46 | - | homePath(home, ".gnupg"), | |
| 46 | + | subPath(home, ".gnupg"), | |
| 47 | 47 | getenv("PASSWORD_STORE_DIR"), | |
| 48 | - | homePath(home, ".password-store"), | |
| 48 | + | subPath(home, ".password-store"), | |
| 49 | 49 | getenv("SOPS_AGE_KEY_FILE"), | |
| 50 | 50 | subPath(configHome, "sops", "age"), | |
| 51 | 51 | }, gordonPaths...) | |
| @@ -101,12 +101,6 @@ func subPath(base string, elems ...string) string { | |||
| 101 | 101 | return filepath.Join(append([]string{base}, elems...)...) | |
| 102 | 102 | } | |
| 103 | 103 | ||
| 104 | - | // homePath joins elems under the home directory, or returns "" when the | |
| 105 | - | // home directory is unknown. | |
| 106 | - | func homePath(home string, elems ...string) string { | |
| 107 | - | return subPath(home, elems...) | |
| 108 | - | } | |
| 109 | - | ||
| 110 | 104 | // configuredGordonPaths returns the config file, the runtime socket in use, | |
| 111 | 105 | // and every private file or directory the config points Gordon at. Log | |
| 112 | 106 | // paths are left to gordonLogDirs. The public TLS certificate is left out | |
| @@ -20,7 +20,7 @@ func protectedRuntimePaths(dataDir string, gordonPaths []string, getenv func(str | |||||
| 20 | runtimeDir := getenv("XDG_RUNTIME_DIR") | 20 | runtimeDir := getenv("XDG_RUNTIME_DIR") | ||
| 21 | configHome := getenv("XDG_CONFIG_HOME") | 21 | configHome := getenv("XDG_CONFIG_HOME") | ||
| 22 | if configHome == "" { | 22 | if configHome == "" { | ||
| 23 | - | configHome = homePath(home, ".config") | 23 | + | configHome = subPath(home, ".config") |
| 24 | } | 24 | } | ||
| 25 | dockerHost, _ := strings.CutPrefix(getenv("DOCKER_HOST"), "unix://") | 25 | dockerHost, _ := strings.CutPrefix(getenv("DOCKER_HOST"), "unix://") | ||
| 26 | if strings.Contains(dockerHost, "://") { | 26 | if strings.Contains(dockerHost, "://") { | ||
| @@ -34,18 +34,18 @@ func protectedRuntimePaths(dataDir string, gordonPaths []string, getenv func(str | |||||
| 34 | subPath(runtimeDir, "docker.sock"), | 34 | subPath(runtimeDir, "docker.sock"), | ||
| 35 | subPath(runtimeDir, "podman", "podman.sock"), | 35 | subPath(runtimeDir, "podman", "podman.sock"), | ||
| 36 | subPath(runtimeDir, "gordon"), | 36 | subPath(runtimeDir, "gordon"), | ||
| 37 | - | homePath(home, ".gordon", "run"), | 37 | + | subPath(home, ".gordon", "run"), |
| 38 | dockerHost, | 38 | dockerHost, | ||
| 39 | - | homePath(home, ".local", "share", "containers"), | 39 | + | subPath(home, ".local", "share", "containers"), |
| 40 | - | homePath(home, ".local", "share", "docker"), | 40 | + | subPath(home, ".local", "share", "docker"), |
| 41 | subPath(configHome, "containers"), | 41 | subPath(configHome, "containers"), | ||
| 42 | - | homePath(home, ".docker"), | 42 | + | subPath(home, ".docker"), |
| 43 | subPath(configHome, "systemd", "user"), | 43 | subPath(configHome, "systemd", "user"), | ||
| 44 | getenv("GORDON_CLOUDFLARE_API_TOKEN_FILE"), | 44 | getenv("GORDON_CLOUDFLARE_API_TOKEN_FILE"), | ||
| 45 | getenv("GNUPGHOME"), | 45 | getenv("GNUPGHOME"), | ||
| 46 | - | homePath(home, ".gnupg"), | 46 | + | subPath(home, ".gnupg"), |
| 47 | getenv("PASSWORD_STORE_DIR"), | 47 | getenv("PASSWORD_STORE_DIR"), | ||
| 48 | - | homePath(home, ".password-store"), | 48 | + | subPath(home, ".password-store"), |
| 49 | getenv("SOPS_AGE_KEY_FILE"), | 49 | getenv("SOPS_AGE_KEY_FILE"), | ||
| 50 | subPath(configHome, "sops", "age"), | 50 | subPath(configHome, "sops", "age"), | ||
| 51 | }, gordonPaths...) | 51 | }, gordonPaths...) | ||
| @@ -101,12 +101,6 @@ func subPath(base string, elems ...string) string { | |||||
| 101 | return filepath.Join(append([]string{base}, elems...)...) | 101 | return filepath.Join(append([]string{base}, elems...)...) | ||
| 102 | } | 102 | } | ||
| 103 | 103 | ||||
| 104 | - | // homePath joins elems under the home directory, or returns "" when the | |||
| 105 | - | // home directory is unknown. | |||
| 106 | - | func homePath(home string, elems ...string) string { | |||
| 107 | - | return subPath(home, elems...) | |||
| 108 | - | } | |||
| 109 | - | ||||
| 110 | // configuredGordonPaths returns the config file, the runtime socket in use, | 104 | // configuredGordonPaths returns the config file, the runtime socket in use, | ||
| 111 | // and every private file or directory the config points Gordon at. Log | 105 | // and every private file or directory the config points Gordon at. Log | ||
| 112 | // paths are left to gordonLogDirs. The public TLS certificate is left out | 106 | // paths are left to gordonLogDirs. The public TLS certificate is left out | ||
M internal/app/runtime_guard_test.go +9 -1
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "github.com/stretchr/testify/assert" | |
| 9 | 9 | "github.com/stretchr/testify/require" | |
| 10 | 10 | ||
| 11 | + | "github.com/bnema/gordon/internal/adapters/out/secretfiles" | |
| 11 | 12 | "github.com/bnema/gordon/internal/domain" | |
| 12 | 13 | ) | |
| 13 | 14 | ||
| @@ -60,8 +61,14 @@ func TestProtectedRuntimePaths_AllSources(t *testing.T) { | |||
| 60 | 61 | } | |
| 61 | 62 | ||
| 62 | 63 | func TestProtectedRuntimePaths_SkipsEmptyAndTCP(t *testing.T) { | |
| 64 | + | // Subset, not Equal: on hosts where a fixed path crosses a symlink | |
| 65 | + | // (for example /var -> /private/var) its resolved form is added too. | |
| 63 | 66 | got := protectedRuntimePaths("/data", nil, fakeEnv(map[string]string{"DOCKER_HOST": "tcp://h:2375"}), "") | |
| 64 | - | assert.Equal(t, []string{"/data", "/run/docker.sock", "/run/podman/podman.sock", "/var/run/docker.sock"}, got) | |
| 67 | + | assert.Subset(t, got, []string{"/data", "/run/docker.sock", "/run/podman/podman.sock", "/var/run/docker.sock"}) | |
| 68 | + | for _, p := range got { | |
| 69 | + | assert.NotContains(t, p, "tcp:") | |
| 70 | + | assert.NotContains(t, p, "h:2375") | |
| 71 | + | } | |
| 65 | 72 | } | |
| 66 | 73 | ||
| 67 | 74 | func TestProtectedRuntimePaths_SymlinkAddsBothForms(t *testing.T) { | |
| @@ -111,6 +118,7 @@ func TestBuildAppRuntimeSafety(t *testing.T) { | |||
| 111 | 118 | store, guard, err := buildAppRuntimeSafety(&serviceInit{}, dataDir) | |
| 112 | 119 | require.NoError(t, err) | |
| 113 | 120 | require.NotNil(t, store) | |
| 121 | + | t.Cleanup(func() { _ = store.(*secretfiles.Store).Close() }) | |
| 114 | 122 | assert.Equal(t, filepath.Join(dataDir, "secrets-run"), guard.SecretFilesDir) | |
| 115 | 123 | assert.Contains(t, guard.ProtectedPaths, dataDir) | |
| 116 | 124 | ||
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "github.com/stretchr/testify/assert" | 8 | "github.com/stretchr/testify/assert" | ||
| 9 | "github.com/stretchr/testify/require" | 9 | "github.com/stretchr/testify/require" | ||
| 10 | 10 | ||||
| 11 | + | "github.com/bnema/gordon/internal/adapters/out/secretfiles" | |||
| 11 | "github.com/bnema/gordon/internal/domain" | 12 | "github.com/bnema/gordon/internal/domain" | ||
| 12 | ) | 13 | ) | ||
| 13 | 14 | ||||
| @@ -60,8 +61,14 @@ func TestProtectedRuntimePaths_AllSources(t *testing.T) { | |||||
| 60 | } | 61 | } | ||
| 61 | 62 | ||||
| 62 | func TestProtectedRuntimePaths_SkipsEmptyAndTCP(t *testing.T) { | 63 | func TestProtectedRuntimePaths_SkipsEmptyAndTCP(t *testing.T) { | ||
| 64 | + | // Subset, not Equal: on hosts where a fixed path crosses a symlink | |||
| 65 | + | // (for example /var -> /private/var) its resolved form is added too. | |||
| 63 | got := protectedRuntimePaths("/data", nil, fakeEnv(map[string]string{"DOCKER_HOST": "tcp://h:2375"}), "") | 66 | got := protectedRuntimePaths("/data", nil, fakeEnv(map[string]string{"DOCKER_HOST": "tcp://h:2375"}), "") | ||
| 64 | - | assert.Equal(t, []string{"/data", "/run/docker.sock", "/run/podman/podman.sock", "/var/run/docker.sock"}, got) | 67 | + | assert.Subset(t, got, []string{"/data", "/run/docker.sock", "/run/podman/podman.sock", "/var/run/docker.sock"}) |
| 68 | + | for _, p := range got { | |||
| 69 | + | assert.NotContains(t, p, "tcp:") | |||
| 70 | + | assert.NotContains(t, p, "h:2375") | |||
| 71 | + | } | |||
| 65 | } | 72 | } | ||
| 66 | 73 | ||||
| 67 | func TestProtectedRuntimePaths_SymlinkAddsBothForms(t *testing.T) { | 74 | func TestProtectedRuntimePaths_SymlinkAddsBothForms(t *testing.T) { | ||
| @@ -111,6 +118,7 @@ func TestBuildAppRuntimeSafety(t *testing.T) { | |||||
| 111 | store, guard, err := buildAppRuntimeSafety(&serviceInit{}, dataDir) | 118 | store, guard, err := buildAppRuntimeSafety(&serviceInit{}, dataDir) | ||
| 112 | require.NoError(t, err) | 119 | require.NoError(t, err) | ||
| 113 | require.NotNil(t, store) | 120 | require.NotNil(t, store) | ||
| 121 | + | t.Cleanup(func() { _ = store.(*secretfiles.Store).Close() }) | |||
| 114 | assert.Equal(t, filepath.Join(dataDir, "secrets-run"), guard.SecretFilesDir) | 122 | assert.Equal(t, filepath.Join(dataDir, "secrets-run"), guard.SecretFilesDir) | ||
| 115 | assert.Contains(t, guard.ProtectedPaths, dataDir) | 123 | assert.Contains(t, guard.ProtectedPaths, dataDir) | ||
| 116 | 124 | ||||
M internal/domain/app_secret_files_test.go +0 -2
| @@ -19,8 +19,6 @@ func TestAppSpec_ValidateSecretFilesOK(t *testing.T) { | |||
| 19 | 19 | require.NoError(t, fileSecretSpec().Validate()) | |
| 20 | 20 | assert.True(t, fileSecretSpec().Services[0].SecretAsFile("DATABASE_URL")) | |
| 21 | 21 | assert.False(t, validSpec().Services[0].SecretAsFile("DATABASE_URL")) | |
| 22 | - | assert.Equal(t, "DATABASE_URL_FILE", domain.SecretFileEnvKey("DATABASE_URL")) | |
| 23 | - | assert.Equal(t, "/run/secrets/DATABASE_URL", domain.SecretFilePath("DATABASE_URL")) | |
| 24 | 22 | } | |
| 25 | 23 | ||
| 26 | 24 | func TestAppSpec_ValidateSecretFilesErrors(t *testing.T) { | |
| @@ -19,8 +19,6 @@ func TestAppSpec_ValidateSecretFilesOK(t *testing.T) { | |||||
| 19 | require.NoError(t, fileSecretSpec().Validate()) | 19 | require.NoError(t, fileSecretSpec().Validate()) | ||
| 20 | assert.True(t, fileSecretSpec().Services[0].SecretAsFile("DATABASE_URL")) | 20 | assert.True(t, fileSecretSpec().Services[0].SecretAsFile("DATABASE_URL")) | ||
| 21 | assert.False(t, validSpec().Services[0].SecretAsFile("DATABASE_URL")) | 21 | assert.False(t, validSpec().Services[0].SecretAsFile("DATABASE_URL")) | ||
| 22 | - | assert.Equal(t, "DATABASE_URL_FILE", domain.SecretFileEnvKey("DATABASE_URL")) | |||
| 23 | - | assert.Equal(t, "/run/secrets/DATABASE_URL", domain.SecretFilePath("DATABASE_URL")) | |||
| 24 | } | 22 | } | ||
| 25 | 23 | ||||
| 26 | func TestAppSpec_ValidateSecretFilesErrors(t *testing.T) { | 24 | func TestAppSpec_ValidateSecretFilesErrors(t *testing.T) { | ||
M internal/domain/app_spec.go +2 -9
| @@ -2,6 +2,7 @@ package domain | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "fmt" | |
| 5 | + | "maps" | |
| 5 | 6 | "net" | |
| 6 | 7 | "path" | |
| 7 | 8 | "reflect" | |
| @@ -1128,15 +1129,7 @@ func appendOperationChanges(changed []string, name string, desired, effective Ap | |||
| 1128 | 1129 | // appendSecretChanges appends sorted secret path changes when the secret | |
| 1129 | 1130 | // names or their delivery modes differ. | |
| 1130 | 1131 | func appendSecretChanges(changed []string, desired, effective AppService) []string { | |
| 1131 | - | if equalStringMaps(desired.Secrets, effective.Secrets) && slices.Equal(desired.SecretFiles, effective.SecretFiles) { | |
| 1132 | - | return changed | |
| 1133 | - | } | |
| 1134 | - | paths := make([]string, 0) | |
| 1135 | - | for path := range secretPathChanges(desired, effective) { | |
| 1136 | - | paths = append(paths, path) | |
| 1137 | - | } | |
| 1138 | - | sort.Strings(paths) | |
| 1139 | - | return append(changed, paths...) | |
| 1132 | + | return append(changed, slices.Sorted(maps.Keys(secretPathChanges(desired, effective)))...) | |
| 1140 | 1133 | } | |
| 1141 | 1134 | ||
| 1142 | 1135 | // secretPathChanges lists secret PATH changes (never values). A key whose | |
| @@ -2,6 +2,7 @@ package domain | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "fmt" | 4 | "fmt" | ||
| 5 | + | "maps" | |||
| 5 | "net" | 6 | "net" | ||
| 6 | "path" | 7 | "path" | ||
| 7 | "reflect" | 8 | "reflect" | ||
| @@ -1128,15 +1129,7 @@ func appendOperationChanges(changed []string, name string, desired, effective Ap | |||||
| 1128 | // appendSecretChanges appends sorted secret path changes when the secret | 1129 | // appendSecretChanges appends sorted secret path changes when the secret | ||
| 1129 | // names or their delivery modes differ. | 1130 | // names or their delivery modes differ. | ||
| 1130 | func appendSecretChanges(changed []string, desired, effective AppService) []string { | 1131 | func appendSecretChanges(changed []string, desired, effective AppService) []string { | ||
| 1131 | - | if equalStringMaps(desired.Secrets, effective.Secrets) && slices.Equal(desired.SecretFiles, effective.SecretFiles) { | 1132 | + | return append(changed, slices.Sorted(maps.Keys(secretPathChanges(desired, effective)))...) |
| 1132 | - | return changed | |||
| 1133 | - | } | |||
| 1134 | - | paths := make([]string, 0) | |||
| 1135 | - | for path := range secretPathChanges(desired, effective) { | |||
| 1136 | - | paths = append(paths, path) | |||
| 1137 | - | } | |||
| 1138 | - | sort.Strings(paths) | |||
| 1139 | - | return append(changed, paths...) | |||
| 1140 | } | 1133 | } | ||
| 1141 | 1134 | ||||
| 1142 | // secretPathChanges lists secret PATH changes (never values). A key whose | 1135 | // secretPathChanges lists secret PATH changes (never values). A key whose | ||
M internal/usecase/deployment/execute.go +1 -1
| @@ -591,7 +591,7 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||
| 591 | 591 | // Revalidate the current authorization and source before withdrawing or | |
| 592 | 592 | // retiring the serving generation. A reload between preflight and this | |
| 593 | 593 | // service step must fail without mutating the existing workload. | |
| 594 | - | if _, err := s.resolveGuardedBinds(app, p.spec); err != nil { | |
| 594 | + | if _, err := s.resolveServiceBinds(app, p.spec); err != nil { | |
| 595 | 595 | return s.failResult(revision, before, "", err) | |
| 596 | 596 | } | |
| 597 | 597 | if _, err := s.resolveServiceDevices(app, p.spec); err != nil { | |
| @@ -591,7 +591,7 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||||
| 591 | // Revalidate the current authorization and source before withdrawing or | 591 | // Revalidate the current authorization and source before withdrawing or | ||
| 592 | // retiring the serving generation. A reload between preflight and this | 592 | // retiring the serving generation. A reload between preflight and this | ||
| 593 | // service step must fail without mutating the existing workload. | 593 | // service step must fail without mutating the existing workload. | ||
| 594 | - | if _, err := s.resolveGuardedBinds(app, p.spec); err != nil { | 594 | + | if _, err := s.resolveServiceBinds(app, p.spec); err != nil { |
| 595 | return s.failResult(revision, before, "", err) | 595 | return s.failResult(revision, before, "", err) | ||
| 596 | } | 596 | } | ||
| 597 | if _, err := s.resolveServiceDevices(app, p.spec); err != nil { | 597 | if _, err := s.resolveServiceDevices(app, p.spec); err != nil { | ||
M internal/usecase/deployment/lifecycle.go +3 -3
| @@ -301,7 +301,7 @@ func (s *Service) restartOneService(ctx context.Context, app, opID, name string, | |||
| 301 | 301 | } | |
| 302 | 302 | // Fail closed before any runtime mutation if a bind's policy was | |
| 303 | 303 | // revoked or became invalid since ACTIVE was published. | |
| 304 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | |
| 304 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { | |
| 305 | 305 | return fail(err.Error()) | |
| 306 | 306 | } | |
| 307 | 307 | // A device grant revoked since ACTIVE was published must fail | |
| @@ -608,7 +608,7 @@ func (s *Service) ensureServiceRunning(ctx context.Context, app, opID, name stri | |||
| 608 | 608 | if eff.Container != "" { | |
| 609 | 609 | // Fail closed before any runtime mutation if a bind's policy was | |
| 610 | 610 | // revoked or became invalid since ACTIVE was published. | |
| 611 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | |
| 611 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { | |
| 612 | 612 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | |
| 613 | 613 | return | |
| 614 | 614 | } | |
| @@ -668,7 +668,7 @@ func (s *Service) redeployPinned(ctx context.Context, app, opID, name string, ef | |||
| 668 | 668 | if err != nil { | |
| 669 | 669 | return failedServiceResult(eff.Container, err), err | |
| 670 | 670 | } | |
| 671 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | |
| 671 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { | |
| 672 | 672 | return failedServiceResult(eff.Container, err), err | |
| 673 | 673 | } | |
| 674 | 674 | // Revoked device grants fail before any runtime mutation. | |
| @@ -301,7 +301,7 @@ func (s *Service) restartOneService(ctx context.Context, app, opID, name string, | |||||
| 301 | } | 301 | } | ||
| 302 | // Fail closed before any runtime mutation if a bind's policy was | 302 | // Fail closed before any runtime mutation if a bind's policy was | ||
| 303 | // revoked or became invalid since ACTIVE was published. | 303 | // revoked or became invalid since ACTIVE was published. | ||
| 304 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | 304 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { |
| 305 | return fail(err.Error()) | 305 | return fail(err.Error()) | ||
| 306 | } | 306 | } | ||
| 307 | // A device grant revoked since ACTIVE was published must fail | 307 | // A device grant revoked since ACTIVE was published must fail | ||
| @@ -608,7 +608,7 @@ func (s *Service) ensureServiceRunning(ctx context.Context, app, opID, name stri | |||||
| 608 | if eff.Container != "" { | 608 | if eff.Container != "" { | ||
| 609 | // Fail closed before any runtime mutation if a bind's policy was | 609 | // Fail closed before any runtime mutation if a bind's policy was | ||
| 610 | // revoked or became invalid since ACTIVE was published. | 610 | // revoked or became invalid since ACTIVE was published. | ||
| 611 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | 611 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { |
| 612 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | 612 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | ||
| 613 | return | 613 | return | ||
| 614 | } | 614 | } | ||
| @@ -668,7 +668,7 @@ func (s *Service) redeployPinned(ctx context.Context, app, opID, name string, ef | |||||
| 668 | if err != nil { | 668 | if err != nil { | ||
| 669 | return failedServiceResult(eff.Container, err), err | 669 | return failedServiceResult(eff.Container, err), err | ||
| 670 | } | 670 | } | ||
| 671 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | 671 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { |
| 672 | return failedServiceResult(eff.Container, err), err | 672 | return failedServiceResult(eff.Container, err), err | ||
| 673 | } | 673 | } | ||
| 674 | // Revoked device grants fail before any runtime mutation. | 674 | // Revoked device grants fail before any runtime mutation. | ||
M internal/usecase/deployment/monitor.go +2 -2
| @@ -635,7 +635,7 @@ func (s *Service) publishRecoveredService(ctx context.Context, app, name string, | |||
| 635 | 635 | func (s *Service) startRecovered(serviceCtx context.Context, app, name string, eff domain.AppEffectiveService, key recoveryKey) (bool, error) { | |
| 636 | 636 | // Fail closed before mutating the runtime if a bind's policy was | |
| 637 | 637 | // revoked or became invalid since ACTIVE was published. | |
| 638 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | |
| 638 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { | |
| 639 | 639 | return false, err | |
| 640 | 640 | } | |
| 641 | 641 | // Revoked device grants fail before any runtime mutation. | |
| @@ -691,7 +691,7 @@ func (s *Service) recoverHealth(serviceCtx context.Context, app, name string, ef | |||
| 691 | 691 | } | |
| 692 | 692 | // Fail closed before mutating the runtime if a bind's policy was | |
| 693 | 693 | // revoked or became invalid since ACTIVE was published. | |
| 694 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | |
| 694 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { | |
| 695 | 695 | return false, false, err | |
| 696 | 696 | } | |
| 697 | 697 | // Revoked device grants fail before any runtime mutation. | |
| @@ -635,7 +635,7 @@ func (s *Service) publishRecoveredService(ctx context.Context, app, name string, | |||||
| 635 | func (s *Service) startRecovered(serviceCtx context.Context, app, name string, eff domain.AppEffectiveService, key recoveryKey) (bool, error) { | 635 | func (s *Service) startRecovered(serviceCtx context.Context, app, name string, eff domain.AppEffectiveService, key recoveryKey) (bool, error) { | ||
| 636 | // Fail closed before mutating the runtime if a bind's policy was | 636 | // Fail closed before mutating the runtime if a bind's policy was | ||
| 637 | // revoked or became invalid since ACTIVE was published. | 637 | // revoked or became invalid since ACTIVE was published. | ||
| 638 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | 638 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { |
| 639 | return false, err | 639 | return false, err | ||
| 640 | } | 640 | } | ||
| 641 | // Revoked device grants fail before any runtime mutation. | 641 | // Revoked device grants fail before any runtime mutation. | ||
| @@ -691,7 +691,7 @@ func (s *Service) recoverHealth(serviceCtx context.Context, app, name string, ef | |||||
| 691 | } | 691 | } | ||
| 692 | // Fail closed before mutating the runtime if a bind's policy was | 692 | // Fail closed before mutating the runtime if a bind's policy was | ||
| 693 | // revoked or became invalid since ACTIVE was published. | 693 | // revoked or became invalid since ACTIVE was published. | ||
| 694 | - | if _, err := s.resolveGuardedBinds(app, eff.Spec); err != nil { | 694 | + | if _, err := s.resolveServiceBinds(app, eff.Spec); err != nil { |
| 695 | return false, false, err | 695 | return false, false, err | ||
| 696 | } | 696 | } | ||
| 697 | // Revoked device grants fail before any runtime mutation. | 697 | // Revoked device grants fail before any runtime mutation. | ||
M internal/usecase/deployment/retirement.go +3 -1
| @@ -94,7 +94,9 @@ func (s *Service) retireContainer(ctx context.Context, app string, opts retireOp | |||
| 94 | 94 | Service: opts.Service, Leftover: containerID, Detail: "release backend claims: " + err.Error(), | |
| 95 | 95 | }) | |
| 96 | 96 | } | |
| 97 | - | if err := s.sweepAppSecretFiles(ctx, app); err != nil { | |
| 97 | + | // The app lock is held, so no candidate of this app can be between | |
| 98 | + | // secret file write and container create. | |
| 99 | + | if err := s.sweepSecretFiles(ctx, app); err != nil { | |
| 98 | 100 | // Fixed detail: the error may name host paths, which must stay | |
| 99 | 101 | // out of journals and API responses. | |
| 100 | 102 | log := zerowrap.FromCtx(ctx) | |
| @@ -94,7 +94,9 @@ func (s *Service) retireContainer(ctx context.Context, app string, opts retireOp | |||||
| 94 | Service: opts.Service, Leftover: containerID, Detail: "release backend claims: " + err.Error(), | 94 | Service: opts.Service, Leftover: containerID, Detail: "release backend claims: " + err.Error(), | ||
| 95 | }) | 95 | }) | ||
| 96 | } | 96 | } | ||
| 97 | - | if err := s.sweepAppSecretFiles(ctx, app); err != nil { | 97 | + | // The app lock is held, so no candidate of this app can be between |
| 98 | + | // secret file write and container create. | |||
| 99 | + | if err := s.sweepSecretFiles(ctx, app); err != nil { | |||
| 98 | // Fixed detail: the error may name host paths, which must stay | 100 | // Fixed detail: the error may name host paths, which must stay | ||
| 99 | // out of journals and API responses. | 101 | // out of journals and API responses. | ||
| 100 | log := zerowrap.FromCtx(ctx) | 102 | log := zerowrap.FromCtx(ctx) | ||
M internal/usecase/deployment/runtime_guard_internal_test.go +19 -20
| @@ -15,29 +15,28 @@ import ( | |||
| 15 | 15 | "github.com/bnema/gordon/internal/domain" | |
| 16 | 16 | ) | |
| 17 | 17 | ||
| 18 | - | func TestCreateContainer_GuardRefusesHostNetwork(t *testing.T) { | |
| 19 | - | runtime := outmocks.NewMockContainerRuntime(t) | |
| 20 | - | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()) | |
| 21 | - | ||
| 22 | - | _, err := svc.createContainer(context.Background(), "web", &domain.ContainerConfig{Name: "c", NetworkMode: "host"}) | |
| 23 | - | require.ErrorIs(t, err, domain.ErrRuntimeGuard) | |
| 24 | - | runtime.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |
| 25 | - | } | |
| 26 | - | ||
| 27 | - | func TestCreateContainer_GuardRefusesProtectedBind(t *testing.T) { | |
| 18 | + | func TestCreateContainer_GuardRefuses(t *testing.T) { | |
| 28 | 19 | protected := t.TempDir() | |
| 29 | 20 | child := filepath.Join(protected, "registry") | |
| 30 | 21 | require.NoError(t, os.MkdirAll(child, 0o700)) | |
| 31 | - | runtime := outmocks.NewMockContainerRuntime(t) | |
| 32 | - | svc := NewService(Deps{Runtime: runtime, Guard: domain.RuntimeGuard{ProtectedPaths: []string{protected}}}, zerowrap.Default()) | |
| 22 | + | cases := map[string]*domain.ContainerConfig{ | |
| 23 | + | "host network": {Name: "c", NetworkMode: "host"}, | |
| 24 | + | "protected bind": { | |
| 25 | + | Name: "c", | |
| 26 | + | Binds: []domain.ContainerBind{{Name: "conf", Source: child, Destination: "/etc/app", ReadOnly: true}}, | |
| 27 | + | }, | |
| 28 | + | } | |
| 29 | + | for name, config := range cases { | |
| 30 | + | t.Run(name, func(t *testing.T) { | |
| 31 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 32 | + | svc := NewService(Deps{Runtime: runtime, Guard: domain.RuntimeGuard{ProtectedPaths: []string{protected}}}, zerowrap.Default()) | |
| 33 | 33 | ||
| 34 | - | _, err := svc.createContainer(context.Background(), "web", &domain.ContainerConfig{ | |
| 35 | - | Name: "c", | |
| 36 | - | Binds: []domain.ContainerBind{{Name: "conf", Source: child, Destination: "/etc/app", ReadOnly: true}}, | |
| 37 | - | }) | |
| 38 | - | require.ErrorIs(t, err, domain.ErrRuntimeGuard) | |
| 39 | - | assert.NotContains(t, err.Error(), protected) | |
| 40 | - | runtime.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |
| 34 | + | _, err := svc.createContainer(context.Background(), "web", config) | |
| 35 | + | require.ErrorIs(t, err, domain.ErrRuntimeGuard) | |
| 36 | + | assert.NotContains(t, err.Error(), protected) | |
| 37 | + | runtime.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |
| 38 | + | }) | |
| 39 | + | } | |
| 41 | 40 | } | |
| 42 | 41 | ||
| 43 | 42 | func TestCreateWithSecretFiles_GuardAllowsOwnSecretBind(t *testing.T) { | |
| @@ -64,7 +63,7 @@ func TestResolveGuardedBinds_RefusesProtectedSource(t *testing.T) { | |||
| 64 | 63 | "config": bindPolicyFor(protected, protected, true, []string{"blog"}, []string{"web"}), | |
| 65 | 64 | }) | |
| 66 | 65 | ||
| 67 | - | _, err := svc.resolveGuardedBinds("blog", domain.AppService{ | |
| 66 | + | _, err := svc.resolveServiceBinds("blog", domain.AppService{ | |
| 68 | 67 | Name: "web", | |
| 69 | 68 | Binds: []domain.AppBind{{Name: "config", Path: "/etc/app", ReadOnly: true}}, | |
| 70 | 69 | }) | |
| @@ -15,29 +15,28 @@ import ( | |||||
| 15 | "github.com/bnema/gordon/internal/domain" | 15 | "github.com/bnema/gordon/internal/domain" | ||
| 16 | ) | 16 | ) | ||
| 17 | 17 | ||||
| 18 | - | func TestCreateContainer_GuardRefusesHostNetwork(t *testing.T) { | 18 | + | func TestCreateContainer_GuardRefuses(t *testing.T) { |
| 19 | - | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 20 | - | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()) | |||
| 21 | - | ||||
| 22 | - | _, err := svc.createContainer(context.Background(), "web", &domain.ContainerConfig{Name: "c", NetworkMode: "host"}) | |||
| 23 | - | require.ErrorIs(t, err, domain.ErrRuntimeGuard) | |||
| 24 | - | runtime.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |||
| 25 | - | } | |||
| 26 | - | ||||
| 27 | - | func TestCreateContainer_GuardRefusesProtectedBind(t *testing.T) { | |||
| 28 | protected := t.TempDir() | 19 | protected := t.TempDir() | ||
| 29 | child := filepath.Join(protected, "registry") | 20 | child := filepath.Join(protected, "registry") | ||
| 30 | require.NoError(t, os.MkdirAll(child, 0o700)) | 21 | require.NoError(t, os.MkdirAll(child, 0o700)) | ||
| 31 | - | runtime := outmocks.NewMockContainerRuntime(t) | 22 | + | cases := map[string]*domain.ContainerConfig{ |
| 32 | - | svc := NewService(Deps{Runtime: runtime, Guard: domain.RuntimeGuard{ProtectedPaths: []string{protected}}}, zerowrap.Default()) | 23 | + | "host network": {Name: "c", NetworkMode: "host"}, |
| 24 | + | "protected bind": { | |||
| 25 | + | Name: "c", | |||
| 26 | + | Binds: []domain.ContainerBind{{Name: "conf", Source: child, Destination: "/etc/app", ReadOnly: true}}, | |||
| 27 | + | }, | |||
| 28 | + | } | |||
| 29 | + | for name, config := range cases { | |||
| 30 | + | t.Run(name, func(t *testing.T) { | |||
| 31 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 32 | + | svc := NewService(Deps{Runtime: runtime, Guard: domain.RuntimeGuard{ProtectedPaths: []string{protected}}}, zerowrap.Default()) | |||
| 33 | 33 | ||||
| 34 | - | _, err := svc.createContainer(context.Background(), "web", &domain.ContainerConfig{ | 34 | + | _, err := svc.createContainer(context.Background(), "web", config) |
| 35 | - | Name: "c", | 35 | + | require.ErrorIs(t, err, domain.ErrRuntimeGuard) |
| 36 | - | Binds: []domain.ContainerBind{{Name: "conf", Source: child, Destination: "/etc/app", ReadOnly: true}}, | 36 | + | assert.NotContains(t, err.Error(), protected) |
| 37 | - | }) | 37 | + | runtime.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) |
| 38 | - | require.ErrorIs(t, err, domain.ErrRuntimeGuard) | 38 | + | }) |
| 39 | - | assert.NotContains(t, err.Error(), protected) | 39 | + | } |
| 40 | - | runtime.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |||
| 41 | } | 40 | } | ||
| 42 | 41 | ||||
| 43 | func TestCreateWithSecretFiles_GuardAllowsOwnSecretBind(t *testing.T) { | 42 | func TestCreateWithSecretFiles_GuardAllowsOwnSecretBind(t *testing.T) { | ||
| @@ -64,7 +63,7 @@ func TestResolveGuardedBinds_RefusesProtectedSource(t *testing.T) { | |||||
| 64 | "config": bindPolicyFor(protected, protected, true, []string{"blog"}, []string{"web"}), | 63 | "config": bindPolicyFor(protected, protected, true, []string{"blog"}, []string{"web"}), | ||
| 65 | }) | 64 | }) | ||
| 66 | 65 | ||||
| 67 | - | _, err := svc.resolveGuardedBinds("blog", domain.AppService{ | 66 | + | _, err := svc.resolveServiceBinds("blog", domain.AppService{ |
| 68 | Name: "web", | 67 | Name: "web", | ||
| 69 | Binds: []domain.AppBind{{Name: "config", Path: "/etc/app", ReadOnly: true}}, | 68 | Binds: []domain.AppBind{{Name: "config", Path: "/etc/app", ReadOnly: true}}, | ||
| 70 | }) | 69 | }) | ||
M internal/usecase/deployment/secret_files.go +0 -7
| @@ -8,13 +8,6 @@ import ( | |||
| 8 | 8 | "github.com/bnema/gordon/internal/boundaries/out" | |
| 9 | 9 | ) | |
| 10 | 10 | ||
| 11 | - | // sweepAppSecretFiles removes the secret file directories of app whose | |
| 12 | - | // container no longer exists in the runtime. Callers hold the app lock, | |
| 13 | - | // so no candidate of this app can be between file write and create. | |
| 14 | - | func (s *Service) sweepAppSecretFiles(ctx context.Context, app string) error { | |
| 15 | - | return s.sweepSecretFiles(ctx, app) | |
| 16 | - | } | |
| 17 | - | ||
| 18 | 11 | // SweepSecretFiles removes every secret file directory whose container no | |
| 19 | 12 | // longer exists, across all apps. It takes no locks: it runs once at | |
| 20 | 13 | // startup, before any listener (and so any deploy) exists. | |
| @@ -8,13 +8,6 @@ import ( | |||||
| 8 | "github.com/bnema/gordon/internal/boundaries/out" | 8 | "github.com/bnema/gordon/internal/boundaries/out" | ||
| 9 | ) | 9 | ) | ||
| 10 | 10 | ||||
| 11 | - | // sweepAppSecretFiles removes the secret file directories of app whose | |||
| 12 | - | // container no longer exists in the runtime. Callers hold the app lock, | |||
| 13 | - | // so no candidate of this app can be between file write and create. | |||
| 14 | - | func (s *Service) sweepAppSecretFiles(ctx context.Context, app string) error { | |||
| 15 | - | return s.sweepSecretFiles(ctx, app) | |||
| 16 | - | } | |||
| 17 | - | ||||
| 18 | // SweepSecretFiles removes every secret file directory whose container no | 11 | // SweepSecretFiles removes every secret file directory whose container no | ||
| 19 | // longer exists, across all apps. It takes no locks: it runs once at | 12 | // longer exists, across all apps. It takes no locks: it runs once at | ||
| 20 | // startup, before any listener (and so any deploy) exists. | 13 | // startup, before any listener (and so any deploy) exists. | ||
M internal/usecase/deployment/secret_files_internal_test.go +5 -2
| @@ -127,7 +127,10 @@ func TestCreateWithSecretFiles_CleanupFailureKeepsCreateError(t *testing.T) { | |||
| 127 | 127 | ||
| 128 | 128 | _, err := svc.createWithSecretFiles(context.Background(), "blog", "web", &domain.ContainerConfig{Name: "c"}, map[string]string{"B": "v"}) | |
| 129 | 129 | require.Error(t, err) | |
| 130 | + | // The create error is returned; the cleanup error is only logged. | |
| 131 | + | assert.Contains(t, err.Error(), `"web"`) | |
| 130 | 132 | assert.Contains(t, err.Error(), "runtime error redacted") | |
| 133 | + | assert.NotContains(t, err.Error(), "busy") | |
| 131 | 134 | } | |
| 132 | 135 | ||
| 133 | 136 | func TestCreateWithSecretFiles_NilStoreRefuses(t *testing.T) { | |
| @@ -208,7 +211,7 @@ func TestSweepSecretFiles(t *testing.T) { | |||
| 208 | 211 | store.EXPECT().Remove(mock.Anything, "blog", "gordon-blog--web--a").Return(nil).Once() | |
| 209 | 212 | ||
| 210 | 213 | svc := NewService(Deps{Runtime: runtime, SecretFiles: store}, zerowrap.Default()) | |
| 211 | - | require.NoError(t, svc.sweepAppSecretFiles(context.Background(), "blog")) | |
| 214 | + | require.NoError(t, svc.sweepSecretFiles(context.Background(), "blog")) | |
| 212 | 215 | }) | |
| 213 | 216 | ||
| 214 | 217 | t.Run("all apps", func(t *testing.T) { | |
| @@ -229,7 +232,7 @@ func TestSweepSecretFiles(t *testing.T) { | |||
| 229 | 232 | store.EXPECT().List(mock.Anything).Return(sets, nil).Once() | |
| 230 | 233 | ||
| 231 | 234 | svc := NewService(Deps{Runtime: runtime, SecretFiles: store}, zerowrap.Default()) | |
| 232 | - | require.NoError(t, svc.sweepAppSecretFiles(context.Background(), "ghost")) | |
| 235 | + | require.NoError(t, svc.sweepSecretFiles(context.Background(), "ghost")) | |
| 233 | 236 | runtime.AssertNotCalled(t, "ListContainers", mock.Anything, mock.Anything) | |
| 234 | 237 | }) | |
| 235 | 238 | ||
| @@ -127,7 +127,10 @@ func TestCreateWithSecretFiles_CleanupFailureKeepsCreateError(t *testing.T) { | |||||
| 127 | 127 | ||||
| 128 | _, err := svc.createWithSecretFiles(context.Background(), "blog", "web", &domain.ContainerConfig{Name: "c"}, map[string]string{"B": "v"}) | 128 | _, err := svc.createWithSecretFiles(context.Background(), "blog", "web", &domain.ContainerConfig{Name: "c"}, map[string]string{"B": "v"}) | ||
| 129 | require.Error(t, err) | 129 | require.Error(t, err) | ||
| 130 | + | // The create error is returned; the cleanup error is only logged. | |||
| 131 | + | assert.Contains(t, err.Error(), `"web"`) | |||
| 130 | assert.Contains(t, err.Error(), "runtime error redacted") | 132 | assert.Contains(t, err.Error(), "runtime error redacted") | ||
| 133 | + | assert.NotContains(t, err.Error(), "busy") | |||
| 131 | } | 134 | } | ||
| 132 | 135 | ||||
| 133 | func TestCreateWithSecretFiles_NilStoreRefuses(t *testing.T) { | 136 | func TestCreateWithSecretFiles_NilStoreRefuses(t *testing.T) { | ||
| @@ -208,7 +211,7 @@ func TestSweepSecretFiles(t *testing.T) { | |||||
| 208 | store.EXPECT().Remove(mock.Anything, "blog", "gordon-blog--web--a").Return(nil).Once() | 211 | store.EXPECT().Remove(mock.Anything, "blog", "gordon-blog--web--a").Return(nil).Once() | ||
| 209 | 212 | ||||
| 210 | svc := NewService(Deps{Runtime: runtime, SecretFiles: store}, zerowrap.Default()) | 213 | svc := NewService(Deps{Runtime: runtime, SecretFiles: store}, zerowrap.Default()) | ||
| 211 | - | require.NoError(t, svc.sweepAppSecretFiles(context.Background(), "blog")) | 214 | + | require.NoError(t, svc.sweepSecretFiles(context.Background(), "blog")) |
| 212 | }) | 215 | }) | ||
| 213 | 216 | ||||
| 214 | t.Run("all apps", func(t *testing.T) { | 217 | t.Run("all apps", func(t *testing.T) { | ||
| @@ -229,7 +232,7 @@ func TestSweepSecretFiles(t *testing.T) { | |||||
| 229 | store.EXPECT().List(mock.Anything).Return(sets, nil).Once() | 232 | store.EXPECT().List(mock.Anything).Return(sets, nil).Once() | ||
| 230 | 233 | ||||
| 231 | svc := NewService(Deps{Runtime: runtime, SecretFiles: store}, zerowrap.Default()) | 234 | svc := NewService(Deps{Runtime: runtime, SecretFiles: store}, zerowrap.Default()) | ||
| 232 | - | require.NoError(t, svc.sweepAppSecretFiles(context.Background(), "ghost")) | 235 | + | require.NoError(t, svc.sweepSecretFiles(context.Background(), "ghost")) |
| 233 | runtime.AssertNotCalled(t, "ListContainers", mock.Anything, mock.Anything) | 236 | runtime.AssertNotCalled(t, "ListContainers", mock.Anything, mock.Anything) | ||
| 234 | }) | 237 | }) | ||
| 235 | 238 | ||||
M internal/usecase/deployment/service.go +7 -15
| @@ -279,22 +279,14 @@ func (s *Service) resolveServiceBinds(app string, svc domain.AppService) ([]doma | |||
| 279 | 279 | } | |
| 280 | 280 | resolved = append(resolved, domain.ContainerBind(r)) | |
| 281 | 281 | } | |
| 282 | - | return resolved, nil | |
| 283 | - | } | |
| 284 | - | ||
| 285 | - | // resolveGuardedBinds resolves the service binds and applies the runtime | |
| 286 | - | // guard to them, so a bind overlapping a protected Gordon path fails at | |
| 287 | - | // preflight and before the serving generation is retired, not only at | |
| 288 | - | // create time. createContainer keeps the full guard as the final check. | |
| 289 | - | func (s *Service) resolveGuardedBinds(app string, svc domain.AppService) ([]domain.ContainerBind, error) { | |
| 290 | - | binds, err := s.resolveServiceBinds(app, svc) | |
| 291 | - | if err != nil { | |
| 292 | - | return nil, err | |
| 293 | - | } | |
| 294 | - | if err := s.deps.Guard.Check(&domain.ContainerConfig{Binds: binds}); err != nil { | |
| 282 | + | // Apply the runtime guard here so a bind overlapping a protected | |
| 283 | + | // Gordon path fails at preflight and before the serving generation is | |
| 284 | + | // retired, not only at create time. createContainer keeps the full | |
| 285 | + | // guard as the final check. | |
| 286 | + | if err := s.deps.Guard.Check(&domain.ContainerConfig{Binds: resolved}); err != nil { | |
| 295 | 287 | return nil, fmt.Errorf("deployment: app %q service %q: %w", app, svc.Name, err) | |
| 296 | 288 | } | |
| 297 | - | return binds, nil | |
| 289 | + | return resolved, nil | |
| 298 | 290 | } | |
| 299 | 291 | ||
| 300 | 292 | // acquireAppContext takes the GC shared lease first and the per-app | |
| @@ -738,7 +730,7 @@ func (s *Service) preflightServices(ctx context.Context, app string, rev domain. | |||
| 738 | 730 | // must fail before any workload mutation. The successful values | |
| 739 | 731 | // are discarded because createAndStart re-resolves against the | |
| 740 | 732 | // current policy immediately before the runtime mutation. | |
| 741 | - | if _, err := s.resolveGuardedBinds(app, svc); err != nil { | |
| 733 | + | if _, err := s.resolveServiceBinds(app, svc); err != nil { | |
| 742 | 734 | return nil, err | |
| 743 | 735 | } | |
| 744 | 736 | if err := s.preflightServiceDevices(ctx, app, svc, pinned); err != nil { | |
| @@ -279,22 +279,14 @@ func (s *Service) resolveServiceBinds(app string, svc domain.AppService) ([]doma | |||||
| 279 | } | 279 | } | ||
| 280 | resolved = append(resolved, domain.ContainerBind(r)) | 280 | resolved = append(resolved, domain.ContainerBind(r)) | ||
| 281 | } | 281 | } | ||
| 282 | - | return resolved, nil | 282 | + | // Apply the runtime guard here so a bind overlapping a protected |
| 283 | - | } | 283 | + | // Gordon path fails at preflight and before the serving generation is |
| 284 | - | 284 | + | // retired, not only at create time. createContainer keeps the full | |
| 285 | - | // resolveGuardedBinds resolves the service binds and applies the runtime | 285 | + | // guard as the final check. |
| 286 | - | // guard to them, so a bind overlapping a protected Gordon path fails at | 286 | + | if err := s.deps.Guard.Check(&domain.ContainerConfig{Binds: resolved}); err != nil { |
| 287 | - | // preflight and before the serving generation is retired, not only at | |||
| 288 | - | // create time. createContainer keeps the full guard as the final check. | |||
| 289 | - | func (s *Service) resolveGuardedBinds(app string, svc domain.AppService) ([]domain.ContainerBind, error) { | |||
| 290 | - | binds, err := s.resolveServiceBinds(app, svc) | |||
| 291 | - | if err != nil { | |||
| 292 | - | return nil, err | |||
| 293 | - | } | |||
| 294 | - | if err := s.deps.Guard.Check(&domain.ContainerConfig{Binds: binds}); err != nil { | |||
| 295 | return nil, fmt.Errorf("deployment: app %q service %q: %w", app, svc.Name, err) | 287 | return nil, fmt.Errorf("deployment: app %q service %q: %w", app, svc.Name, err) | ||
| 296 | } | 288 | } | ||
| 297 | - | return binds, nil | 289 | + | return resolved, nil |
| 298 | } | 290 | } | ||
| 299 | 291 | ||||
| 300 | // acquireAppContext takes the GC shared lease first and the per-app | 292 | // acquireAppContext takes the GC shared lease first and the per-app | ||
| @@ -738,7 +730,7 @@ func (s *Service) preflightServices(ctx context.Context, app string, rev domain. | |||||
| 738 | // must fail before any workload mutation. The successful values | 730 | // must fail before any workload mutation. The successful values | ||
| 739 | // are discarded because createAndStart re-resolves against the | 731 | // are discarded because createAndStart re-resolves against the | ||
| 740 | // current policy immediately before the runtime mutation. | 732 | // current policy immediately before the runtime mutation. | ||
| 741 | - | if _, err := s.resolveGuardedBinds(app, svc); err != nil { | 733 | + | if _, err := s.resolveServiceBinds(app, svc); err != nil { |
| 742 | return nil, err | 734 | return nil, err | ||
| 743 | } | 735 | } | ||
| 744 | if err := s.preflightServiceDevices(ctx, app, svc, pinned); err != nil { | 736 | if err := s.preflightServiceDevices(ctx, app, svc, pinned); err != nil { | ||
M internal/usecase/logs/service.go +1 -6
| @@ -9,7 +9,6 @@ import ( | |||
| 9 | 9 | "fmt" | |
| 10 | 10 | "io" | |
| 11 | 11 | "os" | |
| 12 | - | "os/exec" | |
| 13 | 12 | "strconv" | |
| 14 | 13 | "strings" | |
| 15 | 14 | "time" | |
| @@ -38,11 +37,7 @@ type Service struct { | |||
| 38 | 37 | ||
| 39 | 38 | // execCommandContext starts journalctl without Gordon's GORDON_* variables: | |
| 40 | 39 | // the child is dumpable again after exec, so it must not carry secrets. | |
| 41 | - | var execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { | |
| 42 | - | cmd := exec.CommandContext(ctx, name, args...) // #nosec G204 -- callers pass "journalctl" with internal args only | |
| 43 | - | cmd.Env = childenv.Env() | |
| 44 | - | return cmd | |
| 45 | - | } | |
| 40 | + | var execCommandContext = childenv.CommandContext | |
| 46 | 41 | ||
| 47 | 42 | // NewService creates a new log service. App state is wired after the store opens. | |
| 48 | 43 | func NewService( | |
| @@ -9,7 +9,6 @@ import ( | |||||
| 9 | "fmt" | 9 | "fmt" | ||
| 10 | "io" | 10 | "io" | ||
| 11 | "os" | 11 | "os" | ||
| 12 | - | "os/exec" | |||
| 13 | "strconv" | 12 | "strconv" | ||
| 14 | "strings" | 13 | "strings" | ||
| 15 | "time" | 14 | "time" | ||
| @@ -38,11 +37,7 @@ type Service struct { | |||||
| 38 | 37 | ||||
| 39 | // execCommandContext starts journalctl without Gordon's GORDON_* variables: | 38 | // execCommandContext starts journalctl without Gordon's GORDON_* variables: | ||
| 40 | // the child is dumpable again after exec, so it must not carry secrets. | 39 | // the child is dumpable again after exec, so it must not carry secrets. | ||
| 41 | - | var execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { | 40 | + | var execCommandContext = childenv.CommandContext |
| 42 | - | cmd := exec.CommandContext(ctx, name, args...) // #nosec G204 -- callers pass "journalctl" with internal args only | |||
| 43 | - | cmd.Env = childenv.Env() | |||
| 44 | - | return cmd | |||
| 45 | - | } | |||
| 46 | 41 | ||||
| 47 | // NewService creates a new log service. App state is wired after the store opens. | 42 | // NewService creates a new log service. App state is wired after the store opens. | ||
| 48 | func NewService( | 43 | func NewService( | ||
M pkg/childenv/childenv.go +10 -0
| @@ -5,10 +5,20 @@ | |||
| 5 | 5 | package childenv | |
| 6 | 6 | ||
| 7 | 7 | import ( | |
| 8 | + | "context" | |
| 8 | 9 | "os" | |
| 10 | + | "os/exec" | |
| 9 | 11 | "strings" | |
| 10 | 12 | ) | |
| 11 | 13 | ||
| 14 | + | // CommandContext is exec.CommandContext with Env set to Env(). Use it for | |
| 15 | + | // every child process the daemon starts. | |
| 16 | + | func CommandContext(ctx context.Context, name string, args ...string) *exec.Cmd { | |
| 17 | + | cmd := exec.CommandContext(ctx, name, args...) // #nosec G204 -- callers pass constant binaries with validated args | |
| 18 | + | cmd.Env = Env() | |
| 19 | + | return cmd | |
| 20 | + | } | |
| 21 | + | ||
| 12 | 22 | // Env returns the current environment without GORDON_* variables. | |
| 13 | 23 | func Env() []string { | |
| 14 | 24 | return filter(os.Environ()) | |
| @@ -5,10 +5,20 @@ | |||||
| 5 | package childenv | 5 | package childenv | ||
| 6 | 6 | ||||
| 7 | import ( | 7 | import ( | ||
| 8 | + | "context" | |||
| 8 | "os" | 9 | "os" | ||
| 10 | + | "os/exec" | |||
| 9 | "strings" | 11 | "strings" | ||
| 10 | ) | 12 | ) | ||
| 11 | 13 | ||||
| 14 | + | // CommandContext is exec.CommandContext with Env set to Env(). Use it for | |||
| 15 | + | // every child process the daemon starts. | |||
| 16 | + | func CommandContext(ctx context.Context, name string, args ...string) *exec.Cmd { | |||
| 17 | + | cmd := exec.CommandContext(ctx, name, args...) // #nosec G204 -- callers pass constant binaries with validated args | |||
| 18 | + | cmd.Env = Env() | |||
| 19 | + | return cmd | |||
| 20 | + | } | |||
| 21 | + | ||||
| 12 | // Env returns the current environment without GORDON_* variables. | 22 | // Env returns the current environment without GORDON_* variables. | ||
| 13 | func Env() []string { | 23 | func Env() []string { | ||
| 14 | return filter(os.Environ()) | 24 | return filter(os.Environ()) | ||
M pkg/childenv/childenv_test.go +9 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package childenv | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/stretchr/testify/assert" | |
| @@ -16,3 +17,11 @@ func TestFilterDropsGordonVariables(t *testing.T) { | |||
| 16 | 17 | }) | |
| 17 | 18 | assert.Equal(t, []string{"PATH=/usr/bin", "PASSWORD_STORE_DIR=/keys/pass", "GNUPGHOME=/keys/gnupg"}, got) | |
| 18 | 19 | } | |
| 20 | + | ||
| 21 | + | func TestCommandContextStripsGordonVariables(t *testing.T) { | |
| 22 | + | t.Setenv("GORDON_TEST_SECRET", "s3cr3t") | |
| 23 | + | t.Setenv("CHILDENV_KEEP", "1") | |
| 24 | + | cmd := CommandContext(context.Background(), "true") | |
| 25 | + | assert.Contains(t, cmd.Env, "CHILDENV_KEEP=1") | |
| 26 | + | assert.NotContains(t, cmd.Env, "GORDON_TEST_SECRET=s3cr3t") | |
| 27 | + | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package childenv | 1 | package childenv | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/stretchr/testify/assert" | 7 | "github.com/stretchr/testify/assert" | ||
| @@ -16,3 +17,11 @@ func TestFilterDropsGordonVariables(t *testing.T) { | |||||
| 16 | }) | 17 | }) | ||
| 17 | assert.Equal(t, []string{"PATH=/usr/bin", "PASSWORD_STORE_DIR=/keys/pass", "GNUPGHOME=/keys/gnupg"}, got) | 18 | assert.Equal(t, []string{"PATH=/usr/bin", "PASSWORD_STORE_DIR=/keys/pass", "GNUPGHOME=/keys/gnupg"}, got) | ||
| 18 | } | 19 | } | ||
| 20 | + | ||||
| 21 | + | func TestCommandContextStripsGordonVariables(t *testing.T) { | |||
| 22 | + | t.Setenv("GORDON_TEST_SECRET", "s3cr3t") | |||
| 23 | + | t.Setenv("CHILDENV_KEEP", "1") | |||
| 24 | + | cmd := CommandContext(context.Background(), "true") | |||
| 25 | + | assert.Contains(t, cmd.Env, "CHILDENV_KEEP=1") | |||
| 26 | + | assert.NotContains(t, cmd.Env, "GORDON_TEST_SECRET=s3cr3t") | |||
| 27 | + | } | |||