From 5117c620e67c7e4b04ba3c91a19278ba42857d11 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Tue, 20 Oct 2026 08:56:10 -0400 Subject: [PATCH 1/5] fix(config): identify secret overrides and refuse CI generation (D-0708) --- cmd/commands/bundle_test.go | 3 ++ cmd/commands/canon_engine_test.go | 1 + cmd/commands/ci_bootstrap.go | 2 +- cmd/commands/start_containers.go | 2 +- cmd/commands/surface_identity_test.go | 2 +- internal/build/manifest_test.go | 3 ++ internal/build/orchestrator_secrets.go | 3 ++ .../build/plugins_entitlement_mount_test.go | 8 ++++ internal/build/secrets_ci_test.go | 37 +++++++++++++++++++ internal/config/secrets_hint_test.go | 19 ++++++++++ .../config/validator_placeholder_secrets.go | 6 +-- internal/reconcile/main_test.go | 13 +++++++ 12 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 internal/build/secrets_ci_test.go create mode 100644 internal/config/secrets_hint_test.go create mode 100644 internal/reconcile/main_test.go diff --git a/cmd/commands/bundle_test.go b/cmd/commands/bundle_test.go index fb146502f..9ed67a0c6 100644 --- a/cmd/commands/bundle_test.go +++ b/cmd/commands/bundle_test.go @@ -32,6 +32,9 @@ const fixtureBundlesJSON = `{ }` func TestMain(m *testing.M) { + // Fixtures model local projects; CI-specific tests opt in explicitly. + _ = os.Unsetenv("CI") + _ = os.Unsetenv("NSELF_CI") srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte(fixtureBundlesJSON)) })) diff --git a/cmd/commands/canon_engine_test.go b/cmd/commands/canon_engine_test.go index 0ad0b8a0b..c8598a9ee 100644 --- a/cmd/commands/canon_engine_test.go +++ b/cmd/commands/canon_engine_test.go @@ -771,6 +771,7 @@ func TestCanonEngineSafety(t *testing.T) { // TestCanonEnginePrepare drives the whole pre-dispatch path on the fixture tree: // argv rewrite, tree relocation, destination check, warning, hub preflight. func TestCanonEnginePrepare(t *testing.T) { + t.Setenv("NSELF_PLUGIN_DIR", t.TempDir()) tb := fixtureTable(t) run := func(root *cobra.Command, v15, mount bool, argv ...string) ([]string, string, error) { t.Helper() diff --git a/cmd/commands/ci_bootstrap.go b/cmd/commands/ci_bootstrap.go index 34eb2b9f2..7aa57e27f 100644 --- a/cmd/commands/ci_bootstrap.go +++ b/cmd/commands/ci_bootstrap.go @@ -141,7 +141,7 @@ func retireLegacyCICache() { _ = os.Remove(legacy) } } - fmt.Fprintln(ciNoticeOut, "notice: ~/.nself/bin/nself-ci is no longer used (unverified); nself now installs the signed ci plugin") + _, _ = fmt.Fprintln(ciNoticeOut, "notice: ~/.nself/bin/nself-ci is no longer used (unverified); nself now installs the signed ci plugin") }) } diff --git a/cmd/commands/start_containers.go b/cmd/commands/start_containers.go index 039faedd9..1e6b42cd5 100644 --- a/cmd/commands/start_containers.go +++ b/cmd/commands/start_containers.go @@ -246,7 +246,7 @@ func initializeDatabasePhase(ctx context.Context, cfg *config.Config, opts start // misdirect for this case, so name the real problem instead. hints := []string{ "Check PostgreSQL logs: nself logs postgres", - "Verify POSTGRES_USER and POSTGRES_PASSWORD in .env", + "Verify POSTGRES_USER in .env and POSTGRES_PASSWORD in .env.secrets (which overrides .env)", "Try restarting: nself restart", } if errors.Is(err, database.ErrInvalidIdentifier) { diff --git a/cmd/commands/surface_identity_test.go b/cmd/commands/surface_identity_test.go index 0a4fea984..2a1d079d9 100644 --- a/cmd/commands/surface_identity_test.go +++ b/cmd/commands/surface_identity_test.go @@ -194,7 +194,7 @@ func (e *surfaceEnv) resetProject(t *testing.T) { if envName == "" { envName = "dev" } - if err := os.WriteFile(filepath.Join(e.proj, ".env"), []byte("PROJECT_NAME=fixture\nENV="+envName+"\nMINIO_ENABLED=false\n"), 0o600); err != nil { + if err := os.WriteFile(filepath.Join(e.proj, ".env"), []byte("PROJECT_NAME=fixture\nENV="+envName+"\nMINIO_ENABLED=false\nPOSTGRES_PASSWORD=actuallyuniquevalueforthisenvironmentnumberone\nHASURA_GRAPHQL_ADMIN_SECRET=totallydifferentvaluealtogetherforthisservice\n"), 0o600); err != nil { t.Fatal(err) } } diff --git a/internal/build/manifest_test.go b/internal/build/manifest_test.go index 1c91207e0..51410b26b 100644 --- a/internal/build/manifest_test.go +++ b/internal/build/manifest_test.go @@ -30,6 +30,9 @@ const fixtureBundlesJSON = `{ }` func TestMain(m *testing.M) { + // Fixtures model local projects; CI-specific tests opt in explicitly. + _ = os.Unsetenv("CI") + _ = os.Unsetenv("NSELF_CI") if err := bundle.LoadBytes([]byte(fixtureBundlesJSON)); err != nil { panic("seeding bundle fixture: " + err.Error()) } diff --git a/internal/build/orchestrator_secrets.go b/internal/build/orchestrator_secrets.go index 3bcbb48db..810a71291 100644 --- a/internal/build/orchestrator_secrets.go +++ b/internal/build/orchestrator_secrets.go @@ -91,6 +91,9 @@ func persistGeneratedSecretsFx(workdir string, cfg *config.Config, fx Effects, s } continue } + if os.Getenv("NSELF_CI") != "" || os.Getenv("CI") != "" { + return fmt.Errorf("%s is unset in CI: export it for every build or define it in .env.secrets; refusing to persist an auto-generated value that would shadow later exports", c.key) + } toWrite = append(toWrite, secretEntry{c.key, *c.dst}) } diff --git a/internal/build/plugins_entitlement_mount_test.go b/internal/build/plugins_entitlement_mount_test.go index 1bd6a9156..7382fbdd0 100644 --- a/internal/build/plugins_entitlement_mount_test.go +++ b/internal/build/plugins_entitlement_mount_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/hex" "errors" + "fmt" "os" "path/filepath" "regexp" @@ -89,6 +90,13 @@ func TestMountIgnoresOverride(t *testing.T) { } func TestMountEmptyFileE125(t *testing.T) { + // An installed host licence must not turn this empty-cache fixture into + // an online renewal test. + t.Setenv("HOME", t.TempDir()) + t.Setenv("NSELF_PLUGIN_LICENSE_KEY", "") + for i := 1; i <= 10; i++ { + t.Setenv(fmt.Sprintf("NSELF_LICENSE_KEY_%d", i), "") + } compattest.Set(t, true) t.Setenv("LICENSE_CACHE_PATH", filepath.Join(t.TempDir(), "missing")) t.Setenv("LICENSE_PUBLIC_KEY_OVERRIDE", "") diff --git a/internal/build/secrets_ci_test.go b/internal/build/secrets_ci_test.go new file mode 100644 index 000000000..c76014c57 --- /dev/null +++ b/internal/build/secrets_ci_test.go @@ -0,0 +1,37 @@ +package build + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/nself-org/cli/internal/config" +) + +func TestCIMissingCoreSecretRefusesPersistence(t *testing.T) { + for _, flag := range []string{"CI", "NSELF_CI"} { + for _, key := range []string{"POSTGRES_PASSWORD", "HASURA_GRAPHQL_ADMIN_SECRET"} { + t.Run(flag+"/"+key, func(t *testing.T) { + clearSecretEnv(t) + t.Setenv("CI", "") + t.Setenv("NSELF_CI", "") + t.Setenv(flag, "1") + cfg := &config.Config{Env: "dev"} + if key == "POSTGRES_PASSWORD" { + cfg.Postgres.Password = "fixture-generated" + } else { + cfg.Hasura.AdminSecret = "fixture-generated" + } + dir := t.TempDir() + err := persistGeneratedSecrets(dir, cfg) + if err == nil || !strings.Contains(err.Error(), key) { + t.Fatalf("want missing-key refusal, got %v", err) + } + if _, err := os.Stat(filepath.Join(dir, ".env.secrets")); !os.IsNotExist(err) { + t.Fatal("CI refusal wrote secrets") + } + }) + } + } +} diff --git a/internal/config/secrets_hint_test.go b/internal/config/secrets_hint_test.go new file mode 100644 index 000000000..647d79938 --- /dev/null +++ b/internal/config/secrets_hint_test.go @@ -0,0 +1,19 @@ +package config + +import ( + "strings" + "testing" +) + +func TestPlaceholderHintNamesSecretsFile(t *testing.T) { + for _, value := range []string{"", "changeme"} { + t.Run(value, func(t *testing.T) { + cfg := validPlaceholderTestConfig("prod") + cfg.Postgres.Password = value + err := validateNoPlaceholderSecrets(cfg) + if err == nil || !strings.Contains(err.Error(), ".env.secrets") { + t.Fatalf("hint must identify winning file: %v", err) + } + }) + } +} diff --git a/internal/config/validator_placeholder_secrets.go b/internal/config/validator_placeholder_secrets.go index fdb61656a..b6191dc46 100644 --- a/internal/config/validator_placeholder_secrets.go +++ b/internal/config/validator_placeholder_secrets.go @@ -74,11 +74,11 @@ func validateNoPlaceholderSecrets(cfg *Config) error { } for _, f := range required { if f.value == "" { - return fmt.Errorf("%s is empty in %s environment — set a unique value in .env: %w", + return fmt.Errorf("%s is empty in %s environment — set a unique value in .env.secrets (which overrides .env): %w", f.name, cfg.Env, errs.ErrPlaceholderSecret) } if isPlaceholderSecret(f.value) { - return fmt.Errorf("%s is a known placeholder value in %s environment — set a unique value in .env: %w", + return fmt.Errorf("%s is a known placeholder value in %s environment — set a unique value in .env.secrets (which overrides .env): %w", f.name, cfg.Env, errs.ErrPlaceholderSecret) } } @@ -103,7 +103,7 @@ func validateNoPlaceholderSecrets(cfg *Config) error { } for _, f := range optional { if isPlaceholderSecret(f.value) { - return fmt.Errorf("%s is a known placeholder value in %s environment — set a unique value in .env: %w", + return fmt.Errorf("%s is a known placeholder value in %s environment — set a unique value in .env.secrets (which overrides .env): %w", f.name, cfg.Env, errs.ErrPlaceholderSecret) } } diff --git a/internal/reconcile/main_test.go b/internal/reconcile/main_test.go new file mode 100644 index 000000000..1626f6ab7 --- /dev/null +++ b/internal/reconcile/main_test.go @@ -0,0 +1,13 @@ +package reconcile + +import ( + "os" + "testing" +) + +func TestMain(m *testing.M) { + // Fixtures model local projects; CI-specific tests opt in explicitly. + _ = os.Unsetenv("CI") + _ = os.Unsetenv("NSELF_CI") + os.Exit(m.Run()) +} From 5ac80e3b12cbc1e39932f17a3269ced430cc7a8d Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Tue, 20 Oct 2026 08:56:10 -0400 Subject: [PATCH 2/5] fix(plugin): exclude staging directories from conflict scans (D-0740) --- internal/plugin/config.go | 4 +-- internal/plugin/loader.go | 2 +- internal/plugin/staging_conflict_test.go | 33 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 internal/plugin/staging_conflict_test.go diff --git a/internal/plugin/config.go b/internal/plugin/config.go index ab0fd0a15..19bd4b80f 100644 --- a/internal/plugin/config.go +++ b/internal/plugin/config.go @@ -130,7 +130,7 @@ func checkTablePrefixConflict(pluginDir, newPluginName string, newTables []strin } for _, entry := range entries { - if !entry.IsDir() { + if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") { continue } if isOwnPluginDir(entry.Name(), newPluginName) { @@ -169,7 +169,7 @@ func checkTableConflicts(pluginDir string, newPlugin *PluginManifest) error { } for _, entry := range entries { - if !entry.IsDir() || isOwnPluginDir(entry.Name(), newPlugin.Name) { + if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") || isOwnPluginDir(entry.Name(), newPlugin.Name) { continue } existing, err := parseManifest(filepath.Join(pluginDir, entry.Name(), "plugin.json")) diff --git a/internal/plugin/loader.go b/internal/plugin/loader.go index 707cca206..bf28c2148 100644 --- a/internal/plugin/loader.go +++ b/internal/plugin/loader.go @@ -251,7 +251,7 @@ func collectInstalledPluginRoutes(pluginDir, skipPlugin string) []nginx.NginxRou } for _, entry := range entries { - if !entry.IsDir() { + if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") { continue } // isOwnPluginDir also skips ".prev", the backup Update() diff --git a/internal/plugin/staging_conflict_test.go b/internal/plugin/staging_conflict_test.go new file mode 100644 index 000000000..00f490d96 --- /dev/null +++ b/internal/plugin/staging_conflict_test.go @@ -0,0 +1,33 @@ +package plugin + +import ( + "os" + "path/filepath" + "testing" +) + +func TestConflictScansIgnoreStaging(t *testing.T) { + for _, name := range []string{".staging-thirdparty-123", "installed"} { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, name) + if err := os.Mkdir(p, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(p, "plugin.json"), []byte(`{"name":"other","version":"1.0.0","description":"fixture","category":"utilities","license":"MIT","tables":["np_chat_messages"],"apiEndpoints":["chat.example.test/messages"]}`), 0644); err != nil { + t.Fatal(err) + } + wantConflict := name == "installed" + if err := checkTablePrefixConflict(dir, "chat", []string{"np_chat_messages"}); (err != nil) != wantConflict { + t.Fatalf("prefix conflict: %v", err) + } + if err := checkTableConflicts(dir, &PluginManifest{Name: "chat", Tables: []string{"np_chat_messages"}}); (err != nil) != wantConflict { + t.Fatalf("table conflict: %v", err) + } + routes := collectInstalledPluginRoutes(dir, "chat") + if got := len(routes) > len(baseServiceRoutes); got != wantConflict { + t.Fatalf("route conflict: %v", got) + } + }) + } +} From c890324d39b98d8b38aa409e384a18946c7f73df Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Tue, 20 Oct 2026 08:56:11 -0400 Subject: [PATCH 3/5] fix(images): cover legacy and valkey pins in bump policy (D-0741) --- .github/image-bump-policy.yaml | 3 +++ .github/workflows/ci.yml | 3 +++ tools/imagebump/coverage_test.go | 39 ++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 tools/imagebump/coverage_test.go diff --git a/.github/image-bump-policy.yaml b/.github/image-bump-policy.yaml index 2c070f396..01e2930be 100644 --- a/.github/image-bump-policy.yaml +++ b/.github/image-bump-policy.yaml @@ -33,14 +33,17 @@ services: meilisearch-init: {regex: '^(\d+)\.(\d+)$', track: patch} postgres: {regex: '^(\d+)-alpine$', track: patch} python: {regex: '^(\d+)\.(\d+)-slim$', track: patch} + valkey: {regex: '^(\d+)\.(\d+)-alpine$', track: patch} redis: {regex: '^(\d+)-alpine$', track: patch} # --- exact versions: patch releases within the current minor --- alertmanager: {regex: '^v(\d+)\.(\d+)\.(\d+)$', track: patch} auth: {regex: '^(\d+)\.(\d+)\.(\d+)$', track: patch} + auth-v14: {regex: '^(\d+)\.(\d+)\.(\d+)$', track: patch} cadvisor: {regex: '^v(\d+)\.(\d+)\.(\d+)$', track: patch} elasticsearch: {regex: '^(\d+)\.(\d+)\.(\d+)$', track: patch} grafana: {regex: '^(\d+)\.(\d+)\.(\d+)$', track: patch} hasura: {regex: '^v(\d+)\.(\d+)\.(\d+)$', track: patch} + hasura-v14: {regex: '^v(\d+)\.(\d+)\.(\d+)$', track: patch} lego: {regex: '^v(\d+)\.(\d+)\.(\d+)$', track: patch} loki: {regex: '^(\d+)\.(\d+)\.(\d+)$', track: patch} mlflow: {regex: '^v(\d+)\.(\d+)\.(\d+)$', track: patch} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec6a8a1f4..473ed469c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,9 @@ jobs: # tooling but GitHub rejects the whole file, reporting only "this run # likely failed because of a workflow file issue". Catch it here instead # of burning a push cycle. + - name: Image bump policy coverage + run: go test -mod=vendor ./tools/imagebump -run TestRepositoryPolicyCoverage + - name: workflow YAML lint run: bash scripts/ci/workflow-lint.sh diff --git a/tools/imagebump/coverage_test.go b/tools/imagebump/coverage_test.go new file mode 100644 index 000000000..e57bbebf0 --- /dev/null +++ b/tools/imagebump/coverage_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "os" + "testing" +) + +func TestRepositoryPolicyCoverage(t *testing.T) { + raw, err := os.ReadFile("../../internal/compose/images.yaml") + if err != nil { + t.Fatal(err) + } + lock, err := parseLockFile(raw) + if err != nil { + t.Fatal(err) + } + raw, err = os.ReadFile("../../.github/image-bump-policy.yaml") + if err != nil { + t.Fatal(err) + } + policy, err := loadPolicy(raw) + if err != nil { + t.Fatal(err) + } + for _, image := range lock.Images { + if image.Role == "plugin" { + continue + } + t.Run(image.Name, func(t *testing.T) { + rule, ok := policy.rules[image.Name] + if !ok { + t.Fatal("missing bump policy") + } + if !rule.re.MatchString(image.Version) { + t.Fatal("current version does not match policy") + } + }) + } +} From b7e5ff52cd07374660e789704cc72d3e197233a8 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Tue, 20 Oct 2026 08:56:11 -0400 Subject: [PATCH 4/5] fix(oplock): allow plan and preview under a held lock (D-0755) --- internal/oplock/oplockcmd/preview_test.go | 44 +++++++++++++++++++++++ internal/oplock/oplockcmd/wrap.go | 14 ++++++++ 2 files changed, 58 insertions(+) create mode 100644 internal/oplock/oplockcmd/preview_test.go diff --git a/internal/oplock/oplockcmd/preview_test.go b/internal/oplock/oplockcmd/preview_test.go new file mode 100644 index 000000000..7c2eaa81e --- /dev/null +++ b/internal/oplock/oplockcmd/preview_test.go @@ -0,0 +1,44 @@ +//go:build !windows + +package oplockcmd + +import ( + "context" + "testing" + + "github.com/nself-org/cli/internal/cmdregistry" + "github.com/nself-org/cli/internal/compat/compattest" + "github.com/nself-org/cli/internal/oplock" + "github.com/spf13/cobra" +) + +func TestPreviewUnderHeldLock(t *testing.T) { + for _, row := range []struct{ path, flag string }{{"build", "plan"}, {"config set", "preview"}, {"update", "plan"}} { + t.Run(row.path, func(t *testing.T) { + compattest.Set(t, true) + dir := projectDir(t) + t.Setenv(oplock.EnvToken, "") + held, err := oplock.Acquire(context.Background(), dir, oplock.Opts{Command: "holder"}) + if err != nil { + t.Fatal(err) + } + defer held.Release() + ran := false + _, cmd := newTree(write(), func(*cobra.Command) error { ran = true; return nil }) + cmd.Flags().Bool(row.flag, false, "preview") + if err := cmd.Flags().Set(row.flag, "true"); err != nil { + t.Fatal(err) + } + // The registry's path identifies the real command even in this fixture. + entry := write() + entry.Path = "nself " + row.path + cmd.RunE = Wrap(func(*cobra.Command) (*cmdregistry.Command, error) { return entry, nil }, func(*cobra.Command, []string) error { ran = true; return nil }) + if err := cmd.RunE(cmd, nil); err != nil { + t.Fatal(err) + } + if !ran { + t.Fatal("preview body did not run") + } + }) + } +} diff --git a/internal/oplock/oplockcmd/wrap.go b/internal/oplock/oplockcmd/wrap.go index 892fe5c83..4abf2db38 100644 --- a/internal/oplock/oplockcmd/wrap.go +++ b/internal/oplock/oplockcmd/wrap.go @@ -93,5 +93,19 @@ func Effective(cmd *cobra.Command, e *cmdregistry.Command) (side, out string) { out = *f.Output } } + // These flags render a plan and never apply it, even when the command's + // default registry class is write or destructive. + preview := "" + switch e.Path { + case "nself build", "nself update": + preview = "plan" + case "nself config set": + preview = "preview" + } + if preview != "" { + if f := cmd.Flags().Lookup(preview); f != nil && f.Value.Type() == "bool" && f.Value.String() == "true" { + side = canon.SideEffectRead + } + } return side, out } From 7bfa2553e98f97508c8fdfc5a099bdf06737ff18 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Tue, 20 Oct 2026 08:58:36 -0400 Subject: [PATCH 5/5] fix(reconcile): preserve unrecorded marked files (D-0756) --- internal/build/nginx_sites_prune.go | 10 ++- internal/build/nginx_sites_state.go | 30 +++++++++ internal/reconcile/apply.go | 4 +- internal/reconcile/handedit.go | 19 ++++++ internal/reconcile/planner.go | 2 +- internal/reconcile/state.go | 3 +- internal/reconcile/unrecorded_compare_test.go | 24 +++++++ internal/reconcile/unrecorded_test.go | 66 +++++++++++++++++++ 8 files changed, 153 insertions(+), 5 deletions(-) create mode 100644 internal/build/nginx_sites_state.go create mode 100644 internal/reconcile/unrecorded_compare_test.go create mode 100644 internal/reconcile/unrecorded_test.go diff --git a/internal/build/nginx_sites_prune.go b/internal/build/nginx_sites_prune.go index 5bd5eea57..3e6e9e11b 100644 --- a/internal/build/nginx_sites_prune.go +++ b/internal/build/nginx_sites_prune.go @@ -30,6 +30,7 @@ package build import ( "bytes" "fmt" + "github.com/nself-org/cli/internal/compat" "os" "path/filepath" "sort" @@ -201,13 +202,20 @@ func pruneGeneratedNginxSitesVia(sink Sink, sitesDir string) (removed int, forei return 0, nil, regErr } + var owned map[string]string + if compat.V15() { + owned, err = recordedSiteFiles(sink) + if err != nil { + return 0, nil, err + } + } const headBytes = 128 for _, e := range entries { if e.IsDir() || registered["nginx/sites/"+e.Name()] { continue } path := filepath.Join(sitesDir, e.Name()) - if isGeneratedNginxSiteFile(sink, path, headBytes) { + if isGeneratedNginxSiteFile(sink, path, headBytes) && (!compat.V15() || owned["nginx/sites/"+e.Name()] != "") { if rmErr := sink.Remove(path); rmErr != nil { return removed, foreign, fmt.Errorf("removing generated file %s: %w", path, rmErr) } diff --git a/internal/build/nginx_sites_state.go b/internal/build/nginx_sites_state.go new file mode 100644 index 000000000..e8b6172a0 --- /dev/null +++ b/internal/build/nginx_sites_state.go @@ -0,0 +1,30 @@ +package build + +import ( + "encoding/json" + "fmt" + "os" +) + +// A marker proves provenance, not ownership: copied marked files must survive +// orphan pruning unless this project's generated-state record owns their path. +func recordedSiteFiles(sink Sink) (map[string]string, error) { + data, err := sink.ReadFile(".nself/state/generated.json") + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("reading generated-state ownership: %w", err) + } + var state struct { + SchemaVersion string `json:"schema_version"` + Files map[string]string `json:"files"` + } + if err := json.Unmarshal(data, &state); err != nil { + return nil, fmt.Errorf("reading generated-state ownership: %w", err) + } + if state.SchemaVersion != "1" { + return nil, fmt.Errorf("unsupported generated-state ownership schema %q", state.SchemaVersion) + } + return state.Files, nil +} diff --git a/internal/reconcile/apply.go b/internal/reconcile/apply.go index 474149f34..7708ca344 100644 --- a/internal/reconcile/apply.go +++ b/internal/reconcile/apply.go @@ -77,8 +77,8 @@ func ApplyBuild(ctx context.Context, req Request, opt ApplyOptions) (applied *Pl // P7-LIVE-04 (EPIC D5): the generated-state record tells a generated change // from a human one. A missing record is the first run on an existing - // project: nothing is detected (never block), the apply records what it - // wrote and warns once below. A record a caller already supplied (tests, + // project: compute compares marked files to the fresh render, and the + // apply records what it wrote and warns once below. A record a caller already supplied (tests, // later callers) is used as given. state, err := LoadGeneratedState(req.ProjectDir) if err != nil { diff --git a/internal/reconcile/handedit.go b/internal/reconcile/handedit.go index 074a8a7cb..bff8ad680 100644 --- a/internal/reconcile/handedit.go +++ b/internal/reconcile/handedit.go @@ -19,6 +19,7 @@ package reconcile // formatter or editor makes counts. import ( + "bytes" "crypto/sha256" "encoding/hex" "errors" @@ -212,3 +213,21 @@ func warnHandEdits(req Request, p *Plan, c *computed, v15 bool) error { // firstRunNotice is the one-time warning of a project's first recorded apply. const firstRunNotice = "nself: no generated-state record was found; the files this build wrote are recorded in .nself/state/generated.json, and hand-edits are detected against them from now on\n" + +// Without a recorded baseline, the fresh render is the only safe comparison +// for a marked file. Differences need the same explicit force as recorded edits. +func unrecordedHandEdited(before, after ArtifactSet, state *GeneratedState, next func(string) bool) func(string) bool { + return func(path string) bool { + if next != nil && next(path) { + return true + } + if state != nil { + if _, ok := state.Files[path]; ok { + return false + } + } + current, exists := before[path] + fresh, rendered := after[path] + return exists && rendered && IsGenerated(current.Data) && !bytes.Equal(current.Data, fresh.Data) + } +} diff --git a/internal/reconcile/planner.go b/internal/reconcile/planner.go index 58e96b4be..a4a16445c 100644 --- a/internal/reconcile/planner.go +++ b/internal/reconcile/planner.go @@ -109,7 +109,7 @@ func compute(ctx context.Context, req Request) (*computed, error) { } before[GeneratedStateDisplayPath] = File{Data: raw, Perm: perm} } - arts := append(Diff(before, after, pendingHandEdited(req.HandEdited, state, env, req.ProjectDir, res.Planned.PluginDir)), modeArtifacts(before, after)...) + arts := append(Diff(before, after, unrecordedHandEdited(before, after, state, pendingHandEdited(req.HandEdited, state, env, req.ProjectDir, res.Planned.PluginDir))), modeArtifacts(before, after)...) p := &Plan{ Command: req.Command, Trigger: req.Trigger, diff --git a/internal/reconcile/state.go b/internal/reconcile/state.go index 0ab728b3c..5556e6574 100644 --- a/internal/reconcile/state.go +++ b/internal/reconcile/state.go @@ -22,7 +22,8 @@ package reconcile // Safe-On-Live.md, not normalised here). Recording is bookkeeping both compat // modes do; only the refusal that reads the record is v1.5 behaviour (ADR 0021, // gated in confirm.go). A first run on a project without a record records it -// and warns once (apply.go); it never blocks. +// and warns once (apply.go); marked files differing from the fresh render +// still need --force. import ( "crypto/sha256" diff --git a/internal/reconcile/unrecorded_compare_test.go b/internal/reconcile/unrecorded_compare_test.go new file mode 100644 index 000000000..3aa9c32fe --- /dev/null +++ b/internal/reconcile/unrecorded_compare_test.go @@ -0,0 +1,24 @@ +package reconcile + +import "testing" + +func TestUnrecordedMarkedComparison(t *testing.T) { + for _, row := range []struct { + name, current, fresh string + want bool + }{ + {"same", "# Generated by nself\n", "# Generated by nself\n", false}, + {"edited", "# Generated by nself\n# edit\n", "# Generated by nself\n", true}, + {"unmarked", "user file", "# Generated by nself\n", false}, + } { + t.Run(row.name, func(t *testing.T) { + before := ArtifactSet{"file": {Data: []byte(row.current)}} + after := ArtifactSet{"file": {Data: []byte(row.fresh)}} + for _, state := range []*GeneratedState{nil, {Files: map[string]string{}}} { + if got := unrecordedHandEdited(before, after, state, nil)("file"); got != row.want { + t.Fatalf("hand edited = %v, want %v", got, row.want) + } + } + }) + } +} diff --git a/internal/reconcile/unrecorded_test.go b/internal/reconcile/unrecorded_test.go new file mode 100644 index 000000000..f7ddcb5e5 --- /dev/null +++ b/internal/reconcile/unrecorded_test.go @@ -0,0 +1,66 @@ +package reconcile + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/nself-org/cli/internal/compat/compattest" + "github.com/nself-org/cli/internal/errs" +) + +func TestUnrecordedMarkedFiles(t *testing.T) { + for _, missingState := range []bool{false, true} { + t.Run(map[bool]string{false: "missing entry", true: "missing state"}[missingState], func(t *testing.T) { + compattest.Set(t, true) + f := loadFixture(t, "dev-minimal") + req := Request{Runtime: &fakeRuntime{}, ProjectDir: f.project, Seed: []byte("seed")} + if _, err := Apply(context.Background(), req, ApplyOptions{Yes: true, Force: true}); err != nil { + t.Fatal(err) + } + target := filepath.Join(f.project, "docker-compose.yml") + body, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + edited := append(body, []byte("\n# hand edit\n")...) + if err := os.WriteFile(target, edited, 0644); err != nil { + t.Fatal(err) + } + orphan := filepath.Join(f.project, "nginx/sites/foreign-copy.conf") + const copyBody = "# Generated by nself build\n# hand-edited copy\n" + if err := os.WriteFile(orphan, []byte(copyBody), 0644); err != nil { + t.Fatal(err) + } + if missingState { + if err := os.Remove(GeneratedStatePath(f.project)); err != nil { + t.Fatal(err) + } + } else { + state, err := LoadGeneratedState(f.project) + if err != nil { + t.Fatal(err) + } + delete(state.Files, "docker-compose.yml") + if err := state.Save(f.project); err != nil { + t.Fatal(err) + } + } + before := treeHash(t, f.root) + _, err = Apply(context.Background(), req, ApplyOptions{Yes: true}) + if err == nil || errs.Describe(err) == nil || errs.Describe(err).Code != "E403" { + t.Fatalf("want E403, got %v", err) + } + if after := treeHash(t, f.root); before != after { + t.Fatal("refusal changed tree") + } + if _, err := Apply(context.Background(), req, ApplyOptions{Yes: true, Force: true}); err != nil { + t.Fatal(err) + } + if got, err := os.ReadFile(orphan); err != nil || string(got) != copyBody { + t.Fatalf("foreign copy lost: %v", err) + } + }) + } +}