P05 type bound value tags

cmd/nebel/add.go

@@ -218,7 +218,7 @@ func ensureGitattributes(root string) error {
 // glob to the nebel filter, unless that exact line is already present.
 func addGitattributesPattern(root, glob string) error {
 	path := filepath.Join(root, gitattributesName)
-	line := fmt.Sprintf("%s filter=nebel", glob)
+	line := fmt.Sprintf("%s filter=nebel", escapeGitattributesPattern(glob))
 
 	existing, err := os.ReadFile(path)
 	if err != nil && !os.IsNotExist(err) {
@@ -241,3 +241,25 @@ func addGitattributesPattern(root, glob string) error {
 	}
 	return nil
 }
+
+// escapeGitattributesPattern escapes the characters gitattributes' line
+// syntax gives special meaning to, so a glob written verbatim into a
+// pattern/attribute line still means what the caller intended:
+//   - a leading '#' starts a comment, and a leading '!' negates a
+//     pattern — either would silently turn the whole line into something
+//     other than a filter assignment for glob;
+//   - the line's fields are split on whitespace, so an embedded space
+//     would attach the rest of glob to the "filter=nebel" attribute
+//     instead of the pattern, leaving the intended file unmatched even
+//     though the command reports success.
+//
+// This does not touch glob metacharacters (*, ?, [, ], \) themselves —
+// Rule.Files is intentionally a doublestar glob (see its doc comment),
+// and escaping those would change what the pattern matches instead of
+// merely how the line is parsed.
+func escapeGitattributesPattern(glob string) string {
+	if strings.HasPrefix(glob, "#") || strings.HasPrefix(glob, "!") {
+		glob = `\` + glob
+	}
+	return strings.ReplaceAll(glob, " ", `\ `)
+}

cmd/nebel/add_test.go

@@ -0,0 +1,33 @@
+// Copyright (C) 2026 Marwin Moellers
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import "testing"
+
+// Regression test for the 2026-09-29 audit's P07 sub-issue 2: a glob
+// written unescaped into a .gitattributes line breaks on the characters
+// that syntax gives special meaning to, leaving the intended file with no
+// effective filter assignment even though the add command reports success.
+func TestEscapeGitattributesPattern(t *testing.T) {
+	tests := []struct {
+		name string
+		glob string
+		want string
+	}{
+		{"no special characters", "secrets/*.pem", "secrets/*.pem"},
+		{"embedded space", "My File.env", `My\ File.env`},
+		{"multiple spaces", "a b c.env", `a\ b\ c.env`},
+		{"leading hash", "#backup.env", `\#backup.env`},
+		{"leading bang", "!important.env", `\!important.env`},
+		{"glob metacharacters untouched", "**/*.env", "**/*.env"},
+		{"bracket class untouched", "secrets/[ab].pem", "secrets/[ab].pem"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := escapeGitattributesPattern(tt.glob); got != tt.want {
+				t.Errorf("escapeGitattributesPattern(%q) = %q, want %q", tt.glob, got, tt.want)
+			}
+		})
+	}
+}

cmd/nebel/e2e_test.go

@@ -10,6 +10,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"sync"
 	"testing"
 )
@@ -92,6 +93,37 @@ func runInExpectingError(t *testing.T, dir, pathEnv, name string, args ...string
 	return string(out), err
 }
 
