nebel add: encrypt already-tracked files immediately

USER_INTERACTIONS.md

@@ -101,6 +101,22 @@ $ nebel add file "secrets/*.pem"
 Added rule: secrets/*.pem (mode: file)
 ```
 
+If `secrets/*.pem` already matches files that were tracked before this
+rule existed, they're encrypted right away, not left for the next edit to
+pick up:
+
+```
+$ nebel add file "secrets/*.pem"
+Added rule: secrets/*.pem (mode: file)
+Encrypted and staged 2 files already tracked and matching this rule:
+  secrets/prod.pem
+  secrets/staging.pem
+
+⚠ This only protects future commits — the old plaintext is still
+  recoverable from git history. If this repo has been pushed anywhere,
+  change the underlying secret(s), not just the file.
+```
+
 ### `add field <file> [path...]` — per-value mode
 
 For config you still want to read and diff in git. Paths are dot notation
@@ -138,8 +154,16 @@ Added to rule config/staging.yaml (mode: value):
 - Re-running with the same paths is a no-op.
 - Supported formats: YAML and JSON. `.env` and `.properties` are not
   implemented yet.
-- Both subcommands also add the pattern to `.gitattributes`. After that the
-  normal `git add`/`git commit` flow picks the rule up — no further step.
+- Both subcommands also add the pattern to `.gitattributes`, then
+  immediately re-encrypt any file the new rule already matches among
+  files git currently tracks — no further step, even for a file that
+  predates the rule and hasn't been touched since. (A brand new rule
+  commonly matches nothing yet — that's not an error, just nothing to
+  report.) Anything it re-encrypts was, by definition, still readable in
+  plaintext form in git history before this command ran; if the repo has
+  been pushed anywhere, treat the underlying secret as exposed and change
+  it (not `nebel rotate`, which only changes the shared password — see §
+  4), the same as after any other accidental plaintext commit.
 
 ## 3. `nebel status`
 

cmd/nebel/add.go

@@ -76,7 +76,10 @@ func runAddFile(args []string) error {
 			return fmt.Errorf("%q is already registered with mode: %s — mode conflict", glob, r.Mode)
 		}
 		fmt.Printf("Rule already exists: %s (mode: file)\n", glob)
-		return addGitattributesPattern(root, glob) // idempotent; covers a hand-edited config missing the .gitattributes line
+		if err := addGitattributesPattern(root, glob); err != nil { // idempotent; covers a hand-edited config missing the .gitattributes line
+			return err
+		}
+		return renormalizeAndReport(root, cfg, glob)
 	}
 
 	cfg.Rules = append(cfg.Rules, config.Rule{Files: glob, Mode: config.ModeFile})
@@ -88,7 +91,7 @@ func runAddFile(args []string) error {
 	}
 
 	fmt.Printf("Added rule: %s (mode: file)\n", glob)
-	return nil
+	return renormalizeAndReport(root, cfg, glob)
 }
 
 // runAddField implements `nebel add field <file> [path...]` (AC-8.2).
@@ -134,7 +137,10 @@ func runAddField(args []string) error {
 	}
 	if len(added) == 0 {
 		fmt.Printf("No new fields to add for %s.\n", target)
-		return addGitattributesPattern(root, target)
+		if err := addGitattributesPattern(root, target); err != nil {
+			return err
+		}
+		return renormalizeAndReport(root, cfg, target)
 	}
 
 	if err := applyFieldRule(cfg, target, added); err != nil {
@@ -151,7 +157,7 @@ func runAddField(args []string) error {
 	for _, path := range added {
 		fmt.Printf("  %s\n", path)
 	}
-	return nil
+	return renormalizeAndReport(root, cfg, target)
 }
 
 // existingFields returns the paths already configured for pattern.
@@ -203,6 +209,68 @@ func applyFieldRule(cfg *config.Config, pattern string, added []string) error {
 	return nil
 }
 
+// filesMatchingNewRule returns every currently tracked file that
+// cfg.MatchRule resolves to the rule named pattern — the one this `add`
+// call just wrote or reconfirmed — rather than whatever a raw pathspec
+// match on pattern would return. That distinction matters twice over:
+// spec 08's glob and .gitattributes' pattern can disagree on what they
+// match (audit finding, cross-directory `*.env` semantics), and `git add
+// --renormalize`'s own pathspec form errors outright when pattern
+// matches no tracked file yet — which a brand-new glob commonly does,
+// before anything matching it has ever been committed.
+func filesMatchingNewRule(root string, cfg *config.Config, pattern string) ([]string, error) {
+	tracked, err := gitutil.TrackedFiles(root)
+	if err != nil {
+		return nil, err
+	}
+	var matched []string
+	for _, f := range tracked {
+		if r, ok := cfg.MatchRule(f); ok && r.Files == pattern {
+			matched = append(matched, f)
+		}
+	}
+	return matched, nil
+}
+
+// renormalizeAndReport brings every already-tracked file matching
+// pattern's rule under that rule immediately, instead of leaving it
+// exactly as it was until something unrelated later edits it.
+//
+// Without this, a file that was already tracked and unchanged before
+// `add` registered a rule for it can sit unencrypted indefinitely
+// afterward: git only re-invokes the clean filter for a path when it
+// can't trust its own cached stat info for that path, and a settled,
+// already-tracked file commonly gives it no reason not to — a new
+// `.gitattributes` line routing the path through nebel notwithstanding.
+// `add`'s own documented "no further step" promise depends on this.
+func renormalizeAndReport(root string, cfg *config.Config, pattern string) error {
+	files, err := filesMatchingNewRule(root, cfg, pattern)
+	if err != nil {
+		return err
+	}
+	if len(files) == 0 {
+		return nil
+	}
+
+	migrated, err := gitutil.Renormalize(root, files)
+	if err != nil {
+		return fmt.Errorf("encrypting already-tracked files matching %s (fix by hand with `git add --renormalize -- %s`): %w", pattern, strings.Join(files, " "), err)
+	}
+	if len(migrated) == 0 {
+		return nil
+	}
+
+	fmt.Printf("Encrypted and staged %s already tracked and matching this rule:\n", plural(len(migrated), "file"))
+	for _, path := range migrated {
+		fmt.Printf("  %s\n", path)
+	}
+	fmt.Println()
+	fmt.Println("⚠ This only protects future commits — the old plaintext is still")
+	fmt.Println("  recoverable from git history. If this repo has been pushed anywhere,")
+	fmt.Println("  change the underlying secret(s), not just the file.")
+	return nil
+}
+
 // ensureGitattributes creates an empty .gitattributes file at bootstrap if
 // one doesn't exist yet, so it's ready to commit even before any rule is
 // added.

cmd/nebel/add_existing_files_test.go

@@ -0,0 +1,175 @@
+// Copyright (C) 2026 Marwin Moellers
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// commitTrackedAndSettle writes content at repo-relative relPath, backdates
+// its mtime well into the past, and commits it — reproducing a file that
+// was already tracked and settled in git's index long before any nebel
+// rule ever mentioned it.
+//
+// The backdating matters: without it, the file's mtime sits close enough
+// to "now" that git's own protection against racy stat comparisons (a
+// path whose mtime isn't safely older than the index's own mtime can't be
+// trusted from cached stat info alone) forces a full content re-check on
+// every subsequent git operation anyway — which would mask exactly the
+// bug these tests exist to catch. Once a path's stat info is unambiguously
+// old, git trusts it blindly and skips re-invoking any filter for that
+// path — a new .gitattributes line notwithstanding — until something
+// actually touches the file again.
+func commitTrackedAndSettle(t *testing.T, repo, pathEnv, relPath string, content []byte) {
+	t.Helper()
+	full := filepath.Join(repo, relPath)
+	if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(full, content, 0o644); err != nil {
+		t.Fatal(err)
+	}
+	old := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+	if err := os.Chtimes(full, old, old); err != nil {
+		t.Fatal(err)
+	}
+	runIn(t, repo, pathEnv, "git", "add", relPath)
+	runIn(t, repo, pathEnv, "git", "commit", "-q", "-m", "commit "+relPath+" before any nebel rule exists")
+}
+
+// `add file` used to only register a rule: git's own index would keep
+// treating an already-tracked, already-settled file as clean, and it
+// would stay plaintext in the repository until something unrelated later
+// edited it — contradicting the documented "no further step" promise.
+func TestAddFileEncryptsAlreadyTrackedUnchangedFile(t *testing.T) {
+	pathEnv := pathEnvWithBin(t)
+	repo := newTestRepo(t, pathEnv)
+	initWithPassword(t, repo, pathEnv, "test-password-123")
+
+	plaintext := []byte("-----BEGIN KEY-----\ntotally-secret\n-----END KEY-----\n")
+	commitTrackedAndSettle(t, repo, pathEnv, "secrets/prod.pem", plaintext)
+
+	out := runIn(t, repo, pathEnv, "nebel", "add", "file", "secrets/*.pem")
+	if !strings.Contains(out, "Encrypted and staged 1 file") {
+		t.Errorf("add file did not report re-encrypting the already-tracked file:\n%s", out)
+	}
+
+	staged := runIn(t, repo, pathEnv, "git", "show", ":secrets/prod.pem")
+	if !strings.HasPrefix(staged, "ENC[") {
+		t.Fatalf("already-tracked file was not encrypted by `add file` alone (no manual git add/renormalize ran): got %q", staged)
+	}
+
+	// Re-running is a no-op: nothing left to re-encrypt.
+	out = runIn(t, repo, pathEnv, "nebel", "add", "file", "secrets/*.pem")
+	if strings.Contains(out, "Encrypted") {
+		t.Errorf("re-running add file re-encrypted an already-converged file:\n%s", out)
+	}
+
+	runIn(t, repo, pathEnv, "git", "add", ".")
+	runIn(t, repo, pathEnv, "git", "commit", "-q", "-m", "commit the rule and its ciphertext")
+	onDisk, err := os.ReadFile(filepath.Join(repo, "secrets", "prod.pem"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(onDisk) != string(plaintext) {
+		t.Errorf("working tree changed unexpectedly: got %q, want %q", onDisk, plaintext)
+	}
+}
+
+// A glob that doesn't yet match any tracked file is the common case for a
+// brand new rule — `add file` must not error just because there's nothing
+// to renormalize yet (a raw `git add --renormalize -- <glob>` does error
+// in this case).
+func TestAddFileWithNoExistingMatchesSucceeds(t *testing.T) {
+	pathEnv := pathEnvWithBin(t)
+	repo := newTestRepo(t, pathEnv)
+	initWithPassword(t, repo, pathEnv, "test-password-123")
+
+	out := runIn(t, repo, pathEnv, "nebel", "add", "file", "secrets/*.pem")
+	if !strings.Contains(out, "Added rule") {
+		t.Errorf("add file with no matching tracked files did not succeed:\n%s", out)
+	}
+}
+
+// Same bug, mode: value: `add field` on a file that already exists,
+// tracked and unchanged, must encrypt the named field immediately.
+func TestAddFieldEncryptsAlreadyTrackedUnchangedFile(t *testing.T) {
+	pathEnv := pathEnvWithBin(t)
+	repo := newTestRepo(t, pathEnv)
+	initWithPassword(t, repo, pathEnv, "test-password-123")
+
+	commitTrackedAndSettle(t, repo, pathEnv, "config/staging.yaml", []byte(stagingYAML))
+
+	out := runIn(t, repo, pathEnv, "nebel", "add", "field", "config/staging.yaml", "database.password")
+	if !strings.Contains(out, "Encrypted and staged 1 file") {
+		t.Errorf("add field did not report re-encrypting the already-tracked file:\n%s", out)
+	}
+
+	staged := runIn(t, repo, pathEnv, "git", "show", ":config/staging.yaml")
+	if strings.Contains(staged, "s3cr3t") {
+		t.Errorf("already-tracked file still has the plaintext secret staged:\n%s", staged)
+	}
+	if !strings.Contains(staged, "ENC[") {
+		t.Errorf("field was not encrypted:\n%s", staged)
+	}
+}
+
+// Appending a second field to a rule that already applies to an
+// already-tracked, already-converged file must encrypt the newly added
+// field too, without disturbing the field the rule already covered.
+func TestAddFieldAppendEncryptsNewFieldOnAlreadyTrackedFile(t *testing.T) {
+	pathEnv := pathEnvWithBin(t)
+	repo := newTestRepo(t, pathEnv)
+	initWithPassword(t, repo, pathEnv, "test-password-123")
+
+	commitTrackedAndSettle(t, repo, pathEnv, "config/staging.yaml", []byte(stagingYAML))
+	runIn(t, repo, pathEnv, "nebel", "add", "field", "config/staging.yaml", "database.password")
+	runIn(t, repo, pathEnv, "git", "add", ".")
+	runIn(t, repo, pathEnv, "git", "commit", "-q", "-m", "encrypt password field")
+
+	out := runIn(t, repo, pathEnv, "nebel", "add", "field", "config/staging.yaml", "api.keys[0]")
+	if !strings.Contains(out, "Encrypted and staged 1 file") {
+		t.Errorf("appending a field to an existing rule did not re-encrypt the already-tracked file:\n%s", out)
+	}
+
+	staged := runIn(t, repo, pathEnv, "git", "show", ":config/staging.yaml")
+	if strings.Contains(staged, "alpha") {
+		t.Errorf("newly added field is still plaintext:\n%s", staged)
+	}
+	if strings.Contains(staged, "s3cr3t") {
+		t.Errorf("previously encrypted field regressed to plaintext:\n%s", staged)
+	}
+	if strings.Count(staged, "ENC[") != 2 {
+		t.Errorf("want exactly 2 encrypted fields staged, got:\n%s", staged)
+	}
+}
+
+// A non-ASCII filename must not be silently dropped from renormalize's
+// scope: git's default output quotes it as a C-style escaped string
+// ("secrets/zug\303\244nge.pem" for an actual "ü"), which would never
+// match the same path returned by cfg.MatchRule if that quoting weren't
+// undone (gitutil's ls-files/ls-files -s calls use -z for exactly this
+// reason).
+func TestAddFileEncryptsAlreadyTrackedFileWithNonASCIIName(t *testing.T) {
+	pathEnv := pathEnvWithBin(t)
+	repo := newTestRepo(t, pathEnv)
+	initWithPassword(t, repo, pathEnv, "test-password-123")
+
+	plaintext := []byte("-----BEGIN KEY-----\ntotally-secret\n-----END KEY-----\n")
+	commitTrackedAndSettle(t, repo, pathEnv, "secrets/zugänge.pem", plaintext)
+
+	out := runIn(t, repo, pathEnv, "nebel", "add", "file", "secrets/*.pem")
+	if !strings.Contains(out, "Encrypted and staged 1 file") {
+		t.Errorf("add file did not report re-encrypting the already-tracked non-ASCII-named file:\n%s", out)
+	}
+
+	staged := runIn(t, repo, pathEnv, "git", "show", ":secrets/zugänge.pem")
+	if !strings.HasPrefix(staged, "ENC[") {
+		t.Fatalf("already-tracked non-ASCII-named file was not encrypted by `add file`: got %q", staged)
+	}
+}