diff --git a/internal/hooks/order.go b/internal/hooks/order.go index 4e373e7..8c80d56 100644 --- a/internal/hooks/order.go +++ b/internal/hooks/order.go @@ -17,12 +17,12 @@ import ( // lexically, then names bytewise. After hooks keep their relative order. func SortBefore(hooks []deployment.Hook) { sort.SliceStable(hooks, func(first, second int) bool { - a, b := hooks[first], hooks[second] - if a.Phase != b.Phase { - return a.Phase == deployment.HookBefore + firstHook, secondHook := hooks[first], hooks[second] + if firstHook.Phase != secondHook.Phase { + return firstHook.Phase == deployment.HookBefore } - if a.Phase == deployment.HookBefore { - return LessBefore(a, b) + if firstHook.Phase == deployment.HookBefore { + return LessBefore(firstHook, secondHook) } return false }) @@ -32,12 +32,12 @@ func SortBefore(hooks []deployment.Hook) { // last, then names bytewise. Before hooks keep their relative order. func SortAfter(hooks []deployment.Hook) { sort.SliceStable(hooks, func(first, second int) bool { - a, b := hooks[first], hooks[second] - if a.Phase != b.Phase { - return a.Phase == deployment.HookBefore + firstHook, secondHook := hooks[first], hooks[second] + if firstHook.Phase != secondHook.Phase { + return firstHook.Phase == deployment.HookBefore } - if a.Phase == deployment.HookAfter { - return LessAfter(a, b) + if firstHook.Phase == deployment.HookAfter { + return LessAfter(firstHook, secondHook) } return false }) diff --git a/internal/repository/overlay.go b/internal/repository/overlay.go index c39ebd4..05712cc 100644 --- a/internal/repository/overlay.go +++ b/internal/repository/overlay.go @@ -10,21 +10,21 @@ import ( ) // ResolvePlatform merges the base scan with the platform layer tree. -func ResolvePlatform(root string, base ScanResult, platform deployment.Layer) ([]deployment.ManagedFile, error) { - if !platform.Valid() { - return nil, fmt.Errorf("repository: unknown platform layer %q", platform) +func ResolvePlatform(root string, baseScan ScanResult, platformLayer deployment.Layer) ([]deployment.ManagedFile, error) { + if !platformLayer.Valid() { + return nil, fmt.Errorf("repository: unknown platform layer %q", platformLayer) } - r := resolver{root: root, base: base, platform: platform} - rootView, err := scanLayerTree(root, deployment.NewScope(""), platform) + platformResolver := resolver{root: root, base: baseScan, platform: platformLayer} + platformRootView, err := scanLayerTree(root, deployment.NewScope(""), platformLayer) if err != nil { return nil, err } - records, err := resolveScopeFiles(r.base, deployment.NewScope(""), rootView) + records, err := resolveScopeFiles(platformResolver.base, deployment.NewScope(""), platformRootView) if err != nil { return nil, err } - for _, group := range base.Groups { - groupRecords, err := r.resolveScope(deployment.NewScope(group), rootView) + for _, group := range baseScan.Groups { + groupRecords, err := platformResolver.resolveScope(deployment.NewScope(group), platformRootView) if err != nil { return nil, err } @@ -41,15 +41,15 @@ type resolver struct { } // resolveScope merges one group scope, skipping groups replaced by files. -func (r *resolver) resolveScope(scope deployment.Scope, rootView layerView) ([]deployment.ManagedFile, error) { - if _, replaced := rootView.files[scope.Group]; replaced { +func (resolver *resolver) resolveScope(scope deployment.Scope, platformRootView layerView) ([]deployment.ManagedFile, error) { + if _, replaced := platformRootView.files[scope.Group]; replaced { return nil, nil } - view, err := scanLayerTree(r.root, scope, r.platform) + platformView, err := scanLayerTree(resolver.root, scope, resolver.platform) if err != nil { return nil, err } - return resolveScopeFiles(r.base, scope, view) + return resolveScopeFiles(resolver.base, scope, platformView) } type layerView struct { @@ -58,14 +58,14 @@ type layerView struct { } // covers reports whether the platform layer replaces a base target. -func (v layerView) covers(target string) bool { - if _, ok := v.files[target]; ok || v.dirs[target] { +func (view layerView) covers(target string) bool { + if _, ok := view.files[target]; ok || view.dirs[target] { return true } segments := strings.Split(target, "/") for length := 1; length < len(segments); length++ { prefix := strings.Join(segments[:length], "/") - if _, ok := v.files[prefix]; ok { + if _, ok := view.files[prefix]; ok { return true } } @@ -163,20 +163,20 @@ type layerWalker struct { view layerView } -func (w *layerWalker) walk(relative string, kind deployment.FileKind) error { - entries, err := os.ReadDir(filepath.Join(w.absolute, relative)) +func (walker *layerWalker) walk(relativePath string, fileKind deployment.FileKind) error { + entries, err := os.ReadDir(filepath.Join(walker.absolute, relativePath)) if err != nil { return err } for _, entry := range entries { - entryKind, skip, err := classifyEntry(relative, entry, kind) + entryKind, skip, err := classifyEntry(relativePath, entry, fileKind) if err != nil { return err } if skip { continue } - if err := w.visit(filepath.Join(relative, entry.Name()), entry, entryKind); err != nil { + if err := walker.visit(filepath.Join(relativePath, entry.Name()), entry, entryKind); err != nil { return err } } @@ -201,41 +201,41 @@ func classifyEntry(relative string, entry os.DirEntry, kind deployment.FileKind) } } -func (w *layerWalker) visit(path string, entry os.DirEntry, kind deployment.FileKind) error { - target, err := w.target(path, kind) +func (walker *layerWalker) visit(path string, entry os.DirEntry, kind deployment.FileKind) error { + target, err := walker.target(path, kind) if err != nil { return err } if entry.IsDir() { - w.view.dirs[target] = target != "" - return w.walk(path, kind) + walker.view.dirs[target] = target != "" + return walker.walk(path, kind) } if !entry.Type().IsRegular() { - return fmt.Errorf("repository: non-regular source entry %q", filepath.Join(w.relative, path)) + return fmt.Errorf("repository: non-regular source entry %q", filepath.Join(walker.relative, path)) } info, err := entry.Info() if err != nil { return err } candidate := Candidate{ - Scope: w.scope, Layer: w.layer, Kind: kind, - SourceRepoPath: filepath.Join(w.relative, path), SourceAbsPath: filepath.Join(w.absolute, path), + Scope: walker.scope, Layer: walker.layer, Kind: kind, + SourceRepoPath: filepath.Join(walker.relative, path), SourceAbsPath: filepath.Join(walker.absolute, path), ExecutableBits: info.Mode() & 0o111, } - existing, ok := w.view.files[target] + existing, ok := walker.view.files[target] if ok && existing.Kind != kind { return fmt.Errorf("repository: ordinary and secret sources collide at %q", target) } - w.view.files[target] = candidate + walker.view.files[target] = candidate return nil } -func (w *layerWalker) target(path string, kind deployment.FileKind) (string, error) { +func (walker *layerWalker) target(path string, kind deployment.FileKind) (string, error) { if kind != deployment.FileSecret { return path, nil } target := strings.TrimPrefix(strings.TrimPrefix(path, "_secrets"), "/") - if w.scope.IsRoot() { + if walker.scope.IsRoot() { first := strings.Split(target, "/")[0] representable := strings.HasPrefix(first, ".") || (!strings.Contains(target, "/") && !strings.HasPrefix(first, "_")) diff --git a/internal/repository/scan.go b/internal/repository/scan.go index f9736e6..6964524 100644 --- a/internal/repository/scan.go +++ b/internal/repository/scan.go @@ -109,12 +109,12 @@ func (s *scopeScanner) beginGroup(entry os.DirEntry) error { return err } s.groups = append(s.groups, name) - root, scope := s.scopeRoot, s.scope + previousScopeRoot, previousScope := s.scopeRoot, s.scope s.scopeRoot = filepath.Join(s.scopeRoot, name) s.scope = deployment.NewScope(name) s.rootTree = false err := s.scanScopeRoot() - s.scopeRoot, s.scope, s.rootTree = root, scope, true + s.scopeRoot, s.scope, s.rootTree = previousScopeRoot, previousScope, true return err } diff --git a/internal/routes/config.go b/internal/routes/config.go index cd3f228..5f0ee68 100644 --- a/internal/routes/config.go +++ b/internal/routes/config.go @@ -50,45 +50,45 @@ type Config struct { // appear in more than one section: the active plan unions `all` with the host // platform section, so cross-section repetition is resolved at activation time. func Decode(data []byte) (Config, error) { - raw, err := decodeRaw(data) + decoded, err := decodeRouteInput(data) if err != nil { return Config{}, err } - if raw.Version != routeVersion { - return Config{}, versionError(raw.Version) + if decoded.Version != routeVersion { + return Config{}, versionError(decoded.Version) } - return buildConfig(raw) + return buildConfig(decoded) } func versionError(version int) error { return fmt.Errorf("routes: unsupported version %d", version) } -type rawConfig struct { - Version int `toml:"version"` - Symlinks rawSymlinks `toml:"symlinks"` +type routeInput struct { + Version int `toml:"version"` + Symlinks symlinkSections `toml:"symlinks"` } -type rawSymlinks struct { +type symlinkSections struct { All map[string][]string `toml:"all"` Darwin map[string][]string `toml:"darwin"` Linux map[string][]string `toml:"linux"` } -func decodeRaw(data []byte) (rawConfig, error) { - var raw rawConfig +func decodeRouteInput(data []byte) (routeInput, error) { + var decoded routeInput reader := bytes.NewReader(data) - err := toml.NewDecoder(reader).DisallowUnknownFields().Decode(&raw) - return raw, err + err := toml.NewDecoder(reader).DisallowUnknownFields().Decode(&decoded) + return decoded, err } -func buildConfig(raw rawConfig) (Config, error) { - declarations, err := collectDeclarations(raw.Symlinks) +func buildConfig(decoded routeInput) (Config, error) { + declarations, err := collectDeclarations(decoded.Symlinks) if err != nil { return Config{}, err } sortDeclarations(declarations) - return Config{Version: raw.Version, Declarations: declarations}, nil + return Config{Version: decoded.Version, Declarations: declarations}, nil } func sortDeclarations(declarations []Declaration) { @@ -101,14 +101,14 @@ func byCanonical(declarations []Declaration) func(int, int) bool { } } -func collectDeclarations(syms rawSymlinks) ([]Declaration, error) { - sections := []routeSection{ - {name: SectionAll, rows: syms.All}, - {name: SectionDarwin, rows: syms.Darwin}, - {name: SectionLinux, rows: syms.Linux}, +func collectDeclarations(decodedSymlinkSections symlinkSections) ([]Declaration, error) { + sectionList := []routeSection{ + {name: SectionAll, rows: decodedSymlinkSections.All}, + {name: SectionDarwin, rows: decodedSymlinkSections.Darwin}, + {name: SectionLinux, rows: decodedSymlinkSections.Linux}, } var declarations []Declaration - for _, section := range sections { + for _, section := range sectionList { added, err := sectionDeclarations(section) if err != nil { return nil, err diff --git a/internal/secrets/client.go b/internal/secrets/client.go index 07e43b8..a821d03 100644 --- a/internal/secrets/client.go +++ b/internal/secrets/client.go @@ -54,6 +54,9 @@ type Request struct { // caller-owned; on every other path it is zeroed and discarded, and the error // carries only the operation, safe source path, and exit status. func (client *Client) Run(ctx context.Context, request Request) ([]byte, error) { + if request.StdoutLimit < 0 { + return nil, failure.New(failure.Operational, "sops output limit must not be negative", nil) + } ctx, cancel := context.WithCancel(ctx) defer cancel() stdout := newBounded(request.StdoutLimit, cancel) diff --git a/internal/secrets/client_test.go b/internal/secrets/client_test.go index 2115143..a07b038 100644 --- a/internal/secrets/client_test.go +++ b/internal/secrets/client_test.go @@ -21,6 +21,7 @@ func TestSOPSClient(t *testing.T) { run func(*testing.T) }{ {"missing executable", testMissingExecutable}, + {"negative stdout limit", testNegativeStdoutLimit}, {"nonzero exit", testNonzeroExit}, {"large stderr", testLargeStderr}, {"stdout over limit", testStdoutOverLimit}, @@ -54,6 +55,22 @@ func testMissingExecutable(t *testing.T) { } } +func testNegativeStdoutLimit(t *testing.T) { + executable := sops.Build(t) + repository := t.TempDir() + client, environment := newTestClient(t, clientTarget{executable: executable, repository: repository}) + request := basicRequest("encrypt", "app/token") + request.StdoutLimit = -1 + output, err := client.Run(context.Background(), request) + expectKind(t, err, failure.Operational) + if output != nil { + t.Fatalf("output = %v, want none", output) + } + if _, recorded := peekRecord(envValue(environment, "FAKE_SOPS_RECORD")); recorded { + t.Fatal("fixture was launched") + } +} + // testNonzeroExit also covers the plaintext-stderr redaction rule: neither // captured stream may enter the returned error. func testNonzeroExit(t *testing.T) { diff --git a/internal/testfixture/sops/executable.go b/internal/testfixture/sops/executable.go index 0ff180a..c6ac068 100644 --- a/internal/testfixture/sops/executable.go +++ b/internal/testfixture/sops/executable.go @@ -18,12 +18,11 @@ import ( "time" ) -// specEnv and recordEnv name the contract between Command and the compiled -// fake: the behavior spec path and the metadata log path. +// These environment variables connect Command to the compiled fake. const ( - specEnv = "FAKE_SOPS_SPEC" - recordEnv = "FAKE_SOPS_RECORD" - childEnv = "FAKE_SOPS_CHILD" + behaviorSpecEnvironment = "FAKE_SOPS_SPEC" + invocationRecordEnvironment = "FAKE_SOPS_RECORD" + fixtureChildEnvironment = "FAKE_SOPS_CHILD" ) // Executable is a handle to the compiled fake binary. @@ -54,16 +53,20 @@ func Build(t *testing.T) *Executable { } // Command returns an *exec.Cmd wired to run the fake with the given behavior. -// The behavior is serialized to a spec file; the fake reads it via specEnv. +// The behavior is serialized to a spec file; the fake reads it via the +// behaviorSpecEnvironment variable. func (executable *Executable) Command(behavior Behavior) (*exec.Cmd, error) { directory := filepath.Dir(executable.Path) - spec, err := writeSpec(directory, behavior) + behaviorSpecPath, err := writeSpec(directory, behavior) + if err != nil { + return nil, err + } + record, err := uniquePath(directory, "record") if err != nil { return nil, err } - record := uniquePath(directory, "record") cmd := exec.Command(executable.Path) - cmd.Env = append(os.Environ(), specEnv+"="+spec, recordEnv+"="+record) + cmd.Env = append(os.Environ(), behaviorSpecEnvironment+"="+behaviorSpecPath, invocationRecordEnvironment+"="+record) return cmd, nil } @@ -103,14 +106,16 @@ func writeSpec(directory string, behavior Behavior) (string, error) { return file.Name(), nil } -func uniquePath(directory, prefix string) string { +func uniquePath(directory, prefix string) (string, error) { file, err := os.CreateTemp(directory, prefix+"-*.json") if err != nil { - return "" + return "", err } path := file.Name() - file.Close() - return path + if err := file.Close(); err != nil { + return "", err + } + return path, nil } // fakeSource is the standalone program compiled into the fixture binary. It @@ -129,7 +134,7 @@ import ( "time" ) -type spec struct { +type behaviorSpec struct { Stdout []byte Stderr []byte ExitCode int @@ -154,42 +159,42 @@ func main() { } func run() { - current := loadSpec() + behavior := loadBehavior() stdin, _ := io.ReadAll(os.Stdin) - rec := record{Argv: os.Args, Cwd: cwd(), Stdin: stdin, Pid: os.Getpid()} - if current.Sleep > 0 { - rec.ChildPid = spawnChild() - } - writeRecord(rec) - os.Stderr.Write(current.Stderr) - os.Stdout.Write(current.Stdout) - if current.EchoStdin { + invocationRecord := record{Argv: os.Args, Cwd: currentWorkingDirectory(), Stdin: stdin, Pid: os.Getpid()} + if behavior.Sleep > 0 { + invocationRecord.ChildPid = spawnChild() + } + writeRecord(invocationRecord) + os.Stderr.Write(behavior.Stderr) + os.Stdout.Write(behavior.Stdout) + if behavior.EchoStdin { os.Stdout.Write(stdin) } - if current.Sleep > 0 { - time.Sleep(current.Sleep) + if behavior.Sleep > 0 { + time.Sleep(behavior.Sleep) } - os.Exit(current.ExitCode) + os.Exit(behavior.ExitCode) } -func loadSpec() spec { +func loadBehavior() behaviorSpec { data, err := os.ReadFile(os.Getenv("FAKE_SOPS_SPEC")) if err != nil { os.Exit(2) } - var current spec - if err := json.Unmarshal(data, ¤t); err != nil { + var behavior behaviorSpec + if err := json.Unmarshal(data, &behavior); err != nil { os.Exit(2) } - return current + return behavior } -func writeRecord(rec record) { +func writeRecord(invocationRecord record) { path := os.Getenv("FAKE_SOPS_RECORD") if path == "" { return } - data, err := json.Marshal(rec) + data, err := json.Marshal(invocationRecord) if err != nil { return } @@ -197,19 +202,19 @@ func writeRecord(rec record) { } func spawnChild() int { - exe, err := os.Executable() + executablePath, err := os.Executable() if err != nil { return 0 } - cmd := exec.Command(exe) - cmd.Env = append(os.Environ(), "FAKE_SOPS_CHILD=1") - if err := cmd.Start(); err != nil { + childProcess := exec.Command(executablePath) + childProcess.Env = append(os.Environ(), "FAKE_SOPS_CHILD=1") + if err := childProcess.Start(); err != nil { return 0 } - return cmd.Process.Pid + return childProcess.Process.Pid } -func cwd() string { +func currentWorkingDirectory() string { dir, err := os.Getwd() if err != nil { return "" diff --git a/internal/testfixture/sops/executable_test.go b/internal/testfixture/sops/executable_test.go index aeed4c8..2e3e894 100644 --- a/internal/testfixture/sops/executable_test.go +++ b/internal/testfixture/sops/executable_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "os" "os/exec" + "path/filepath" "strings" "syscall" "testing" @@ -20,6 +21,7 @@ func TestSOPSExecutableFixture(t *testing.T) { {"large output", testLargeOutput}, {"descendant dies with group", testDescendantDiesWithGroup}, {"cleanup removes the binary", testCleanupRemovesBinary}, + {"record path creation failure", testRecordPathCreationFailure}, } for _, scenario := range scenarios { t.Run(scenario.name, scenario.run) @@ -91,6 +93,14 @@ func testCleanupRemovesBinary(t *testing.T) { } } +func testRecordPathCreationFailure(t *testing.T) { + directory := t.TempDir() + _, err := uniquePath(filepath.Join(directory, "missing"), "record") + if err == nil { + t.Fatal("missing record directory was accepted") + } +} + func waitForChild(t *testing.T, cmd *exec.Cmd) int { t.Helper() path := recordPath(cmd) @@ -106,7 +116,7 @@ func waitForChild(t *testing.T, cmd *exec.Cmd) int { } func recordPath(cmd *exec.Cmd) string { - prefix := recordEnv + "=" + prefix := invocationRecordEnvironment + "=" for _, entry := range cmd.Env { if strings.HasPrefix(entry, prefix) { return strings.TrimPrefix(entry, prefix) @@ -120,11 +130,11 @@ func childPidOf(path string) int { if err != nil { return 0 } - var rec struct{ ChildPid int } - if err := json.Unmarshal(data, &rec); err != nil { + var invocationRecord struct{ ChildPid int } + if err := json.Unmarshal(data, &invocationRecord); err != nil { return 0 } - return rec.ChildPid + return invocationRecord.ChildPid } func killGroup(t *testing.T, pid int) {