diff --git a/internal/agent/kernelio/fake.go b/internal/agent/kernelio/fake.go index b2313c3..c6c12ef 100644 --- a/internal/agent/kernelio/fake.go +++ b/internal/agent/kernelio/fake.go @@ -7,6 +7,7 @@ import ( "path/filepath" "github.com/Hanalyx/kensa/api" + "github.com/Hanalyx/kensa/internal/agent/fsatomic" ) // FakeSysctlTransport is an in-memory test double implementing @@ -80,20 +81,36 @@ func (f *FakeSysctlTransport) MkdirAll(path string, _ fs.FileMode) error { return nil } -// AtomicReplace writes content to the in-memory persist layer. +// AtomicReplace writes content to an EXISTING in-memory file, modeling +// fsatomic.AtomicReplace: it errors fsatomic.ErrNotExist when the target +// is absent. (The earlier no-op map-set masked the live-caught bug where a +// handler used AtomicReplace on a not-yet-created drop-in.) func (f *FakeSysctlTransport) AtomicReplace(_ context.Context, fullPath string, _ fs.FileMode, content []byte) error { + if _, ok := f.Files[fullPath]; !ok { + return fmt.Errorf("%w: %s", fsatomic.ErrNotExist, fullPath) + } f.Files[fullPath] = string(content) return nil } -// AtomicWrite writes dir/name to the in-memory persist layer. +// AtomicWrite creates a NEW in-memory file, modeling fsatomic.AtomicWrite: +// it errors fsatomic.ErrAlreadyExists when the target already exists. func (f *FakeSysctlTransport) AtomicWrite(_ context.Context, dir, name string, _ fs.FileMode, content []byte) error { - f.Files[filepath.Join(dir, name)] = string(content) + p := filepath.Join(dir, name) + if _, ok := f.Files[p]; ok { + return fmt.Errorf("%w: %s", fsatomic.ErrAlreadyExists, p) + } + f.Files[p] = string(content) return nil } -// AtomicRemove deletes path from the in-memory persist layer. +// AtomicRemove deletes an EXISTING in-memory file, modeling +// fsatomic.AtomicRemove: it errors fsatomic.ErrNotExist when the target is +// absent. func (f *FakeSysctlTransport) AtomicRemove(_ context.Context, fullPath string) error { + if _, ok := f.Files[fullPath]; !ok { + return fmt.Errorf("%w: %s", fsatomic.ErrNotExist, fullPath) + } delete(f.Files, fullPath) return nil } diff --git a/internal/agent/kernelio/writefile.go b/internal/agent/kernelio/writefile.go new file mode 100644 index 0000000..4091b77 --- /dev/null +++ b/internal/agent/kernelio/writefile.go @@ -0,0 +1,45 @@ +package kernelio + +import ( + "context" + "errors" + "io/fs" + "path/filepath" + + "github.com/Hanalyx/kensa/internal/agent/fsatomic" +) + +// WriteFile atomically writes content to fullPath whether or not it +// already exists — the create-OR-replace primitive the drop-in handlers +// need. fsatomic deliberately splits the two cases (AtomicWrite errors +// ErrAlreadyExists on an existing target; AtomicReplace errors ErrNotExist +// on a missing one), so this tries AtomicWrite (create) and falls back to +// AtomicReplace (replace) when the file already exists. The two-step form +// is also race-tolerant: if the file is created or removed by another +// writer between the attempts, the fallback still lands on the right +// primitive. +// +// This is the fix for the live-caught bug where a handler used +// AtomicReplace for a drop-in file that did not yet exist (e.g. a fresh +// /etc/sysctl.d/99-kensa.conf), which failed ErrNotExist on first apply. +func WriteFile(ctx context.Context, ft FileTransport, fullPath string, mode fs.FileMode, content []byte) error { + dir, name := filepath.Split(fullPath) + dir = filepath.Clean(dir) + err := ft.AtomicWrite(ctx, dir, name, mode, content) + if errors.Is(err, fsatomic.ErrAlreadyExists) { + return ft.AtomicReplace(ctx, fullPath, mode, content) + } + return err +} + +// RemoveFile atomically removes fullPath, treating an already-absent file +// as success (fsatomic.AtomicRemove errors ErrNotExist on a missing +// target). Rollback uses it so "ensure this drop-in is gone" is idempotent +// — it must not fail when the file was never created. +func RemoveFile(ctx context.Context, ft FileTransport, fullPath string) error { + err := ft.AtomicRemove(ctx, fullPath) + if errors.Is(err, fsatomic.ErrNotExist) { + return nil + } + return err +} diff --git a/internal/agent/kernelio/writefile_test.go b/internal/agent/kernelio/writefile_test.go new file mode 100644 index 0000000..632d16b --- /dev/null +++ b/internal/agent/kernelio/writefile_test.go @@ -0,0 +1,59 @@ +package kernelio_test + +import ( + "context" + "testing" + + "github.com/Hanalyx/kensa/internal/agent/kernelio" +) + +// WriteFile is create-OR-replace: it succeeds whether or not the target +// exists (the live-caught bug was AtomicReplace failing on a new file). +// +// @spec kernelio-sysctl +// @ac AC-03 +func TestWriteFile_CreateOrReplace(t *testing.T) { + t.Run("kernelio-sysctl/AC-03", func(t *testing.T) {}) + f := kernelio.NewFakeSysctl() + const p = "/etc/sysctl.d/99-kensa.conf" + + // Create (absent → must succeed; the bug was an ErrNotExist here). + if err := kernelio.WriteFile(context.Background(), f, p, 0o644, []byte("a\n")); err != nil { + t.Fatalf("WriteFile create: %v", err) + } + if f.Files[p] != "a\n" { + t.Errorf("after create = %q, want a\\n", f.Files[p]) + } + // Replace (present → must also succeed). + if err := kernelio.WriteFile(context.Background(), f, p, 0o644, []byte("b\n")); err != nil { + t.Fatalf("WriteFile replace: %v", err) + } + if f.Files[p] != "b\n" { + t.Errorf("after replace = %q, want b\\n", f.Files[p]) + } +} + +// RemoveFile treats an already-absent file as success (the live-caught +// rollback bug was AtomicRemove failing ErrNotExist on a never-created +// drop-in, which aborted rollback and left the runtime value unrestored). +// +// @spec kernelio-sysctl +// @ac AC-03 +func TestRemoveFile_AbsentIsNoop(t *testing.T) { + t.Run("kernelio-sysctl/AC-03", func(t *testing.T) {}) + f := kernelio.NewFakeSysctl() + const p = "/etc/sysctl.d/99-kensa.conf" + + // Remove an absent file → no error (the fix). + if err := kernelio.RemoveFile(context.Background(), f, p); err != nil { + t.Errorf("RemoveFile(absent) = %v, want nil", err) + } + // Remove a present file → gone. + f.Files[p] = "x" + if err := kernelio.RemoveFile(context.Background(), f, p); err != nil { + t.Fatalf("RemoveFile(present): %v", err) + } + if _, ok := f.Files[p]; ok { + t.Error("file should be gone after RemoveFile") + } +} diff --git a/internal/handlers/auditruleset/auditruleset.go b/internal/handlers/auditruleset/auditruleset.go index 3106967..3425e86 100644 --- a/internal/handlers/auditruleset/auditruleset.go +++ b/internal/handlers/auditruleset/auditruleset.go @@ -27,6 +27,7 @@ import ( "github.com/Hanalyx/kensa/api" "github.com/Hanalyx/kensa/internal/agent/auditnl" + "github.com/Hanalyx/kensa/internal/agent/kernelio" ) // mechanism is the canonical handler name. @@ -133,7 +134,7 @@ func (h *Handler) applyNetlink(ctx context.Context, at auditnl.AuditTransport, p } content := "# Managed by Kensa.\n" + p.Rule + "\n" - if werr := at.AtomicReplace(ctx, p.RuleFile, auditFileMode, []byte(content)); werr != nil { + if werr := kernelio.WriteFile(ctx, at, p.RuleFile, auditFileMode, []byte(content)); werr != nil { return nil, fmt.Errorf("audit_rule_set: persist write: %w", werr) } return &api.StepResult{ @@ -332,10 +333,10 @@ func (h *Handler) Rollback(ctx context.Context, transport api.Transport, pre *ap func (h *Handler) rollbackNetlink(ctx context.Context, at auditnl.AuditTransport, path string, fileExisted bool, priorContent string, added []string) (*api.RollbackResult, error) { // Restore persist layer first. if fileExisted { - if err := at.AtomicReplace(ctx, path, auditFileMode, []byte(priorContent)); err != nil { + if err := kernelio.WriteFile(ctx, at, path, auditFileMode, []byte(priorContent)); err != nil { return nil, fmt.Errorf("audit_rule_set: rollback persist write: %w", err) } - } else if err := at.AtomicRemove(ctx, path); err != nil { + } else if err := kernelio.RemoveFile(ctx, at, path); err != nil { return nil, fmt.Errorf("audit_rule_set: rollback persist remove: %w", err) } diff --git a/internal/handlers/dconfset/dconfset.go b/internal/handlers/dconfset/dconfset.go index 52e7c4d..c09576b 100644 --- a/internal/handlers/dconfset/dconfset.go +++ b/internal/handlers/dconfset/dconfset.go @@ -220,7 +220,7 @@ func (h *Handler) applyKernel(ctx context.Context, ft kernelio.FileTransport, tr if _, existed, rerr := ft.ReadFileIfExists(paths.profile); rerr != nil { return nil, fmt.Errorf("dconf_set: read profile: %w", rerr) } else if !existed { - if werr := ft.AtomicReplace(ctx, paths.profile, dconfFileMode, []byte(profileBody(p.DB))); werr != nil { + if werr := kernelio.WriteFile(ctx, ft, paths.profile, dconfFileMode, []byte(profileBody(p.DB))); werr != nil { return nil, fmt.Errorf("dconf_set: profile write: %w", werr) } } @@ -228,7 +228,7 @@ func (h *Handler) applyKernel(ctx context.Context, ft kernelio.FileTransport, tr if err := ft.MkdirAll(paths.dbDir, dconfDirMode); err != nil { return nil, fmt.Errorf("dconf_set: mkdir db.d: %w", err) } - if err := ft.AtomicReplace(ctx, paths.snippet, dconfFileMode, []byte(snippetBody(p))); err != nil { + if err := kernelio.WriteFile(ctx, ft, paths.snippet, dconfFileMode, []byte(snippetBody(p))); err != nil { return nil, fmt.Errorf("dconf_set: snippet write: %w", err) } // 3. Optional lock. @@ -236,7 +236,7 @@ func (h *Handler) applyKernel(ctx context.Context, ft kernelio.FileTransport, tr if err := ft.MkdirAll(paths.locksD, dconfDirMode); err != nil { return nil, fmt.Errorf("dconf_set: mkdir locks: %w", err) } - if err := ft.AtomicReplace(ctx, paths.lock, dconfFileMode, []byte(lockBody(p))); err != nil { + if err := kernelio.WriteFile(ctx, ft, paths.lock, dconfFileMode, []byte(lockBody(p))); err != nil { return nil, fmt.Errorf("dconf_set: lock write: %w", err) } } @@ -456,10 +456,10 @@ func (h *Handler) Rollback(ctx context.Context, transport api.Transport, pre *ap // `dconf update` (the compile step, stays shell). func (h *Handler) rollbackKernel(ctx context.Context, ft kernelio.FileTransport, transport api.Transport, filePath, priorContent string, fileExisted bool) (*api.RollbackResult, error) { if fileExisted { - if err := ft.AtomicReplace(ctx, filePath, dconfFileMode, []byte(priorContent)); err != nil { + if err := kernelio.WriteFile(ctx, ft, filePath, dconfFileMode, []byte(priorContent)); err != nil { return nil, fmt.Errorf("dconf_set: rollback restore: %w", err) } - } else if err := ft.AtomicRemove(ctx, filePath); err != nil { + } else if err := kernelio.RemoveFile(ctx, ft, filePath); err != nil { return nil, fmt.Errorf("dconf_set: rollback remove: %w", err) } if res, err := transport.Run(ctx, "dconf update"); err != nil { diff --git a/internal/handlers/kernelmoduledisable/kernelmoduledisable.go b/internal/handlers/kernelmoduledisable/kernelmoduledisable.go index 8042cb5..3a1ee82 100644 --- a/internal/handlers/kernelmoduledisable/kernelmoduledisable.go +++ b/internal/handlers/kernelmoduledisable/kernelmoduledisable.go @@ -104,7 +104,7 @@ func (h *Handler) Apply(ctx context.Context, transport api.Transport, params api // persistent blacklist is what guarantees it stays out. func (h *Handler) applyKernel(ctx context.Context, mt kernelio.ModuleTransport, p *Params) (*api.StepResult, error) { path := blacklistPath(p.Module) - if err := mt.AtomicReplace(ctx, path, blacklistMode, []byte(blacklistContent(p.Module))); err != nil { + if err := kernelio.WriteFile(ctx, mt, path, blacklistMode, []byte(blacklistContent(p.Module))); err != nil { return nil, fmt.Errorf("kernel_module_disable: write blacklist: %w", err) } // Best-effort unload; the blacklist is the load-bearing change. @@ -224,10 +224,10 @@ func (h *Handler) Rollback(ctx context.Context, transport api.Transport, pre *ap // rollbackKernel restores or removes the blacklist drop-in atomically. func (h *Handler) rollbackKernel(ctx context.Context, mt kernelio.ModuleTransport, path string, fileExisted bool, priorContent string) (*api.RollbackResult, error) { if fileExisted { - if err := mt.AtomicReplace(ctx, path, blacklistMode, []byte(priorContent)); err != nil { + if err := kernelio.WriteFile(ctx, mt, path, blacklistMode, []byte(priorContent)); err != nil { return nil, fmt.Errorf("kernel_module_disable: rollback rewrite %s: %w", path, err) } - } else if err := mt.AtomicRemove(ctx, path); err != nil { + } else if err := kernelio.RemoveFile(ctx, mt, path); err != nil { return nil, fmt.Errorf("kernel_module_disable: rollback remove %s: %w", path, err) } return &api.RollbackResult{ diff --git a/internal/handlers/sysctlset/sysctlset.go b/internal/handlers/sysctlset/sysctlset.go index 6fda6eb..6551ea1 100644 --- a/internal/handlers/sysctlset/sysctlset.go +++ b/internal/handlers/sysctlset/sysctlset.go @@ -136,7 +136,7 @@ func (h *Handler) applyKernel(_ context.Context, k kernelio.SysctlTransport, p * Detail: fmt.Sprintf("sysctl_set: runtime apply failed: %v", err), }, nil } - if err := k.AtomicReplace(context.Background(), p.PersistFile, persistMode, []byte(persistContent(p.Key, p.Value))); err != nil { + if err := kernelio.WriteFile(context.Background(), k, p.PersistFile, persistMode, []byte(persistContent(p.Key, p.Value))); err != nil { return nil, fmt.Errorf("sysctl_set: persist write failed: %w", err) } return &api.StepResult{ @@ -294,10 +294,10 @@ func (h *Handler) rollbackKernel(k kernelio.SysctlTransport, key, persistFile, r // Restore persist layer first; the runtime write then sets the // captured runtime value (which may differ from the file's value). if persistFileExisted { - if err := k.AtomicReplace(context.Background(), persistFile, persistMode, []byte(persistFileContent)); err != nil { + if err := kernelio.WriteFile(context.Background(), k, persistFile, persistMode, []byte(persistFileContent)); err != nil { return nil, fmt.Errorf("sysctl_set: rollback persist write failed: %w", err) } - } else if err := k.AtomicRemove(context.Background(), persistFile); err != nil { + } else if err := kernelio.RemoveFile(context.Background(), k, persistFile); err != nil { return nil, fmt.Errorf("sysctl_set: rollback persist remove failed: %w", err) } if err := k.WriteSysctl(key, runtimeValue); err != nil {