+// registeredKeyValue returns the raw stored value (a base64-encoded key)
+// for version in dir's local key storage, treating nebel as a black box
+// rather than assuming which of its two storage locations actually holds
+// it: the key file it writes to as of the 2026-09-29 audit's P06 fix
+// (internal/localkey's keyFileName), checked first, falling back to the
+// legacy git-config location a version could still be registered under —
+// mirroring internal/localkey.Get's own precedence, so these tests keep
+// working regardless of which one a given key landed in.
+func registeredKeyValue(t *testing.T, dir, pathEnv string, version int) (string, bool) {
+	t.Helper()
+	commonDir := strings.TrimSpace(runIn(t, dir, pathEnv, "git", "rev-parse", "--git-common-dir"))
+	if data, err := os.ReadFile(filepath.Join(dir, commonDir, "nebel-keys")); err == nil {
+		prefix := fmt.Sprintf("%d=", version)
+		for _, line := range strings.Split(string(data), "\n") {
+			if val, ok := strings.CutPrefix(line, prefix); ok {
+				return val, true
+			}
+		}
+	}
+
+	name := "filter.nebel.key"
+	if version != 1 {
+		name = fmt.Sprintf("filter.nebel.key%d", version)
+	}
+	val, ok, err := getLocalConfig(t, dir, pathEnv, name)
+	if err != nil {
+		return "", false
+	}
+	return val, ok
+}
+
 // initWithPassword runs `nebel init`, supplying the password through
 // the environment — the command line no longer accepts one.
 func initWithPassword(t *testing.T, dir, pathEnv, password string) {
@@ -233,14 +265,14 @@ func TestJoinWithWrongPasswordFails(t *testing.T) {
 	repo := newTestRepo(t, pathEnv)
 	initWithPassword(t, repo, pathEnv, "correct-password")
 
-	keyBefore := runIn(t, repo, pathEnv, "git", "config", "--local", "--get", "filter.nebel.key")
+	keyBefore, _ := registeredKeyValue(t, repo, pathEnv, 1)
 
 	out, err := runInitWith(t, repo, pathEnv, []string{passwordEnv + "=wrong-password"}, "")
 	if err == nil {
 		t.Fatalf("nebel init with wrong password: want an error, got success: %s", out)
 	}
 
-	keyAfter := runIn(t, repo, pathEnv, "git", "config", "--local", "--get", "filter.nebel.key")
+	keyAfter, _ := registeredKeyValue(t, repo, pathEnv, 1)
 	if keyBefore != keyAfter {
 		t.Error("wrong password changed the registered local key")
 	}

cmd/nebel/filtermatch_e2e_test.go

@@ -0,0 +1,73 @@
+// Copyright (C) 2026 Marwin Moellers
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Regression test for the 2026-09-29 audit's P07(1): a bare glob like
+// "*.env" (no leading "**/") matches recursively in .gitattributes syntax
+// but, per doublestar's semantics, only at the depth it's written at (see
+// config.Rule.Files's doc comment). `nebel add file` writes such a pattern
+// unchanged into both files, so a file living in a subdirectory is one
+// git invokes the filter for but that config.MatchRule does not recognize.
+//
+// Before the fix, Clean silently returned this content unchanged and
+// `git commit` succeeded with the secret stored as plaintext. The filter
+// must instead fail the commit outright (filterop.ErrNoMatchingRule),
+// since filter.nebel.required is set to true.
+func TestAddFileBareGlobDoesNotSilentlyCommitPlaintextInSubdirectory(t *testing.T) {
+	pathEnv := pathEnvWithBin(t)
+	repo := newTestRepo(t, pathEnv)
+
+	initWithPassword(t, repo, pathEnv, "test-password-123")
+	runIn(t, repo, pathEnv, "nebel", "add", "file", "*.env")
+
+	configDir := filepath.Join(repo, "config")
+	if err := os.MkdirAll(configDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	plaintext := "DB_PASSWORD=super-secret\n"
+	envPath := filepath.Join(configDir, "prod.env")
+	if err := os.WriteFile(envPath, []byte(plaintext), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	out, err := runInExpectingError(t, repo, pathEnv, "git", "add", "--", "config/prod.env")
+	if err == nil {
+		t.Fatalf("git add config/prod.env: want an error (glob/attribute mismatch), got success:\n%s", out)
+	}
+	if !strings.Contains(out, "no rule") && !strings.Contains(out, "ErrNoMatchingRule") && !strings.Contains(out, "filterop:") {
+		t.Errorf("git add error output = %q, want it to mention the rule mismatch", out)
+	}
+
+	// Confirm the secret was never staged at all, plaintext or otherwise —
+	// a failed clean must not leave a partial index entry behind.
+	lsFiles := runIn(t, repo, pathEnv, "git", "status", "--porcelain")
+	if strings.Contains(lsFiles, "A  config/prod.env") {
+		t.Errorf("git status shows config/prod.env staged despite the filter error:\n%s", lsFiles)
+	}
+
+	// The root-level case (matching depth) must still work exactly as
+	// before — this is a mismatch fix, not a semantics change.
+	rootPlaintext := "DB_PASSWORD=another-secret\n"
+	rootEnvPath := filepath.Join(repo, "root.env")
+	if err := os.WriteFile(rootEnvPath, []byte(rootPlaintext), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	runIn(t, repo, pathEnv, "git", "add", "--", "root.env")
+	runIn(t, repo, pathEnv, "git", "commit", "-q", "-m", "add root-level env file")
+
+	stored := runIn(t, repo, pathEnv, "git", "show", "HEAD:root.env")
+	if stored == rootPlaintext {
+		t.Error("root-level file matching the same bare glob was committed as plaintext")
+	}
+	if !strings.HasPrefix(stored, "ENC[") {
+		t.Errorf("root-level file's committed blob is not ciphertext: %q", stored)
+	}
+}

cmd/nebel/init_version_e2e_test.go

@@ -87,8 +87,8 @@ func TestInitVersionRecoversContentFromBeforeRotation(t *testing.T) {
 	}
 
 	// The v2 key registered earlier must still be there.
-	if _, err := runInExpectingError(t, clone, pathEnv, "git", "config", "--local", "--get", "filter.nebel.key2"); err != nil {
-		t.Errorf("init --version 1 disturbed the already-registered v2 key: %v", err)
+	if _, ok := registeredKeyValue(t, clone, pathEnv, 2); !ok {
+		t.Error("init --version 1 disturbed the already-registered v2 key")
 	}
 }
 
@@ -104,7 +104,7 @@ func TestInitVersionUnknownFails(t *testing.T) {
 	if err == nil {
 		t.Fatalf("nebel init --version 99: want an error, got success:\n%s", out)
 	}
-	if _, ok, kerr := getLocalConfig(t, clone, pathEnv, "filter.nebel.key99"); kerr == nil && ok {
+	if _, ok := registeredKeyValue(t, clone, pathEnv, 99); ok {
 		t.Error("init --version 99 registered a key for a version that doesn't exist")
 	}
 }

cmd/nebel/legacytag_e2e_test.go

@@ -0,0 +1,124 @@
+// Copyright (C) 2026 Marwin Moellers
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"encoding/base64"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/MuffinUser/nebel/internal/siv"
+	"github.com/MuffinUser/nebel/internal/tag"
+)
+
+// Regression test for an interaction between Package D (P01's dirty-check
+// in gitutil.CheckoutAll) and Package F (P05's type-authenticated value
+// tags), found during review rather than by the original audit:
+// DirtyFiles compares by running the clean filter (that's what `git diff
+// HEAD` against a filtered path does), and a legacy (pre-AC-3.10) value
+// tag re-encrypts to different bytes the moment anything cleans it again
+// — the AAD it's produced under changed — even when the secret itself was
+// never touched. That makes a repeated `nebel init` refuse with
+// ErrDirtyManagedFiles for a file the user never edited, until the legacy
+// tag is upgraded and *committed* (renormalizing alone updates the index,
+// not HEAD, so `git diff HEAD` still sees the old committed form either
+// way — confirmed here, not merely asserted).
+func TestInitRefusesOnLegacyValueTagUntilRenormalizedAndCommitted(t *testing.T) {
+	pathEnv := pathEnvWithBin(t)
+	repo := setupValueRepo(t, pathEnv)
+	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 database.password")
+
+	committed := runIn(t, repo, pathEnv, "git", "show", "HEAD:config/staging.yaml")
+	if !strings.Contains(committed, "AES256_SIV_TB,") {
+		t.Fatalf("test setup did not produce a type-bound tag:\n%s", committed)
+	}
+
+	// Downgrade the committed tag to the legacy (pre-AC-3.10) form,
+	// re-encrypting the real secret under the legacy (type-unbound) AAD
+	// with this repo's actual registered key — reproducing exactly what a
+	// pre-Package-F build would have committed, not a synthetic stand-in.
+	keyB64, ok := registeredKeyValue(t, repo, pathEnv, 1)
+	if !ok {
+		t.Fatal("no key registered for version 1")
+	}
+	key, err := base64.StdEncoding.DecodeString(keyB64)
+	if err != nil {
+		t.Fatalf("decoding registered key: %v", err)
+	}
+	legacyAAD := siv.AAD(siv.ModeValue, "config/staging.yaml", "database.password")
+	ciphertext, err := siv.Encrypt(key, []byte("s3cr3t"), legacyAAD)
+	if err != nil {
+		t.Fatalf("siv.Encrypt: %v", err)
+	}
+	legacyTag := fmt.Sprintf("ENC[%s,key:1,data:%s,type:str]", tag.AlgoAES256SIV, base64.StdEncoding.EncodeToString(ciphertext))
+	legacyBlob := strings.Replace(stagingYAML, `password: "s3cr3t"`, `password: "`+legacyTag+`"`, 1)
+
+	cfgPath := filepath.Join(repo, "config", "staging.yaml")
+	if err := os.WriteFile(cfgPath, []byte(legacyBlob), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	// -c filter.nebel.clean=cat: commit the legacy blob's bytes exactly as
+	// written, the same technique TestPreRotationRepoMigration uses to
+	// simulate a commit predating a tag-format fix.
+	runIn(t, repo, pathEnv, "git", "-c", "filter.nebel.clean=cat", "add", "-A")
+	runIn(t, repo, pathEnv, "git", "-c", "filter.nebel.clean=cat", "commit", "-q", "-m", "simulate a pre-AC-3.10 commit")
+
+	// First init after the simulated legacy commit: the working tree
+	// currently holds that same legacy-tagged text verbatim (nothing
+	// cleaned it yet), so it's not "dirty" relative to HEAD — this must
+	// still succeed, and it's what actually decrypts the field into the
+	// working tree for the first time.
+	initWithPassword(t, repo, pathEnv, "value-mode-password")
+	onDisk, err := os.ReadFile(cfgPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(onDisk), `password: "s3cr3t"`) {
+		t.Fatalf("first init did not decrypt the legacy tag:\n%s", onDisk)
+	}
+
+	// Second init, simulating "run nebel init again" with no edits in
+	// between: the working tree is now plaintext, and re-cleaning it
+	// produces a fresh type-bound tag that differs from HEAD's still-legacy
+	// one — DirtyFiles sees that as a real difference and refuses, exactly
+	// the interaction this test is about.
+	out, err := runInitWithExpectingError(t, repo, pathEnv, "value-mode-password")
+	if err == nil {
+		t.Fatalf("second init: want ErrDirtyManagedFiles (legacy tag re-encrypts differently), got success:\n%s", out)
+	}
+	if !strings.Contains(out, "renormalize") {
+		t.Errorf("init error does not point at the documented fix:\n%s", out)
+	}
+
+	// The fix as actually documented: renormalize AND commit. Renormalize
+	// alone only updates the index, not HEAD — `git diff HEAD` (what
+	// DirtyFiles runs) would still see the old committed form either way,
+	// which this confirms rather than assumes.
+	runIn(t, repo, pathEnv, "git", "add", "--renormalize", "--", "config/staging.yaml")
+	if _, err := runInitWithExpectingError(t, repo, pathEnv, "value-mode-password"); err == nil {
+		t.Fatal("init after renormalizing but before committing: want it to still refuse, got success")
+	}
+	runIn(t, repo, pathEnv, "git", "commit", "-q", "-m", "upgrade database.password to a type-bound tag")
+
+	initWithPassword(t, repo, pathEnv, "value-mode-password")
+	afterUpgrade, err := os.ReadFile(cfgPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(afterUpgrade), `password: "s3cr3t"`) {
+		t.Errorf("init after the documented fix did not leave the field decrypted:\n%s", afterUpgrade)
+	}
+}
+
+// runInitWithExpectingError runs `nebel init` with password, expecting it
+// to fail, and returns its combined output for inspecting the error text.
+func runInitWithExpectingError(t *testing.T, dir, pathEnv, password string) (string, error) {
+	t.Helper()
+	return runInitWith(t, dir, pathEnv, []string{passwordEnv + "=" + password}, "")
+}

cmd/nebel/password_e2e_test.go

@@ -114,7 +114,7 @@ func TestInitRejectsPasswordArgument(t *testing.T) {
 	if !strings.Contains(out, "process list") {
 		t.Errorf("error does not explain the exposure:\n%s", out)
 	}
-	if _, err := runInExpectingError(t, clone, pathEnv, "git", "config", "--local", "--get", "filter.nebel.key"); err == nil {
+	if _, ok := registeredKeyValue(t, clone, pathEnv, 1); ok {
 		t.Error("a refused init registered a local key")
 	}
 }
@@ -138,7 +138,7 @@ func TestInitWithoutAnyPasswordSourceFails(t *testing.T) {
 	if _, err := os.Stat(filepath.Join(clone, "secrets", "prod.pem")); err != nil {
 		t.Fatal(err)
 	}
-	if _, err := runInExpectingError(t, clone, pathEnv, "git", "config", "--local", "--get", "filter.nebel.key"); err == nil {
+	if _, ok := registeredKeyValue(t, clone, pathEnv, 1); ok {
 		t.Error("a failed init registered a local key")
 	}
 }

cmd/nebel/rotate_e2e_test.go

@@ -176,19 +176,19 @@ func TestRotateKeepsOldKeyRegistered(t *testing.T) {
 	pathEnv := pathEnvWithBin(t)
 	origin, plaintext := setupEncryptedOrigin(t, pathEnv, "origin-password")
 
-	keyV1Before := runIn(t, origin, pathEnv, "git", "config", "--local", "--get", "filter.nebel.key")
+	keyV1Before, _ := registeredKeyValue(t, origin, pathEnv, 1)
 
 	out, err := runRotateWith(t, origin, pathEnv, []string{passwordEnv + "=rotated-password"}, "")
 	if err != nil {
 		t.Fatalf("nebel rotate: %v\n%s", err, out)
 	}
 
-	keyV1After := runIn(t, origin, pathEnv, "git", "config", "--local", "--get", "filter.nebel.key")
+	keyV1After, _ := registeredKeyValue(t, origin, pathEnv, 1)
 	if keyV1Before != keyV1After {
 		t.Error("rotate disturbed the existing version's registered key")
 	}
-	if _, err := runInExpectingError(t, origin, pathEnv, "git", "config", "--local", "--get", "filter.nebel.key2"); err != nil {
-		t.Errorf("rotate did not register the new version's key: %v", err)
+	if _, ok := registeredKeyValue(t, origin, pathEnv, 2); !ok {
+		t.Error("rotate did not register the new version's key")
 	}
 
 	// The pre-rotation content, still tagged version 1, must still decrypt.