diff --git a/cmd/commands/init_templates_source.go b/cmd/commands/init_templates_source.go index 1865199d4..c5ed6fb55 100644 --- a/cmd/commands/init_templates_source.go +++ b/cmd/commands/init_templates_source.go @@ -9,8 +9,6 @@ package commands import ( "context" - "slices" - "strings" "github.com/nself-org/cli/internal/license" "github.com/nself-org/cli/internal/plugin/catalog" @@ -27,7 +25,7 @@ var templateSource clonetemplate.Source = func(ctx context.Context) (clonetempla e, ok := res.Catalog.Lookup(slug) return clonetemplate.PluginInfo{Slug: e.Slug, Tier: e.Tier, Bundles: e.Bundles}, ok } - keys := templateLicenseKeys() + keys := license.ConfiguredKeys() return find, func(ctx context.Context, bundle string) (bool, error) { var firstErr error for _, key := range keys { @@ -42,20 +40,3 @@ var templateSource clonetemplate.Source = func(ctx context.Context) (clonetempla return false, firstErr }, nil } - -// templateLicenseKeys is every configured licence key, the owner key first, so -// coverage under any of them counts (multi-key support, as plugin install). -func templateLicenseKeys() []string { - keys := license.CollectLicenseKeys() - // CollectLicenseKeys reads only the primary key file; key.2 to key.10 are - // stored keys too, and a covered second key must not read as E110. - for _, k := range license.GetAllStoredKeys() { - if k = strings.TrimSpace(k); k != "" && !slices.Contains(keys, k) { - keys = append(keys, k) - } - } - if k := strings.TrimSpace(license.CollectLicenseKey()); k != "" { - keys = append([]string{k}, slices.DeleteFunc(keys, func(x string) bool { return x == k })...) - } - return keys -} diff --git a/cmd/commands/init_templates_test.go b/cmd/commands/init_templates_test.go index 0557e3499..b21e27524 100644 --- a/cmd/commands/init_templates_test.go +++ b/cmd/commands/init_templates_test.go @@ -15,6 +15,7 @@ import ( "github.com/nself-org/cli/internal/compat" "github.com/nself-org/cli/internal/compat/compattest" "github.com/nself-org/cli/internal/errs" + "github.com/nself-org/cli/internal/license" clonetemplate "github.com/nself-org/cli/internal/templates/clone" ) @@ -249,7 +250,7 @@ func TestTemplateLicenseKeysOwnerFirst(t *testing.T) { t.Setenv("NSELF_PLUGIN_LICENSE_KEY", "fake-key-alpha") t.Setenv("NSELF_LICENSE_KEY_1", "fake-key-bravo") t.Setenv("NSELF_LICENSE_KEY_2", "fake-key-charlie") - if got := strings.Join(templateLicenseKeys(), ","); got != "fake-key-bravo,fake-key-alpha,fake-key-charlie" { + if got := strings.Join(license.ConfiguredKeys(), ","); got != "fake-key-bravo,fake-key-alpha,fake-key-charlie" { t.Errorf("keys = %s, want the owner key first, each once", got) } } @@ -268,7 +269,7 @@ func TestTemplateLicenseKeysStoredNumbered(t *testing.T) { t.Fatal(err) } } - if got := strings.Join(templateLicenseKeys(), ","); got != "fake-key-one,fake-key-two,fake-key-ten" { + if got := strings.Join(license.ConfiguredKeys(), ","); got != "fake-key-one,fake-key-two,fake-key-ten" { t.Errorf("keys = %q, want the primary then key.2 and key.10, each once", got) } } @@ -279,7 +280,7 @@ func TestTemplateLicenseKeysOwnerKeyTrimmed(t *testing.T) { compattest.Set(t, true) isolateSource(t, func(http.ResponseWriter, *http.Request) {}) t.Setenv("NSELF_PLUGIN_LICENSE_KEY_OWNER", "fake-key-bravo\n") - if got := strings.Join(templateLicenseKeys(), ","); got != "fake-key-bravo" { + if got := strings.Join(license.ConfiguredKeys(), ","); got != "fake-key-bravo" { t.Errorf("keys = %q, want the trimmed owner key", got) } } diff --git a/cmd/commands/start_containers.go b/cmd/commands/start_containers.go index f2b30619b..0f55edd04 100644 --- a/cmd/commands/start_containers.go +++ b/cmd/commands/start_containers.go @@ -220,6 +220,12 @@ func postgresAlreadyRunning(healthStatus string) bool { return healthStatus == "healthy" } +var dbInitCredentialHints = []string{ // .env.secrets overrides .env; build persists credentials there (D-0708) + "Check PostgreSQL logs: nself logs postgres", + "Verify POSTGRES_USER and POSTGRES_PASSWORD in the file that defines them; .env.secrets overrides .env, so edit it there once build has persisted a value", + "Try restarting: nself restart", +} + // initializeDatabasePhase runs database initialization (Step 5). Must run // BEFORE other services start: the auth service requires the auth schema to // exist in PostgreSQL, so schemas and extensions must be created while only @@ -244,11 +250,7 @@ func initializeDatabasePhase(ctx context.Context, cfg *config.Config, opts start // hyphenated project directory name) is a config problem, not a // connectivity/credentials one — the generic hints below actively // 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", - "Try restarting: nself restart", - } + hints := dbInitCredentialHints if errors.Is(err, database.ErrInvalidIdentifier) { hints = []string{ "The name is not a valid SQL identifier: it must start with a letter or underscore and contain only letters, digits, and underscores", diff --git a/cmd/commands/start_containers_test.go b/cmd/commands/start_containers_test.go index ad351ee17..c9baee2e6 100644 --- a/cmd/commands/start_containers_test.go +++ b/cmd/commands/start_containers_test.go @@ -22,7 +22,10 @@ package commands // value ("healthy", "unhealthy", "starting", "none", // "not_found") plus an empty string for the zero value. -import "testing" +import ( + "strings" + "testing" +) func TestPostgresAlreadyRunning(t *testing.T) { tests := []struct { @@ -46,3 +49,11 @@ func TestPostgresAlreadyRunning(t *testing.T) { }) } } + +// D-0708: .env.secrets wins over .env, so the credential hint must name it. +func TestDBInitCredentialHints_NameEnvSecrets(t *testing.T) { + joined := strings.Join(dbInitCredentialHints, "\n") + if !strings.Contains(joined, ".env.secrets") { + t.Errorf("hints must point at .env.secrets, got:\n%s", joined) + } +} diff --git a/internal/backup/destinations/inline_option.go b/internal/backup/destinations/inline_option.go new file mode 100644 index 000000000..f4536ea21 --- /dev/null +++ b/internal/backup/destinations/inline_option.go @@ -0,0 +1,19 @@ +package destinations + +import ( + "regexp" + "strings" +) + +// inlineOptionRe is the `,key=` shape of an inline rclone option ("myr,pass=x"). +var inlineOptionRe = regexp.MustCompile(`,[A-Za-z0-9_]+=`) + +// HasInlineOption reports whether a relative uri carries an inline rclone +// option with no ":" after it ("myr,pass=ab/cd"): a malformed remote, not a +// directory. "./" and "../" always mean a path, so they never match. +func HasInlineOption(uri string) bool { + if strings.HasPrefix(uri, "./") || strings.HasPrefix(uri, "../") { + return false + } + return inlineOptionRe.MatchString(uri) +} diff --git a/internal/backup/destinations/inline_option_test.go b/internal/backup/destinations/inline_option_test.go new file mode 100644 index 000000000..d28ad348f --- /dev/null +++ b/internal/backup/destinations/inline_option_test.go @@ -0,0 +1,17 @@ +package destinations + +import "testing" + +func TestHasInlineOption(t *testing.T) { + for uri, want := range map[string]bool{ + "myr,pass=ab/cd": true, + "./myr,pass=ab/cd": false, + "../myr,pass=ab/cd": false, + "backups": false, + "back,ups": false, + } { + if got := HasInlineOption(uri); got != want { + t.Errorf("HasInlineOption(%q) = %v, want %v", uri, got, want) + } + } +} diff --git a/internal/backup/stream_dest_compat.go b/internal/backup/stream_dest_compat.go index 217ea1215..a5f4f0dd4 100644 --- a/internal/backup/stream_dest_compat.go +++ b/internal/backup/stream_dest_compat.go @@ -12,7 +12,6 @@ import ( "io" "os" "path/filepath" - "regexp" "strings" "sync" @@ -30,20 +29,14 @@ var ( bareNoticed = map[string]bool{} ) -// inlineOptionRe is the `,key=` shape of an inline rclone option -// ("myr,pass=x"). A relative value carrying it is a malformed remote, not a -// directory, even when no ":" follows. -var inlineOptionRe = regexp.MustCompile(`,[A-Za-z0-9_]+=`) - // isBarePath reports whether rclone would have read uri as a local path: not // empty, not option-shaped, and no ":" before the first "/" (so no scheme, no // name:path remote and no :backend connection string). A value starting with // "/" is always a path (no rclone remote starts with "/"), commas included. // A value with padding whitespace or a control character, and a relative // value with an inline-credential shape (a "," before a ":", e.g. -// "myr,pass=ab/cd:p", or a ",key=" option with no ":" at all, "myr,pass=ab/cd"; "./" and "../" always mean a path), -// are left to the parser, which refuses them without -// echoing them (D-0467). +// "myr,pass=ab/cd:p", or a ",key=" option with no ":", see HasInlineOption), are +// left to the parser, which refuses them without echoing them (D-0467). func isBarePath(uri string) bool { if uri == "" || strings.HasPrefix(uri, "-") || uri != strings.TrimSpace(uri) { return false @@ -54,11 +47,7 @@ func isBarePath(uri string) bool { if strings.HasPrefix(uri, "/") { return true } - if strings.Contains(uri, ":") && destinations.HasInlineCredentials(uri) { - return false - } - explicitPath := strings.HasPrefix(uri, "./") || strings.HasPrefix(uri, "../") - if !explicitPath && inlineOptionRe.MatchString(uri) { + if (strings.Contains(uri, ":") && destinations.HasInlineCredentials(uri)) || destinations.HasInlineOption(uri) { return false } head, _, _ := strings.Cut(uri, "/") diff --git a/internal/build/orchestrator_secrets.go b/internal/build/orchestrator_secrets.go index 3bcbb48db..cf8c5d8f2 100644 --- a/internal/build/orchestrator_secrets.go +++ b/internal/build/orchestrator_secrets.go @@ -23,6 +23,7 @@ import ( "path/filepath" "strings" + "github.com/nself-org/cli/internal/composeenv" "github.com/nself-org/cli/internal/config" ) @@ -84,7 +85,7 @@ func persistGeneratedSecretsFx(workdir string, cfg *config.Config, fx Effects, s if *c.dst == "" || onDisk[c.key] || os.Getenv(c.key) != "" { continue } - if live, ok := adoptLiveCredential(sink, workdir, c.key); ok { + if live, ok := adoptLiveCredential(sink, workdir, composeenv.EnvName(cfg.Env), c.key); ok { *c.dst = live if !fx.Planning() { slog.Warn("key is in no env file; reusing the value the running stack was built with. Pin it by adding the key to .env.secrets (or your env file)", "key", c.key) diff --git a/internal/build/orchestrator_secrets_adopt.go b/internal/build/orchestrator_secrets_adopt.go index a73809eea..f745c3faf 100644 --- a/internal/build/orchestrator_secrets_adopt.go +++ b/internal/build/orchestrator_secrets_adopt.go @@ -10,44 +10,30 @@ package build // ${VAR} reference or an empty value is not a credential. Values // are never logged. An adopted value is used for this build's // render only and is never written to .env.secrets. .nself/ -// compose.env holds whichever ENV built last; adoption only +// compose.env holds whichever ENV built last (composeenv +// records that ENV and refuses another's); adoption only // happens when no env file and no process export supplies the // key, so it can only pick up a value an earlier build got from a // process export or generated itself. .env.computed is not read: // it never carries these keys literally. import ( - "bufio" - "bytes" "path/filepath" + "strings" + "github.com/nself-org/cli/internal/composeenv" "github.com/nself-org/cli/internal/config" ) // adoptLiveCredential returns key's literal value from the previous build's -// .nself/compose.env. -func adoptLiveCredential(sink Sink, workdir, key string) (string, bool) { +// .nself/compose.env, unless a different ENV (env) wrote it (D-1149). +func adoptLiveCredential(sink Sink, workdir, env, key string) (string, bool) { data, err := sink.ReadFile(filepath.Join(workdir, composeEnvFile)) if err != nil { return "", false } - sc := bufio.NewScanner(bytes.NewReader(data)) - sc.Buffer(make([]byte, 0, 64*1024), 1<<20) - for sc.Scan() { - line := strings.TrimSpace(sc.Text()) - if line == "" || line[0] == '#' { - continue - } - k, raw, ok := strings.Cut(line, "=") - if !ok || k != key { - continue - } - if v := parseEnvValue(raw); v != "" && !strings.HasPrefix(v, "${") { - return v, true - } - } - return "", false + return composeenv.Literal(data, env, key, parseEnvValue) } // parseEnvValue is the inverse of config.QuoteComposeEnvValue and diff --git a/internal/build/orchestrator_secrets_adopt_test.go b/internal/build/orchestrator_secrets_adopt_test.go index eb325a1c4..c892c9942 100644 --- a/internal/build/orchestrator_secrets_adopt_test.go +++ b/internal/build/orchestrator_secrets_adopt_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "github.com/nself-org/cli/internal/composeenv" "github.com/nself-org/cli/internal/config" ) @@ -243,3 +244,54 @@ func TestSecretScanList_FollowsCascadeRule(t *testing.T) { } } } + +// D-1149: compose.env written by a dev build must not hand its password to a +// prod build; the same env still adopts, and a file without the marker (built +// before it existed) keeps adopting. +func TestPersistGeneratedSecrets_AdoptHonoursBuildEnvMarker(t *testing.T) { + const body = "POSTGRES_PASSWORD=fake-dev-pg-password\nHASURA_GRAPHQL_ADMIN_SECRET=fake-dev-admin-secret\n" + for _, tc := range []struct { + name, header, env string + adopt bool + }{ + {"dev file, prod build", composeenv.Marker + "dev\n", "prod", false}, + {"prod file, dev build", composeenv.Marker + "prod\n", "dev", false}, + {"prod file, prod build", composeenv.Marker + "prod\n", "prod", true}, + {"dev file, unset env is dev", composeenv.Marker + "dev\n", "", true}, + {"no marker", "", "prod", true}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := projectWithComposeEnv(t, tc.header+body) + cfg := genCfg() + cfg.Env = tc.env + if err := persistGeneratedSecrets(dir, cfg); err != nil { + t.Fatal(err) + } + adopted := cfg.Postgres.Password == "fake-dev-pg-password" && cfg.Hasura.AdminSecret == "fake-dev-admin-secret" + if adopted != tc.adopt { + t.Fatalf("adopted = %v, want %v", adopted, tc.adopt) + } + if !tc.adopt && !strings.Contains(readSecrets(t, dir), "POSTGRES_PASSWORD=fake-generated-pg-password\n") { + t.Error("a refused adoption must fall back to persisting the generated value") + } + }) + } +} + +// The writer records the ENV that the reader checks. +func TestWriteComposeEnv_RecordsBuildEnv(t *testing.T) { + dir := t.TempDir() + cfg := genCfg() + cfg.Env = "prod" + cfg.ProjectName = "test" + sink := newDiskSink(dir) + if err := writeComposeEnvVia(sink, dir, cfg, map[string]string{"POSTGRES_PASSWORD": "fake-pg"}, nil); err != nil { + t.Fatal(err) + } + if got, ok := adoptLiveCredential(sink, dir, "prod", "POSTGRES_PASSWORD"); !ok || got != "fake-pg" { + t.Errorf("same env: got %q, %v", got, ok) + } + if _, ok := adoptLiveCredential(sink, dir, "dev", "POSTGRES_PASSWORD"); ok { + t.Error("dev build adopted a prod compose.env") + } +} diff --git a/internal/build/plugins_entitlement_mount_test.go b/internal/build/plugins_entitlement_mount_test.go index 1bd6a9156..2f984b6eb 100644 --- a/internal/build/plugins_entitlement_mount_test.go +++ b/internal/build/plugins_entitlement_mount_test.go @@ -90,6 +90,11 @@ func TestMountIgnoresOverride(t *testing.T) { func TestMountEmptyFileE125(t *testing.T) { compattest.Set(t, true) + // No licence key from the host: a stored or env key would renew a real JWT online. + t.Setenv("HOME", t.TempDir()) + t.Setenv("USERPROFILE", t.TempDir()) + t.Setenv("NSELF_PLUGIN_LICENSE_KEY", "") + t.Setenv("NSELF_PLUGIN_LICENSE_KEY_OWNER", "") t.Setenv("LICENSE_CACHE_PATH", filepath.Join(t.TempDir(), "missing")) t.Setenv("LICENSE_PUBLIC_KEY_OVERRIDE", "") dir, in := entitlementFixture(t, "licensed") diff --git a/internal/build/secrets_template.go b/internal/build/secrets_template.go index ad945564c..ce47e3c3d 100644 --- a/internal/build/secrets_template.go +++ b/internal/build/secrets_template.go @@ -26,6 +26,7 @@ import ( "sort" "strings" + "github.com/nself-org/cli/internal/composeenv" "github.com/nself-org/cli/internal/config" ) @@ -232,6 +233,7 @@ func writeComposeEnvVia(sink Sink, workdir string, cfg *config.Config, secrets, sb.WriteString("# GENERATED BY nself build - DO NOT HAND EDIT\n") sb.WriteString("# Interpolation values for the ${VAR} references in docker-compose.yml.\n") sb.WriteString("# Passed to docker compose via --env-file by nself start/stop/restart.\n") + sb.WriteString(composeenv.Marker + composeenv.EnvName(cfg.Env) + "\n") // D-1149 for _, k := range keys { sb.WriteString(k) sb.WriteString("=") diff --git a/internal/build/testdata/plan-mode/golden-dev-minimal.txt b/internal/build/testdata/plan-mode/golden-dev-minimal.txt index cead9c1af..825f73a89 100644 --- a/internal/build/testdata/plan-mode/golden-dev-minimal.txt +++ b/internal/build/testdata/plan-mode/golden-dev-minimal.txt @@ -16,7 +16,7 @@ d 0755 project/ssl d 0755 project/ssl/certificates f 0600 project/.env 3f9f5e92353c2eafb5d2fbf56c4693cf8feb9bf82bb07d1b1e3bc230339f2db5 f 0600 project/.env.computed 11d69b678463f3b0ecff1c6a2036bd3ac6ac39c6f0ab75b775da0196f209ed55 -f 0600 project/.nself/compose.env fd68648754cf9a6c5f72298ef87d050aee49495249735d97bea75fbb09622caa +f 0600 project/.nself/compose.env 2850ac3f38865ec99ae420df326ba1bc4cf199e09f705731d6f56caad5a55023 f 0600 project/docker-compose.yml 916a3b8f73f849cdf1ac401dca17e3991695f54bc0d9fad3779f00ebf73647de f 0644 project/.nself/build-profile a172cedcae47474b615c54d510a5d84a8dea3032e958587430b413538be3f333 f 0644 project/.nself/build-version 8ceaa7b1b997a4743b4d5e74b2a46af8c5ebe7765abb07eca372441aa3cbf51e diff --git a/internal/build/testdata/plan-mode/golden-fronted.txt b/internal/build/testdata/plan-mode/golden-fronted.txt index fb1af2f49..5186166f3 100644 --- a/internal/build/testdata/plan-mode/golden-fronted.txt +++ b/internal/build/testdata/plan-mode/golden-fronted.txt @@ -19,7 +19,7 @@ d 0755 home d 0755 plugins f 0600 fronted-stack/backend/.env 779469afb28aab57a312a30294588741be9f1bae8794eaca9a5f2fc4630e45b5 f 0600 fronted-stack/backend/.env.computed 11d69b678463f3b0ecff1c6a2036bd3ac6ac39c6f0ab75b775da0196f209ed55 -f 0600 fronted-stack/backend/.nself/compose.env fd68648754cf9a6c5f72298ef87d050aee49495249735d97bea75fbb09622caa +f 0600 fronted-stack/backend/.nself/compose.env 2850ac3f38865ec99ae420df326ba1bc4cf199e09f705731d6f56caad5a55023 f 0600 fronted-stack/backend/docker-compose.yml 680e12ec51abc9b8ab6443494c6f4d1e4bd79faee3e2be3a0b9795fd3132c3a3 f 0644 fronted-stack/backend/.nself/build-profile a172cedcae47474b615c54d510a5d84a8dea3032e958587430b413538be3f333 f 0644 fronted-stack/backend/.nself/build-version 8ceaa7b1b997a4743b4d5e74b2a46af8c5ebe7765abb07eca372441aa3cbf51e diff --git a/internal/build/testdata/plan-mode/golden-prod-plugins.txt b/internal/build/testdata/plan-mode/golden-prod-plugins.txt index 9e3df72eb..13f3be5ba 100644 --- a/internal/build/testdata/plan-mode/golden-prod-plugins.txt +++ b/internal/build/testdata/plan-mode/golden-prod-plugins.txt @@ -25,7 +25,7 @@ d 0755 project/ssl/certificates d 0755 project/ssl/certificates/example-org f 0600 project/.env a453ab255f1ff28c49aa67169dea59e6ed810549318c1bf13d96b484bc299829 f 0600 project/.env.computed 2f35a3b9b8f489945a149d23e41588cca4a7a21686ab5e8b365d09b991ed6756 -f 0600 project/.nself/compose.env 2694d3f78d0e965fc7cef1e35bd08629860d0141633e4be6676ce11e036356e6 +f 0600 project/.nself/compose.env 5e447d1c6fda2d1d9921cd2418eacda7c07066bf981db2c60ee7842dab9794d2 f 0600 project/docker-compose.yml 3b0c42186c3ef4fd16bd5228d3e92a11ef4c631c000faffa30036ce2ffea1e59 f 0600 project/ssl/certificates/example-org/privkey.pem 4a3ec3730504983f960fb2df35a1d68d640ff55d151fa3128ca0fc707f86882e f 0644 plugins/nself-alpha/Dockerfile bb57c7da220a8753d7bdabac0d3afdb6efa742e4c736c5bc93ab40dfd5e23b9b diff --git a/internal/composeenv/composeenv.go b/internal/composeenv/composeenv.go new file mode 100644 index 000000000..fa0b618ff --- /dev/null +++ b/internal/composeenv/composeenv.go @@ -0,0 +1,60 @@ +package composeenv + +// Purpose: read .nself/compose.env, the interpolation file `nself build` +// writes, for the credential a later build may adopt (D-0506, D-1149). +// Inputs: the file bytes, the ENV the reading build renders for, a key. +// Outputs: the literal value the previous build rendered for the key. +// Constraints: compose.env holds whichever ENV built last. The writer records +// that ENV in a comment line (Marker); a different ENV adopts +// nothing, so a prod build never inherits a dev password. A file +// without the marker predates it and is still read. A ${VAR} +// reference or an empty value is not a credential. The first +// literal wins. Values are never logged. + +import ( + "bufio" + "bytes" + "strings" +) + +// Marker prefixes the comment line naming the ENV that wrote the file. It is a +// comment, so docker compose ignores it. +const Marker = "# nself-build-env: " + +// EnvName is the ENV a build renders for; an unset env is dev. +func EnvName(env string) string { + if env == "" { + return "dev" + } + return env +} + +// Literal returns key's literal value from compose.env data written by env. +// parse turns the raw text after `=` into the value (it undoes the quoting the +// writer applied). +func Literal(data []byte, env, key string, parse func(string) string) (string, bool) { + sc := bufio.NewScanner(bytes.NewReader(data)) + sc.Buffer(make([]byte, 0, 64*1024), 1<<20) + var found string + var have bool + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + if built, ok := strings.CutPrefix(line, Marker); ok { + if strings.TrimSpace(built) != env { + return "", false + } + continue + } + if line == "" || line[0] == '#' { + continue + } + k, raw, ok := strings.Cut(line, "=") + if !ok || k != key { + continue + } + if v := parse(raw); v != "" && !have && !strings.HasPrefix(v, "${") { + found, have = v, true + } + } + return found, have +} diff --git a/internal/composeenv/composeenv_test.go b/internal/composeenv/composeenv_test.go new file mode 100644 index 000000000..2c6a74286 --- /dev/null +++ b/internal/composeenv/composeenv_test.go @@ -0,0 +1,35 @@ +package composeenv + +import ( + "strings" + "testing" +) + +func unquote(s string) string { return strings.Trim(s, `"`) } + +// D-1149: a file written by another ENV yields nothing; no marker still reads. +func TestLiteral(t *testing.T) { + const body = "POSTGRES_PASSWORD=\"fake-pg\"\nEMPTY=\nREF=${X}\n" + for _, tc := range []struct { + name, file, env, key, want string + }{ + {"same env", Marker + "prod\n" + body, "prod", "POSTGRES_PASSWORD", "fake-pg"}, + {"other env", Marker + "dev\n" + body, "prod", "POSTGRES_PASSWORD", ""}, + {"marker after key", body + Marker + "dev\n", "prod", "POSTGRES_PASSWORD", ""}, + {"no marker", body, "prod", "POSTGRES_PASSWORD", "fake-pg"}, + {"empty value", body, "prod", "EMPTY", ""}, + {"reference", body, "prod", "REF", ""}, + {"first literal wins", "K=one\nK=two\n", "dev", "K", "one"}, + } { + got, ok := Literal([]byte(tc.file), tc.env, tc.key, unquote) + if got != tc.want || ok != (tc.want != "") { + t.Errorf("%s: Literal = %q, %v; want %q", tc.name, got, ok, tc.want) + } + } +} + +func TestEnvName(t *testing.T) { + if EnvName("") != "dev" || EnvName("prod") != "prod" { + t.Error("EnvName: an unset env is dev, any other passes through") + } +} diff --git a/internal/config/secretkeys.go b/internal/config/secretkeys.go index 09fef183e..e1e00f26f 100644 --- a/internal/config/secretkeys.go +++ b/internal/config/secretkeys.go @@ -5,14 +5,15 @@ package config // helpers, internal/env and the machine audit log (P7-SURF-25, P7-SURF-12). // Inputs: a config key or flag name; a config value. // Outputs: true when the name contains SECRET, PASS (which covers PASSWORD and -// AUTH_SMTP_PASS), KEY, TOKEN, CREDENTIAL, WEBHOOK, PRIVATE, DSN, SALT or BASICAUTH -// (case-insensitive); a value with its URL credentials masked. +// AUTH_SMTP_PASS), KEY, TOKEN, CREDENTIAL, WEBHOOK, PRIVATE, DSN, SALT or BASICAUTH, +// or is AUTH or ends in _AUTH (case-insensitive); a value with its URL credentials masked. // Constraints: over-matching (for example PUBLIC_KEY or BYPASS) is the safe // side: a match only redacts. internal/env must not keep a second list. import ( - "regexp" "strings" + + "github.com/nself-org/cli/internal/credmask" ) // secretKeyParts are the substrings whose presence in an upper-cased key name @@ -27,228 +28,12 @@ func IsSecretKey(key string) bool { return true } } - return false -} - -// urlParam is one query, matrix or fragment parameter found by urlParams: -// the name is v[nameStart:nameEnd]; the value starts at valueStart, after -// the `=` or its encoded form `%3D` (encodedEq). -type urlParam struct { - start, nameStart, nameEnd, valueStart int - encodedEq bool -} - -// urlParams finds every parameter of v: a separator (?, &, ; or #), the -// name (any bytes but &, ; and #; spaces and ? allowed) and the first `=` or -// `%3D` after it. It never consumes a value, so a parameter nested in another -// parameter's value (?redirect=https://x/?token=...) is still seen. Two -// reverse-built tables make it linear in len(v) on any input. -func urlParams(v string) []urlParam { - n := len(v) - nextEq, nextStop := make([]int, n+1), make([]int, n+1) - nextEq[n], nextStop[n] = n, n - for k := n - 1; k >= 0; k-- { - nextEq[k], nextStop[k] = nextEq[k+1], nextStop[k+1] - switch { - case v[k] == '=' || (v[k] == '%' && k+2 < n && v[k+1] == '3' && (v[k+2] == 'D' || v[k+2] == 'd')): - nextEq[k] = k - case v[k] == '&' || v[k] == ';' || v[k] == '#': - nextStop[k] = k - } - } - var out []urlParam - for k := 0; k < n; k++ { - if c := v[k]; c != '?' && c != '&' && c != ';' && c != '#' { - continue - } - eq := nextEq[k+1] - if eq >= n || nextStop[k+1] < eq { - continue - } - p := urlParam{start: k, nameStart: k + 1, nameEnd: eq, valueStart: eq + 1} - if v[eq] == '%' { - p.encodedEq, p.valueStart = true, eq+3 - } - out = append(out, p) - k = p.valueStart - 1 - } - return out -} - -// credentialWord matches a credential word anywhere in a decoded parameter -// name (access_token, client_secret, api-key, X-Amz-Signature, auth, jwt, -// jsessionid). Over-matching only masks. -var credentialWord = regexp.MustCompile(`(?i)pass|pwd|token|secret|key|signature|auth|credential|jwt|session`) - -// credentialShortName matches the short credential names that would mask -// ordinary words as substrings (signup, design, country_code), so they count -// only as the whole name: Azure SAS sig, the OAuth code, pw. -var credentialShortName = regexp.MustCompile(`(?i)^\s*(sig|code|pw)\s*$`) - -// isCredentialName reports whether a decoded parameter name names a credential. -func isCredentialName(name string) bool { - return credentialWord.MatchString(name) || credentialShortName.MatchString(name) -} - -// maxNestedChecks bounds the decode checks for parameters nested in one value; -// a value holding more is masked whole, so a hostile value cannot make the -// scan quadratic. -const maxNestedChecks = 8 - -// MaskURLCredentials masks the credentials of a value that holds a URL. For -// `scheme://` followed later by `@`, everything between `://` and the LAST `@` -// becomes `****` (user and password), so a password that contains `@`, `/`, -// `:`, `+` or spaces never leaks and the rule does not depend on net/url -// accepting the string. Query, matrix (`;`) and fragment (`#`) parameters -// whose name is a credential (isCredentialName, any case, spaces around the -// name allowed) are masked too; the name is percent-decoded first, so -// `access%5Ftoken=`, `access_token%3D` and `#access_token=` are caught, and -// a parameter nested in another one's value is found (D-0430). A value -// whose percent-decoded form holds a credential parameter (an encoded -// redirect such as https%3A%2F%2Fx%2F%3Ftoken%3D...) is masked whole. A -// credential value runs to the next `&` or whitespace (a credential holding -// a raw `&` or space keeps the part after it); a value masked whole for an -// encoded credential runs to the next `&`, `;`, `#` or whitespace. Time is -// linear in len(v). A value without `://` is returned unchanged. The rule -// over-masks a path that holds an `@`; that is the safe side. -func MaskURLCredentials(v string) string { - i := strings.Index(v, "://") - if i < 0 { - return v - } - if rest := v[i+3:]; strings.Contains(rest, "@") { - v = v[:i+3] + "****" + rest[strings.LastIndex(rest, "@"):] - } - params := urlParams(v) - if len(params) == 0 { - return v - } - valueEnd, credEnd := valueEnds(v) - checks := map[int]int{} // value end -> nested decode checks spent - var b strings.Builder - done := 0 // v[:done] is already written - for _, p := range params { - if p.start < done { - continue - } - end := valueEnd[p.valueStart] - if isCredentialName(decodeParamName(v[p.nameStart:p.nameEnd])) { - end = credEnd[p.valueStart] - } else { - checks[end]++ - if checks[end] <= maxNestedChecks && !hidesCredentialParam(v[p.valueStart:end], p.encodedEq) { - continue - } - } - b.WriteString(v[done:p.valueStart]) - b.WriteString("****") - done = end - } - if done == 0 { - return v - } - b.WriteString(v[done:]) - return b.String() -} - -// valueEnds returns, for every offset of v (and len(v)), where a parameter -// value starting there ends: valueEnd at the next `&`, `;`, `#` or -// whitespace, credEnd at the next `&` or whitespace (len(v) when none). One -// reverse pass, so lookups stay linear. -func valueEnds(v string) (valueEnd, credEnd []int) { - valueEnd, credEnd = make([]int, len(v)+1), make([]int, len(v)+1) - valueEnd[len(v)], credEnd[len(v)] = len(v), len(v) - for k := len(v) - 1; k >= 0; k-- { - valueEnd[k], credEnd[k] = valueEnd[k+1], credEnd[k+1] - switch v[k] { - case '&', ' ', '\t', '\n', '\r', '\f': - valueEnd[k], credEnd[k] = k, k - case ';', '#': - valueEnd[k] = k - } - } - return valueEnd, credEnd -} - -// hidesCredentialParam reports whether a parameter value, once -// percent-decoded, holds a parameter whose name is a credential: the -// encoded redirect case, which the raw scan cannot see. Values without an -// escape are skipped (the raw scan already covers them) unless the name -// ended in an encoded `=` (encodedSep), where the raw scan cannot restart. -func hidesCredentialParam(value string, encodedSep bool) bool { - if !encodedSep && !hasPercentEscape(value) { - return false - } - d := "?" + decodeParamName(value) - for _, p := range urlParams(d) { - if isCredentialName(d[p.nameStart:p.nameEnd]) { - return true - } - } - return false + // A bare AUTH or a _AUTH suffix (BASIC_AUTH=user:pass, REDISCLI_AUTH) is + // secret; AUTH_*, HASURA_AUTH_URL and AUTH_JWT_* config keys stay visible. + return upper == "AUTH" || strings.HasSuffix(upper, "_AUTH") } -// decodeParamName percent-decodes a parameter name up to three times, so a -// double-encoded name (access%255Ftoken) is matched too. It is lenient: an -// invalid escape (%zz) stays as written and the valid ones around it still -// decode, so one bad escape cannot hide a credential word. Linear per pass. -func decodeParamName(name string) string { - for n := 0; n < 3; n++ { - d, changed := percentDecodeOnce(name) - if !changed { - break - } - name = d - } - return name -} - -// hasPercentEscape reports whether s holds a valid %XX escape. -func hasPercentEscape(s string) bool { - _, changed := percentDecodeOnce(s) - return changed -} - -// percentDecodeOnce decodes every valid %XX escape of s once and reports -// whether any was found. -func percentDecodeOnce(s string) (string, bool) { - i := strings.IndexByte(s, '%') - if i < 0 { - return s, false - } - var b strings.Builder - b.Grow(len(s)) - changed := false - last := 0 - for ; i < len(s); i++ { - if s[i] != '%' || i+2 >= len(s) { - continue - } - hi, lo := unhex(s[i+1]), unhex(s[i+2]) - if hi < 0 || lo < 0 { - continue - } - b.WriteString(s[last:i]) - b.WriteByte(byte(hi<<4 | lo)) - last, changed = i+3, true - i += 2 - } - if !changed { - return s, false - } - b.WriteString(s[last:]) - return b.String(), true -} - -// unhex returns the value of one hex digit, or -1. -func unhex(c byte) int { - switch { - case '0' <= c && c <= '9': - return int(c - '0') - case 'a' <= c && c <= 'f': - return int(c-'a') + 10 - case 'A' <= c && c <= 'F': - return int(c-'A') + 10 - } - return -1 -} +// MaskURLCredentials masks the credentials of a value that holds a URL, a Go +// DSN, a bare user:pass@host or password=/pwd= pairs. The rules and their +// bounds are documented on credmask.URL. +func MaskURLCredentials(v string) string { return credmask.URL(v) } diff --git a/internal/config/secretkeys_schemeless_test.go b/internal/config/secretkeys_schemeless_test.go new file mode 100644 index 000000000..725f65674 --- /dev/null +++ b/internal/config/secretkeys_schemeless_test.go @@ -0,0 +1,62 @@ +package config + +import "testing" + +// D-0743: credentials in values without `scheme://`. Fake values only. +func TestMaskURLCredentials_Schemeless(t *testing.T) { + for _, tc := range []struct{ name, in, want string }{ + {"go mysql dsn", "app:fakepw@tcp(db:3306)/app", "****@tcp(db:3306)/app"}, + {"dsn password with @", "app:fa@ke:pw@tcp(db:3306)/app", "****@tcp(db:3306)/app"}, + {"dsn unix socket", "app:fakepw@unix(/run/mysql.sock)/app", "****@unix(/run/mysql.sock)/app"}, + {"dsn after key", "dsn=app:fakepw@tcp(db:3306)/app", "dsn=****@tcp(db:3306)/app"}, + {"dsn in prose", "connect failed for app:fakepw@tcp(db:3306)/app: refused", "connect failed for ****@tcp(db:3306)/app: refused"}, + {"dsn without password", "app@tcp(db:3306)/app", "app@tcp(db:3306)/app"}, + {"bare user:pass@host", "app:fakepw@db.internal:5432/app", "****@db.internal:5432/app"}, + {"libpq keyword", "host=db user=app password=fakepw dbname=app", "host=db user=app password=**** dbname=app"}, + {"libpq quoted", "host=db password='fake pw' dbname=app", "host=db password=**** dbname=app"}, + {"libpq double quoted", `host=db password="fake pw" dbname=app`, `host=db password=**** dbname=app`}, + {"libpq unclosed quote", "host=db password='fake pw", "host=db password=****"}, + {"libpq spaced and cased", "host=db PWD = fakepw", "host=db PWD = ****"}, + {"libpq sslpassword", "sslpassword=fakepw host=db", "sslpassword=**** host=db"}, + {"url unchanged rule", "postgres://app:fakepw@db:5432/app", "postgres://****@db:5432/app"}, + {"url then keyword", "postgres://db/app?x=1 password=fakepw", "postgres://db/app?x=1 password=****"}, + {"url param keeps neighbours", "https://h/?password=fakepw&x=1", "https://h/?password=****&x=1"}, + {"idempotent", "password=****", "password=****"}, + // Must stay as written. + {"plain user@host", "app@db.internal", "app@db.internal"}, + {"scp target", "git@github.com:org/repo", "git@github.com:org/repo"}, + {"email", "ops@example.com", "ops@example.com"}, + {"word ending in password", "mypassword=fake-keep", "mypassword=fake-keep"}, + {"host:port", "db.internal:5432", "db.internal:5432"}, + {"plain text", "hello world", "hello world"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := MaskURLCredentials(tc.in); got != tc.want { + t.Errorf("MaskURLCredentials(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestIsSecretKey_AuthSuffix(t *testing.T) { + for _, k := range []string{"BASIC_AUTH", "basic_auth", "REDISCLI_AUTH", "DB_AUTH", "AUTH"} { + if !IsSecretKey(k) { + t.Errorf("IsSecretKey(%q) = false, want true", k) + } + } + // Hasura/auth config keys that must stay visible. + for _, k := range []string{"HASURA_AUTH_URL", "AUTH_SERVER_URL", "AUTH_JWT_ALG", "RATE_LIMIT_AUTH_RPS", "POSTGRES_HOST_AUTH_METHOD", "SSH_AUTH_SOCK", "BASIC_AUTH_USER", "HASURA_GRAPHQL_AUTH_HOOK"} { + if IsSecretKey(k) { + t.Errorf("IsSecretKey(%q) = true, want false", k) + } + } +} + +// A caller's own placeholder must survive a second masking pass. +func TestMaskURLCredentials_KeepsExistingPlaceholders(t *testing.T) { + for _, in := range []string{`password = "[REDACTED]"`, "pwd=****", "password = \x00redacted\x00"} { + if got := MaskURLCredentials(in); got != in { + t.Errorf("MaskURLCredentials(%q) = %q, want unchanged", in, got) + } + } +} diff --git a/internal/config/secretkeys_test.go b/internal/config/secretkeys_test.go index 73ce67051..79b9926c2 100644 --- a/internal/config/secretkeys_test.go +++ b/internal/config/secretkeys_test.go @@ -141,6 +141,12 @@ func TestMaskURLCredentialsLinear(t *testing.T) { t.Errorf("unit %q: 4 KB %v, 40 KB %v: not linear", unit, small, large) } } + for _, unit := range []string{"@a(", ":@a(", "a:b@c(", "password="} { + small, large := schemelessTimeOf(unit, 4000), schemelessTimeOf(unit, 40000) + if large > 30*small+250*time.Millisecond { + t.Errorf("schemeless unit %q: 4 KB %v, 40 KB %v: not linear", unit, small, large) + } + } nested := "https://h/?r=" + strings.Repeat("x?a%3D", 20) + "%3Ftoken%3DFAKE36" if got := MaskURLCredentials(nested); strings.Contains(got, "FAKE36") { t.Errorf("over-nested value leaks: %q", got) @@ -149,7 +155,8 @@ func TestMaskURLCredentialsLinear(t *testing.T) { // TestMaskURLCredentialsNoPanic feeds random strings over the separator and // escape alphabet; the scanner indexes by hand, so it must never panic, and a -// value without :// must come back unchanged. +// value without :// or an `@`/`=` (the schemeless rules, D-0743) must come +// back unchanged. func TestMaskURLCredentialsNoPanic(t *testing.T) { const alphabet = "?&;#=%3Dd2F5aFt@:/ x" seed := uint32(7) @@ -161,8 +168,15 @@ func TestMaskURLCredentialsNoPanic(t *testing.T) { } seed = seed*1664525 + 1013904223 _ = MaskURLCredentials(string(b)) - if got := MaskURLCredentials(string(b[8:])); !strings.Contains(string(b[8:]), "://") && got != string(b[8:]) { + if got := MaskURLCredentials(string(b[8:])); !strings.ContainsAny(string(b[8:]), "@=") && got != string(b[8:]) { t.Fatalf("schemeless %q changed to %q", b[8:], got) } } } + +func schemelessTimeOf(unit string, size int) time.Duration { + v := strings.Repeat(unit, size/len(unit)) + start := time.Now() + _ = MaskURLCredentials(v) + return time.Since(start) +} diff --git a/internal/config/validator_placeholder_secrets.go b/internal/config/validator_placeholder_secrets.go index fdb61656a..9893577a0 100644 --- a/internal/config/validator_placeholder_secrets.go +++ b/internal/config/validator_placeholder_secrets.go @@ -54,6 +54,11 @@ type placeholderSecretField struct { value string } +// secretFileHint tells where to fix a refused secret. .env.secrets loads after +// .env and .env. and wins, so editing .env does nothing for a key that +// file already holds (build persists generated credentials there, D-0708). +const secretFileHint = "in the file that defines it (.env.secrets overrides .env and .env.)" + // validateNoPlaceholderSecrets refuses to proceed when cfg.Env is not "dev" // and any critical secret is empty or matches a known placeholder value. // Dev is exempt: placeholders there are expected and must keep working @@ -74,12 +79,12 @@ 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", - f.name, cfg.Env, errs.ErrPlaceholderSecret) + return fmt.Errorf("%s is empty in %s environment — set a unique value %s: %w", + f.name, cfg.Env, secretFileHint, 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", - f.name, cfg.Env, errs.ErrPlaceholderSecret) + return fmt.Errorf("%s is a known placeholder value in %s environment — set a unique value %s: %w", + f.name, cfg.Env, secretFileHint, errs.ErrPlaceholderSecret) } } @@ -103,8 +108,8 @@ 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", - f.name, cfg.Env, errs.ErrPlaceholderSecret) + return fmt.Errorf("%s is a known placeholder value in %s environment — set a unique value %s: %w", + f.name, cfg.Env, secretFileHint, errs.ErrPlaceholderSecret) } } diff --git a/internal/config/validator_placeholder_secrets_test.go b/internal/config/validator_placeholder_secrets_test.go index 1fc1c166a..a29d6ef88 100644 --- a/internal/config/validator_placeholder_secrets_test.go +++ b/internal/config/validator_placeholder_secrets_test.go @@ -185,3 +185,17 @@ func TestValidateNoPlaceholderSecretsWiredIntoRunAll(t *testing.T) { t.Errorf("runAll error %q does not name HASURA_GRAPHQL_ADMIN_SECRET", err.Error()) } } + +// D-0708: a persisted .env.secrets value beats .env, so the refusal must not +// send the user to .env alone. +func TestValidateNoPlaceholderSecrets_HintNamesEnvSecrets(t *testing.T) { + cfg := validPlaceholderTestConfig("prod") + cfg.Postgres.Password = "" + err := validateNoPlaceholderSecrets(cfg) + if err == nil { + t.Fatal("expected an error for an empty POSTGRES_PASSWORD") + } + if !strings.Contains(err.Error(), ".env.secrets") || strings.Contains(err.Error(), "value in .env:") { + t.Errorf("hint must name .env.secrets, not only .env: %v", err) + } +} diff --git a/internal/credmask/schemeless.go b/internal/credmask/schemeless.go new file mode 100644 index 000000000..29f70f61c --- /dev/null +++ b/internal/credmask/schemeless.go @@ -0,0 +1,108 @@ +package credmask + +// Purpose: mask credentials in values that carry no `scheme://` (D-0743): a Go +// MySQL DSN (user:pass@tcp(host:3306)/db), a bare user:pass@host target and +// libpq or ODBC keyword pairs (password=..., pwd=...). +// Inputs: any string. Outputs: the string with those credentials replaced by +// `****`. +// Constraints: never touches plain user@host or an email (no colon before the +// `@`). Linear in len(v) apart from the regexp, which is RE2. Over-masking is +// the safe side, as in config.MaskURLCredentials. + +import ( + "regexp" + "strings" +) + +// dsnProtocol matches the `@protocol(` that follows the userinfo of a Go DSN. +var dsnProtocol = regexp.MustCompile(`@[A-Za-z][A-Za-z0-9]*\(`) + +// bareUserinfo matches a whole value of the form user:pass@host[...]. +var bareUserinfo = regexp.MustCompile(`^([^\s:@/]+):\S*@([^\s@]+)$`) + +// keywordPassword matches a keyword/value password pair. The value is a +// closed single- or double-quoted string, an unclosed quote (to end of line), +// or a run up to whitespace. +var keywordPassword = regexp.MustCompile(`(?i)((?:^|[^A-Za-z0-9_])(?:ssl)?(?:password|passwd|pwd)[ \t]*=[ \t]*)('(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|['"].*|[^\s'"]\S*)`) + +// urlKeywordPassword is keywordPassword for a value that holds a URL: an +// unquoted value also ends at `&`, `;` and `#`, so the parameters around it +// (already handled by the URL rules) stay readable. +var urlKeywordPassword = regexp.MustCompile(`(?i)((?:^|[^A-Za-z0-9_])(?:ssl)?(?:password|passwd|pwd)[ \t]*=[ \t]*)('(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|['"].*|[^\s'"&;#][^\s&;#]*)`) + +// Schemeless masks the credentials config.MaskURLCredentials' `://` rule cannot +// see. +func Schemeless(v string) string { + isURL := strings.Contains(v, "://") + v = maskDSNUserinfo(v) + if m := bareUserinfo.FindStringSubmatch(v); m != nil && !isURL { + v = "****@" + m[2] + } + if strings.Contains(v, "=") { + kw := keywordPassword + if isURL { + kw = urlKeywordPassword + } + v = maskKeywordValues(kw, v) + } + return v +} + +// maskDSNUserinfo masks user:pass before each `@protocol(`. The token runs back +// to the previous whitespace or quote; a `key=` prefix on the user is kept; a +// token holding `://` was already masked as a URL and is left alone. +func maskDSNUserinfo(v string) string { + locs := dsnProtocol.FindAllStringIndex(v, -1) + if len(locs) == 0 { + return v + } + var b strings.Builder + done := 0 + // One forward pass tracks where the token holding each match starts, so a + // long run without whitespace is not rescanned per match. + pos, tokenStart := 0, 0 + for _, loc := range locs { + at := loc[0] + for ; pos < at; pos++ { + if strings.IndexByte(" \t\r\n\"'", v[pos]) >= 0 { + tokenStart = pos + 1 + } + } + if at < done { + continue + } + start := max(tokenStart, done) + token := v[start:at] + colon := strings.IndexByte(token, ':') + if colon < 0 || strings.Contains(token, "://") { + continue + } + keep := token[:strings.LastIndexByte(token[:colon], '=')+1] + b.WriteString(v[done:start]) + b.WriteString(keep) + b.WriteString("****") + done = at + } + b.WriteString(v[done:]) + return b.String() +} + +// alreadyMasked are the values an earlier pass left in place of a secret; a +// keyword pair holding one is not rewritten, so a caller's own placeholder +// (the unified-diff `[REDACTED]`, or its internal NUL-delimited sentinel) +// survives. +var alreadyMasked = map[string]bool{"****": true, "[REDACTED]": true} + +// maskKeywordValues replaces the value of every keyword pair kw finds in v +// with `****`, except a value that is already masked (quotes ignored) or holds +// a NUL byte, which no real credential does. +func maskKeywordValues(kw *regexp.Regexp, v string) string { + return kw.ReplaceAllStringFunc(v, func(m string) string { + sub := kw.FindStringSubmatch(m) + val := strings.Trim(sub[2], `'"`) + if alreadyMasked[val] || strings.Contains(val, "\x00") { + return m + } + return sub[1] + "****" + }) +} diff --git a/internal/credmask/schemeless_test.go b/internal/credmask/schemeless_test.go new file mode 100644 index 000000000..f7aa1a25f --- /dev/null +++ b/internal/credmask/schemeless_test.go @@ -0,0 +1,24 @@ +package credmask + +import "testing" + +// D-0743: the schemeless rules, directly. Fake values only. +func TestSchemeless(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + {"app:fakepw@tcp(db:3306)/app", "****@tcp(db:3306)/app"}, + {"app:fakepw@db.internal:5432/app", "****@db.internal:5432/app"}, + {"host=db password='fake pw' dbname=app", "host=db password=**** dbname=app"}, + {"app@db.internal", "app@db.internal"}, + {"password=****", "password=****"}, + } { + if got := Schemeless(tc.in); got != tc.want { + t.Errorf("Schemeless(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestURL_MasksSchemeAndSchemeless(t *testing.T) { + if got, want := URL("postgres://app:fakepw@db/app?sslpassword=fakepw2&x=1"), "postgres://****@db/app?sslpassword=****&x=1"; got != want { + t.Errorf("URL = %q, want %q", got, want) + } +} diff --git a/internal/credmask/url.go b/internal/credmask/url.go new file mode 100644 index 000000000..9c3414bdf --- /dev/null +++ b/internal/credmask/url.go @@ -0,0 +1,238 @@ +package credmask + +// Purpose: mask the credentials inside a value that holds a URL: userinfo +// before the last `@` and credential-named query, matrix and fragment +// parameters (D-0430), then the schemeless forms (Schemeless). config +// .MaskURLCredentials is the public entry; it lives here so internal/config +// stays under its package-size budget. +// Inputs: any string. Outputs: the string with credentials replaced by `****`. +// Constraints: no dependency on net/url; time linear in len(v); over-masking +// is the safe side. + +import ( + "regexp" + "strings" +) + +// urlParam is one query, matrix or fragment parameter found by urlParams: +// the name is v[nameStart:nameEnd]; the value starts at valueStart, after +// the `=` or its encoded form `%3D` (encodedEq). +type urlParam struct { + start, nameStart, nameEnd, valueStart int + encodedEq bool +} + +// urlParams finds every parameter of v: a separator (?, &, ; or #), the +// name (any bytes but &, ; and #; spaces and ? allowed) and the first `=` or +// `%3D` after it. It never consumes a value, so a parameter nested in another +// parameter's value (?redirect=https://x/?token=...) is still seen. Two +// reverse-built tables make it linear in len(v) on any input. +func urlParams(v string) []urlParam { + n := len(v) + nextEq, nextStop := make([]int, n+1), make([]int, n+1) + nextEq[n], nextStop[n] = n, n + for k := n - 1; k >= 0; k-- { + nextEq[k], nextStop[k] = nextEq[k+1], nextStop[k+1] + switch { + case v[k] == '=' || (v[k] == '%' && k+2 < n && v[k+1] == '3' && (v[k+2] == 'D' || v[k+2] == 'd')): + nextEq[k] = k + case v[k] == '&' || v[k] == ';' || v[k] == '#': + nextStop[k] = k + } + } + var out []urlParam + for k := 0; k < n; k++ { + if c := v[k]; c != '?' && c != '&' && c != ';' && c != '#' { + continue + } + eq := nextEq[k+1] + if eq >= n || nextStop[k+1] < eq { + continue + } + p := urlParam{start: k, nameStart: k + 1, nameEnd: eq, valueStart: eq + 1} + if v[eq] == '%' { + p.encodedEq, p.valueStart = true, eq+3 + } + out = append(out, p) + k = p.valueStart - 1 + } + return out +} + +// credentialWord matches a credential word anywhere in a decoded parameter +// name (access_token, client_secret, api-key, X-Amz-Signature, auth, jwt, +// jsessionid). Over-matching only masks. +var credentialWord = regexp.MustCompile(`(?i)pass|pwd|token|secret|key|signature|auth|credential|jwt|session`) + +// credentialShortName matches the short credential names that would mask +// ordinary words as substrings (signup, design, country_code), so they count +// only as the whole name: Azure SAS sig, the OAuth code, pw. +var credentialShortName = regexp.MustCompile(`(?i)^\s*(sig|code|pw)\s*$`) + +// isCredentialName reports whether a decoded parameter name names a credential. +func isCredentialName(name string) bool { + return credentialWord.MatchString(name) || credentialShortName.MatchString(name) +} + +// maxNestedChecks bounds the decode checks for parameters nested in one value; +// a value holding more is masked whole, so a hostile value cannot make the +// scan quadratic. +const maxNestedChecks = 8 + +// URL masks the credentials of a value that holds a URL. For +// `scheme://` followed later by `@`, everything between `://` and the LAST `@` +// becomes `****` (user and password), so a password that contains `@`, `/`, +// `:`, `+` or spaces never leaks and the rule does not depend on net/url +// accepting the string. Query, matrix (`;`) and fragment (`#`) parameters +// whose name is a credential (isCredentialName, any case, spaces around the +// name allowed) are masked too; the name is percent-decoded first, so +// `access%5Ftoken=`, `access_token%3D` and `#access_token=` are caught, and +// a parameter nested in another one's value is found (D-0430). A value +// whose percent-decoded form holds a credential parameter (an encoded +// redirect such as https%3A%2F%2Fx%2F%3Ftoken%3D...) is masked whole. A +// credential value runs to the next `&` or whitespace (a credential holding +// a raw `&` or space keeps the part after it); a value masked whole for an +// encoded credential runs to the next `&`, `;`, `#` or whitespace. Time is +// linear in len(v). Schemeless then masks a Go DSN, a bare user:pass@host +// and password=/pwd= pairs. The rule over-masks a path with an `@`: the safe side. +func URL(v string) string { + i := strings.Index(v, "://") + if i < 0 { + return Schemeless(v) + } + if rest := v[i+3:]; strings.Contains(rest, "@") { + v = v[:i+3] + "****" + rest[strings.LastIndex(rest, "@"):] + } + params := urlParams(v) + if len(params) == 0 { + return Schemeless(v) + } + valueEnd, credEnd := valueEnds(v) + checks := map[int]int{} // value end -> nested decode checks spent + var b strings.Builder + done := 0 // v[:done] is already written + for _, p := range params { + if p.start < done { + continue + } + end := valueEnd[p.valueStart] + if isCredentialName(decodeParamName(v[p.nameStart:p.nameEnd])) { + end = credEnd[p.valueStart] + } else { + checks[end]++ + if checks[end] <= maxNestedChecks && !hidesCredentialParam(v[p.valueStart:end], p.encodedEq) { + continue + } + } + b.WriteString(v[done:p.valueStart]) + b.WriteString("****") + done = end + } + if done == 0 { + return Schemeless(v) + } + b.WriteString(v[done:]) + return Schemeless(b.String()) +} + +// valueEnds returns, for every offset of v (and len(v)), where a parameter +// value starting there ends: valueEnd at the next `&`, `;`, `#` or +// whitespace, credEnd at the next `&` or whitespace (len(v) when none). One +// reverse pass, so lookups stay linear. +func valueEnds(v string) (valueEnd, credEnd []int) { + valueEnd, credEnd = make([]int, len(v)+1), make([]int, len(v)+1) + valueEnd[len(v)], credEnd[len(v)] = len(v), len(v) + for k := len(v) - 1; k >= 0; k-- { + valueEnd[k], credEnd[k] = valueEnd[k+1], credEnd[k+1] + switch v[k] { + case '&', ' ', '\t', '\n', '\r', '\f': + valueEnd[k], credEnd[k] = k, k + case ';', '#': + valueEnd[k] = k + } + } + return valueEnd, credEnd +} + +// hidesCredentialParam reports whether a parameter value, once +// percent-decoded, holds a parameter whose name is a credential: the +// encoded redirect case, which the raw scan cannot see. Values without an +// escape are skipped (the raw scan already covers them) unless the name +// ended in an encoded `=` (encodedSep), where the raw scan cannot restart. +func hidesCredentialParam(value string, encodedSep bool) bool { + if !encodedSep && !hasPercentEscape(value) { + return false + } + d := "?" + decodeParamName(value) + for _, p := range urlParams(d) { + if isCredentialName(d[p.nameStart:p.nameEnd]) { + return true + } + } + return false +} + +// decodeParamName percent-decodes a parameter name up to three times, so a +// double-encoded name (access%255Ftoken) is matched too. It is lenient: an +// invalid escape (%zz) stays as written and the valid ones around it still +// decode, so one bad escape cannot hide a credential word. Linear per pass. +func decodeParamName(name string) string { + for n := 0; n < 3; n++ { + d, changed := percentDecodeOnce(name) + if !changed { + break + } + name = d + } + return name +} + +// hasPercentEscape reports whether s holds a valid %XX escape. +func hasPercentEscape(s string) bool { + _, changed := percentDecodeOnce(s) + return changed +} + +// percentDecodeOnce decodes every valid %XX escape of s once and reports +// whether any was found. +func percentDecodeOnce(s string) (string, bool) { + i := strings.IndexByte(s, '%') + if i < 0 { + return s, false + } + var b strings.Builder + b.Grow(len(s)) + changed := false + last := 0 + for ; i < len(s); i++ { + if s[i] != '%' || i+2 >= len(s) { + continue + } + hi, lo := unhex(s[i+1]), unhex(s[i+2]) + if hi < 0 || lo < 0 { + continue + } + b.WriteString(s[last:i]) + b.WriteByte(byte(hi<<4 | lo)) + last, changed = i+3, true + i += 2 + } + if !changed { + return s, false + } + b.WriteString(s[last:]) + return b.String(), true +} + +// unhex returns the value of one hex digit, or -1. +func unhex(c byte) int { + switch { + case '0' <= c && c <= '9': + return int(c - '0') + case 'a' <= c && c <= 'f': + return int(c-'a') + 10 + case 'A' <= c && c <= 'F': + return int(c-'A') + 10 + } + return -1 +} diff --git a/internal/license/entitlement/write_test.go b/internal/license/entitlement/write_test.go index 009a311f1..b756d708b 100644 --- a/internal/license/entitlement/write_test.go +++ b/internal/license/entitlement/write_test.go @@ -103,6 +103,11 @@ func TestEntitlementRefreshWrites(t *testing.T) { func TestBuildE125UnderV15(t *testing.T) { compattest.Both(t, func(t *testing.T) { + // No licence key from the host: a stored or env key would renew a real JWT online. + t.Setenv("HOME", t.TempDir()) + t.Setenv("USERPROFILE", t.TempDir()) + t.Setenv("NSELF_PLUGIN_LICENSE_KEY", "") + t.Setenv("NSELF_PLUGIN_LICENSE_KEY_OWNER", "") t.Setenv("LICENSE_CACHE_PATH", filepath.Join(t.TempDir(), "missing.json")) dir, plugins := t.TempDir(), t.TempDir() installed := filepath.Join(plugins, "fixture-licensed") diff --git a/internal/license/keys_configured.go b/internal/license/keys_configured.go new file mode 100644 index 000000000..75270cb1e --- /dev/null +++ b/internal/license/keys_configured.go @@ -0,0 +1,23 @@ +package license + +import ( + "slices" + "strings" +) + +// ConfiguredKeys is every configured licence key, the owner key first, so +// coverage under any of them counts (multi-key support, as plugin install). +func ConfiguredKeys() []string { + keys := CollectLicenseKeys() + // CollectLicenseKeys reads only the primary key file; key.2 to key.10 are + // stored keys too, and a covered second key must not read as E110. + for _, k := range GetAllStoredKeys() { + if k = strings.TrimSpace(k); k != "" && !slices.Contains(keys, k) { + keys = append(keys, k) + } + } + if k := strings.TrimSpace(CollectLicenseKey()); k != "" { + keys = append([]string{k}, slices.DeleteFunc(keys, func(x string) bool { return x == k })...) + } + return keys +} diff --git a/internal/reconcile/testdata/fixtures/dev-minimal/golden-tree.txt b/internal/reconcile/testdata/fixtures/dev-minimal/golden-tree.txt index cfc44a083..4872d50e0 100644 --- a/internal/reconcile/testdata/fixtures/dev-minimal/golden-tree.txt +++ b/internal/reconcile/testdata/fixtures/dev-minimal/golden-tree.txt @@ -14,7 +14,7 @@ d 0755 ssl d 0755 ssl/certificates f 0600 .env 2b4db21db0f5f82d03afa03af77b37a14407742b3632ec4910dd70a070a709a1 f 0600 .env.computed b6f45aa109d82fcb343e8483c1100906615feaab2a5c0309e6128a5eb8fcef4c -f 0600 .nself/compose.env 95d22e89575cff1414445d6200990d097dd7c95a897b64758ca1ea6297782881 +f 0600 .nself/compose.env 6f47de432b2f2a39a2e7191cfd7398773f61c6784b5b60db20d12f9ffb174a52 f 0600 docker-compose.yml 8e65b13b46bf08acfe40a36267b5f0be02ad85d3a96d828ce20c035c3fbbb48b f 0644 .nself/build-profile a172cedcae47474b615c54d510a5d84a8dea3032e958587430b413538be3f333 f 0644 .nself/build-version 8ceaa7b1b997a4743b4d5e74b2a46af8c5ebe7765abb07eca372441aa3cbf51e diff --git a/internal/repoqa/testdata/layers.yaml b/internal/repoqa/testdata/layers.yaml index 9ab164555..3ae18cc08 100644 --- a/internal/repoqa/testdata/layers.yaml +++ b/internal/repoqa/testdata/layers.yaml @@ -36,6 +36,8 @@ rules: - {match: "schemas/**", layer: L0} # L0: shared kernel plus the P7-REG packages - {match: "internal/config/**", layer: L0} + - {match: "internal/composeenv/**", layer: L0} + - {match: "internal/credmask/**", layer: L0} - {match: "internal/errs/**", layer: L0} - {match: "internal/httptimeout/**", layer: L0} - {match: "internal/ui/**", layer: L0}