From 031db0b779e5ef4997d6888bee99779c77e2159f Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:40:03 -0400 Subject: [PATCH 1/4] feat: list the keys of one workspace configuration group Query.WorkspaceConfigurationKeys(name) and WorkspaceSecretKeys(name) return, sorted, the keys of one workspace configuration group the process received, each spelled so the per-key accessor reads it. A consumer whose group is keyed by generated names (runnable-bindings, keyed __) can now enumerate it without reading Codefly's environment variable encoding. The listing enumerates carrier names from the snapshot and the live process environment (a file carrier under the key it carries) and keeps a key only when the ordinary per-key lookup answers it non-empty. An absent group is an empty list and an error wrapping the new ErrWorkspaceConfigurationAbsent, which the per-key accessors' absent error now wraps too (message unchanged), replacing the string match isWorkspaceValueAbsent used. An unreadable delivery fails the listing with an error that is not absence. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 77a997e50d5405b598df39202974887ad8d26b7b) --- codefly.go | 30 ++++++ for.go | 85 +++++++++++++-- workspace_configuration_keys_test.go | 156 +++++++++++++++++++++++++++ 3 files changed, 262 insertions(+), 9 deletions(-) create mode 100644 workspace_configuration_keys_test.go diff --git a/codefly.go b/codefly.go index 9b832a3..dad3b2f 100644 --- a/codefly.go +++ b/codefly.go @@ -269,6 +269,36 @@ func codeflyEnvironmentVariables() []string { return append([]string(nil), environmentVariablesSnapshot...) } +// carrierNames lists the name of every carrier the lookup sources hold: the +// snapshot (process values, file-carried values and injected values) and the +// live process environment, where a file carrier is listed under the key it +// carries. A listing accessor enumerates these names and then reads each one +// through the ordinary lookup, so it never answers from a source the lookup +// would not read. +func carrierNames() []string { + names := map[string]struct{}{} + for _, entry := range codeflyEnvironmentVariables() { + name, _, _ := strings.Cut(entry, "=") + names[name] = struct{}{} + } + for _, entry := range os.Environ() { + name, _, found := strings.Cut(entry, "=") + if !found { + continue + } + if resources.IsFileCarrierKey(name) { + name = resources.CarriedKey(name) + } + names[name] = struct{}{} + } + listed := make([]string, 0, len(names)) + for name := range names { + listed = append(listed, name) + } + sort.Strings(listed) + return listed +} + func injectedEnvironmentValue(name string) (string, bool) { environmentVariablesMu.RLock() defer environmentVariablesMu.RUnlock() diff --git a/for.go b/for.go index 180c1fc..9939eb8 100644 --- a/for.go +++ b/for.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "github.com/codefly-dev/core/configurations" @@ -382,6 +383,78 @@ func (q *Query) WorkspaceSecret(name string, key string) (string, error) { return q.workspaceConfigurationValue(resources.WorkspaceSecretConfigurationPrefix, name, key) } +// ErrWorkspaceConfigurationAbsent reports that no carrier holds the workspace +// configuration asked for: a group with no key, or a key with no value. It is +// absence, an answer; a carrier that is there and cannot be read is a +// different error and never wraps it. +var ErrWorkspaceConfigurationAbsent = errors.New("no workspace configuration value") + +// WorkspaceConfigurationKeys lists, sorted, the keys of one non-secret +// workspace configuration group this process received. A group whose keys +// carry an identity the caller must discover (a group keyed by generated +// names, read value by value) is enumerated here rather than by reading +// Codefly's environment variable encoding. +// +// Each key is spelled so that WorkspaceConfiguration(name, key) reads its +// value, and only keys that read a non-empty value are listed. A group no +// carrier holds returns an empty list and an error wrapping +// ErrWorkspaceConfigurationAbsent; a key whose delivery cannot be read fails +// the whole listing, because a partial list would silently omit that key. +// +// Listing reads no authority pin: it returns names, not values. A value is +// read through the per-key accessors, which apply their own rules. +func (q *Query) WorkspaceConfigurationKeys(name string) ([]string, error) { + return q.workspaceConfigurationKeys(resources.WorkspaceConfigurationPrefix, name) +} + +// WorkspaceSecretKeys is WorkspaceConfigurationKeys for the secret namespace: +// each key it lists is read with WorkspaceSecret(name, key). +func (q *Query) WorkspaceSecretKeys(name string) ([]string, error) { + return q.workspaceConfigurationKeys(resources.WorkspaceSecretConfigurationPrefix, name) +} + +// workspaceConfigurationKeys enumerates every carrier name under the group's +// spellings (exact, then normalized, as workspaceConfigurationValue reads +// them) and keeps a key only when the per-key lookup answers it, so a listed +// key and a readable key are one fact. +func (q *Query) workspaceConfigurationKeys(prefix string, name string) ([]string, error) { + groups := []string{ + fmt.Sprintf("%s__%s__", prefix, strings.ToUpper(name)), + fmt.Sprintf("%s__%s__", prefix, normalizeWorkspaceEnvironmentKey(name)), + } + found := map[string]struct{}{} + for _, carrier := range carrierNames() { + for _, group := range groups { + key, ok := strings.CutPrefix(carrier, group) + if !ok || key == "" || key != normalizeWorkspaceEnvironmentKey(key) { + continue + } + if _, listed := found[key]; listed { + continue + } + value, err := q.workspaceConfigurationValue(prefix, name, key) + if err != nil { + if isWorkspaceValueAbsent(err) { + continue + } + return []string{}, err + } + if value != "" { + found[key] = struct{}{} + } + } + } + if len(found) == 0 { + return []string{}, fmt.Errorf("%w in group %s", ErrWorkspaceConfigurationAbsent, name) + } + keys := make([]string, 0, len(found)) + for key := range found { + keys = append(keys, key) + } + slices.Sort(keys) + return keys, nil +} + // WorkspaceValue resolves a workspace value from the public namespace first, // then the secret namespace. It is intended for settings whose sensitivity is // deployment-defined while preserving a single SDK-only lookup boundary. @@ -465,15 +538,9 @@ func (q *Query) workspaceValueIfSetLive(name string, key string) (string, bool, // isWorkspaceValueAbsent reports whether err is the "nothing carries this name" // answer rather than a failure to read something that is there. func isWorkspaceValueAbsent(err error) bool { - return err != nil && strings.Contains(err.Error(), workspaceValueAbsentReason) + return errors.Is(err, ErrWorkspaceConfigurationAbsent) } -// workspaceValueAbsentReason is the text workspaceConfigurationValue returns -// when no namespace carries the name. It is matched here because the lookup -// reports absence with a wool error rather than a sentinel; a sentinel is the -// better shape and this is where it would be read from if one lands. -const workspaceValueAbsentReason = "no workspace configuration value for" - func (q *Query) workspaceValueLive(name string, key string) (string, error) { if value, err := q.WorkspaceConfiguration(name, key); err == nil && value != "" { return value, nil @@ -481,7 +548,7 @@ func (q *Query) workspaceValueLive(name string, key string) (string, error) { if value, err := q.WorkspaceSecret(name, key); err == nil && value != "" { return value, nil } - return "", wool.Get(q.ctx).In("WorkspaceValue").NewError("no workspace configuration value for %s/%s", name, key) + return "", fmt.Errorf("%w for %s/%s", ErrWorkspaceConfigurationAbsent, name, key) } func (q *Query) workspaceConfigurationValue(prefix string, name string, key string) (string, error) { @@ -504,7 +571,7 @@ func (q *Query) workspaceConfigurationValue(prefix string, name string, key stri break } } - return "", wool.Get(q.ctx).In("WorkspaceConfiguration").NewError("no workspace configuration value for %s/%s", name, key) + return "", fmt.Errorf("%w for %s/%s", ErrWorkspaceConfigurationAbsent, name, key) } func normalizeWorkspaceEnvironmentKey(value string) string { diff --git a/workspace_configuration_keys_test.go b/workspace_configuration_keys_test.go new file mode 100644 index 0000000..4814960 --- /dev/null +++ b/workspace_configuration_keys_test.go @@ -0,0 +1,156 @@ +package codefly_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + "github.com/codefly-dev/core/resources" + codefly "github.com/codefly-dev/sdk-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// setWorkspaceCarriers sets process carriers and rebuilds the snapshot, and +// rebuilds it again once t.Setenv has restored them, so no neighbouring test +// reads a group this one left behind. +func setWorkspaceCarriers(t *testing.T, carriers map[string]string) { + t.Helper() + t.Cleanup(func() { _ = codefly.LoadEnvironmentVariables() }) + for key, value := range carriers { + t.Setenv(key, value) + } + require.NoError(t, codefly.LoadEnvironmentVariables()) +} + +// A group keyed by generated names — runnable-bindings, keyed +// __ — is enumerated through the SDK, sorted, and each key +// it lists reads its value through the per-key accessor. Neighbouring groups, +// a group whose name merely extends this one, and the secret namespace are +// not part of it. +func TestWorkspaceConfigurationKeysListsOneGroupInItsReadableSpelling(t *testing.T) { + ctx := t.Context() + setWorkspaceCarriers(t, map[string]string{ + "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_BINDINGS__DOCUMENTS__COUNT_DOCUMENTS": `{"operation":"count"}`, + "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_BINDINGS__ACCOUNTS__WHO_AM_I": `{"operation":"whoami"}`, + "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_BINDINGS__EMPTY": "", + "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_BINDINGS__lower": "not a spelling the accessor reads", + "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_BINDINGS_EXTRA__OTHER": "another group", + "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_OTHER__DOCUMENTS__COUNT_DOCUMENTS": "another group", + "CODEFLY__WORKSPACE_SECRET_CONFIGURATION__KEYS_BINDINGS__HIDDEN": "secret", + }) + + keys, err := codefly.For(ctx).WorkspaceConfigurationKeys("keys-bindings") + + require.NoError(t, err) + assert.Equal(t, []string{"ACCOUNTS__WHO_AM_I", "DOCUMENTS__COUNT_DOCUMENTS"}, keys) + value, err := codefly.For(ctx).WorkspaceConfiguration("keys-bindings", keys[1]) + require.NoError(t, err) + assert.Equal(t, `{"operation":"count"}`, value) + + secrets, err := codefly.For(ctx).WorkspaceSecretKeys("keys-bindings") + require.NoError(t, err) + assert.Equal(t, []string{"HIDDEN"}, secrets) + value, err = codefly.For(ctx).WorkspaceSecret("keys-bindings", secrets[0]) + require.NoError(t, err) + assert.Equal(t, "secret", value) +} + +// The group is read under both spellings the per-key accessor reads: the +// exact name and the '-'-normalized one. +func TestWorkspaceConfigurationKeysReadsBothGroupSpellings(t *testing.T) { + setWorkspaceCarriers(t, map[string]string{ + "CODEFLY__WORKSPACE_CONFIGURATION__KEYS-SPELLING__EXACT": "exact", + "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_SPELLING__NORMALIZED": "normalized", + }) + + keys, err := codefly.For(t.Context()).WorkspaceConfigurationKeys("keys-spelling") + + require.NoError(t, err) + assert.Equal(t, []string{"EXACT", "NORMALIZED"}, keys) +} + +// A prepared binding is large enough to be delivered by file. Its key is +// listed under the key it carries, never under the carrier's own name, and its +// value reads through the file. +func TestWorkspaceConfigurationKeysListsAFileDeliveredKey(t *testing.T) { + key := "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_FILED__DOCUMENTS__COUNT_DOCUMENTS" + content := strings.Repeat("x", resources.FileCarrierThreshold+1) + path := filepath.Join(t.TempDir(), "binding") + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + setWorkspaceCarriers(t, map[string]string{resources.FileCarrierKey(key): path}) + + keys, err := codefly.For(t.Context()).WorkspaceConfigurationKeys("keys-filed") + + require.NoError(t, err) + assert.Equal(t, []string{"DOCUMENTS__COUNT_DOCUMENTS"}, keys) + value, err := codefly.For(t.Context()).WorkspaceConfiguration("keys-filed", keys[0]) + require.NoError(t, err) + assert.Equal(t, content, value) +} + +// An embedded host injects configuration without touching the process +// environment; the listing sees what the per-key accessor sees. +func TestWorkspaceConfigurationKeysListsInjectedConfiguration(t *testing.T) { + t.Cleanup(func() { require.NoError(t, codefly.InjectConfigurations()) }) + require.NoError(t, codefly.InjectConfigurations(&basev0.Configuration{ + Origin: resources.ConfigurationWorkspace, + Infos: []*basev0.ConfigurationInformation{{ + Name: "keys-injected", + ConfigurationValues: []*basev0.ConfigurationValue{ + {Key: "documents__count-documents", Value: "count"}, + {Key: "credential", Value: "secret", Secret: true}, + }, + }}, + })) + + keys, err := codefly.For(t.Context()).WorkspaceConfigurationKeys("keys-injected") + + require.NoError(t, err) + assert.Equal(t, []string{"DOCUMENTS__COUNT_DOCUMENTS"}, keys) + secrets, err := codefly.For(t.Context()).WorkspaceSecretKeys("keys-injected") + require.NoError(t, err) + assert.Equal(t, []string{"CREDENTIAL"}, secrets) +} + +// Absence is an answer and a failed read is not: an absent group (or one +// whose only carrier is empty) is an empty list with the error the per-key +// accessor gives an absent key, and a delivery that cannot be read fails the +// listing with an error that is not absence. +func TestWorkspaceConfigurationKeysSeparatesAbsenceFromFailure(t *testing.T) { + ctx := t.Context() + + t.Run("a group nothing carries", func(t *testing.T) { + keys, err := codefly.For(ctx).WorkspaceConfigurationKeys("keys-absent") + + assert.ErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent) + assert.NotNil(t, keys) + assert.Empty(t, keys) + _, err = codefly.For(ctx).WorkspaceConfiguration("keys-absent", "any") + assert.ErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent, "one classification for an absent key and an absent group") + }) + + t.Run("a group whose only carrier is empty", func(t *testing.T) { + setWorkspaceCarriers(t, map[string]string{"CODEFLY__WORKSPACE_CONFIGURATION__KEYS_BLANK__KEY": ""}) + + keys, err := codefly.For(ctx).WorkspaceConfigurationKeys("keys-blank") + + assert.ErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent) + assert.Empty(t, keys) + }) + + t.Run("a delivery that cannot be read", func(t *testing.T) { + key := "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_UNREADABLE__BINDING" + t.Setenv("CODEFLY__WORKSPACE_CONFIGURATION__KEYS_UNREADABLE__READABLE", "value") + t.Setenv(resources.FileCarrierKey(key), filepath.Join(t.TempDir(), "absent")) + + keys, err := codefly.For(ctx).WorkspaceConfigurationKeys("keys-unreadable") + + require.Error(t, err) + assert.NotErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent, "an unreadable delivery is not absence") + assert.ErrorIs(t, err, resources.ErrFileCarrier) + assert.Empty(t, keys, "a partial list would silently omit the unreadable key") + }) +} From 1e2189c6c7b7eb42b0f504e3896dfa7116c32ee8 Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 23:36:20 -0400 Subject: [PATCH 2/4] fix: list workspace keys from delivered names, reading no value (r135) The listing imported slices, which is not on the root import allowlist, so the "no second Work Context implementation" gate, race (workcontext) and coverage were red. It now sorts with sort.Strings. WorkspaceSecretKeys also read every value in the group to decide membership, which opened every live secret file and kept its content in the file-carrier cache just to return names. The listing now matches the names the SDK already holds (the snapshot, and the live environment with a file carrier listed under the key it carries) and opens no file. A listed key whose delivery cannot be read fails where its value is read, naming its reason, instead of being dropped from the list or failing the whole listing. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 5e83bca2968b22f4322a029f49eded7737704315) --- codefly.go | 24 +++++++------- for.go | 47 +++++++++++----------------- workspace_configuration_keys_test.go | 29 +++++++++++++++-- 3 files changed, 57 insertions(+), 43 deletions(-) diff --git a/codefly.go b/codefly.go index dad3b2f..d435e9c 100644 --- a/codefly.go +++ b/codefly.go @@ -269,21 +269,23 @@ func codeflyEnvironmentVariables() []string { return append([]string(nil), environmentVariablesSnapshot...) } -// carrierNames lists the name of every carrier the lookup sources hold: the -// snapshot (process values, file-carried values and injected values) and the -// live process environment, where a file carrier is listed under the key it -// carries. A listing accessor enumerates these names and then reads each one -// through the ordinary lookup, so it never answers from a source the lookup -// would not read. -func carrierNames() []string { +// deliveredNames lists, from the carriers the SDK already holds, the name of +// every value that is delivered non-empty: the snapshot (process values, the +// values file carriers delivered at load, and injected values) and the live +// process environment, where an inline value is listed by its own name and a +// file carrier by the key it carries. It reads no file: a carrier's path is +// evidence of delivery, and its content is read only by the per-key lookup, so +// listing a secret namespace neither materialises nor caches a secret value. +func deliveredNames() []string { names := map[string]struct{}{} for _, entry := range codeflyEnvironmentVariables() { - name, _, _ := strings.Cut(entry, "=") - names[name] = struct{}{} + if name, value, found := strings.Cut(entry, "="); found && value != "" { + names[name] = struct{}{} + } } for _, entry := range os.Environ() { - name, _, found := strings.Cut(entry, "=") - if !found { + name, value, found := strings.Cut(entry, "=") + if !found || value == "" { continue } if resources.IsFileCarrierKey(name) { diff --git a/for.go b/for.go index 9939eb8..ac4bd5d 100644 --- a/for.go +++ b/for.go @@ -6,7 +6,7 @@ import ( "fmt" "os" "path/filepath" - "slices" + "sort" "strings" "github.com/codefly-dev/core/configurations" @@ -396,50 +396,39 @@ var ErrWorkspaceConfigurationAbsent = errors.New("no workspace configuration val // Codefly's environment variable encoding. // // Each key is spelled so that WorkspaceConfiguration(name, key) reads its -// value, and only keys that read a non-empty value are listed. A group no +// value, and only keys a carrier delivers non-empty are listed. A group no // carrier holds returns an empty list and an error wrapping -// ErrWorkspaceConfigurationAbsent; a key whose delivery cannot be read fails -// the whole listing, because a partial list would silently omit that key. +// ErrWorkspaceConfigurationAbsent. // -// Listing reads no authority pin: it returns names, not values. A value is -// read through the per-key accessors, which apply their own rules. +// Listing reads names, never values: no file a carrier names is opened, and +// no authority pin is read. A value is read through the per-key accessors, +// which apply their own rules, so a listed key whose delivery cannot be read +// fails there, naming its reason, rather than being dropped from the list. func (q *Query) WorkspaceConfigurationKeys(name string) ([]string, error) { - return q.workspaceConfigurationKeys(resources.WorkspaceConfigurationPrefix, name) + return workspaceConfigurationKeys(resources.WorkspaceConfigurationPrefix, name) } // WorkspaceSecretKeys is WorkspaceConfigurationKeys for the secret namespace: -// each key it lists is read with WorkspaceSecret(name, key). +// each key it lists is read with WorkspaceSecret(name, key). Listing opens no +// secret file and caches no secret value. func (q *Query) WorkspaceSecretKeys(name string) ([]string, error) { - return q.workspaceConfigurationKeys(resources.WorkspaceSecretConfigurationPrefix, name) + return workspaceConfigurationKeys(resources.WorkspaceSecretConfigurationPrefix, name) } -// workspaceConfigurationKeys enumerates every carrier name under the group's +// workspaceConfigurationKeys matches every delivered name against the group's // spellings (exact, then normalized, as workspaceConfigurationValue reads -// them) and keeps a key only when the per-key lookup answers it, so a listed -// key and a readable key are one fact. -func (q *Query) workspaceConfigurationKeys(prefix string, name string) ([]string, error) { +// them). A key is kept only in the spelling the per-key lookup builds, so the +// name it lists is exactly the carrier that lookup reads. +func workspaceConfigurationKeys(prefix string, name string) ([]string, error) { groups := []string{ fmt.Sprintf("%s__%s__", prefix, strings.ToUpper(name)), fmt.Sprintf("%s__%s__", prefix, normalizeWorkspaceEnvironmentKey(name)), } found := map[string]struct{}{} - for _, carrier := range carrierNames() { + for _, carrier := range deliveredNames() { for _, group := range groups { key, ok := strings.CutPrefix(carrier, group) - if !ok || key == "" || key != normalizeWorkspaceEnvironmentKey(key) { - continue - } - if _, listed := found[key]; listed { - continue - } - value, err := q.workspaceConfigurationValue(prefix, name, key) - if err != nil { - if isWorkspaceValueAbsent(err) { - continue - } - return []string{}, err - } - if value != "" { + if ok && key != "" && key == normalizeWorkspaceEnvironmentKey(key) { found[key] = struct{}{} } } @@ -451,7 +440,7 @@ func (q *Query) workspaceConfigurationKeys(prefix string, name string) ([]string for key := range found { keys = append(keys, key) } - slices.Sort(keys) + sort.Strings(keys) return keys, nil } diff --git a/workspace_configuration_keys_test.go b/workspace_configuration_keys_test.go index 4814960..0c410d2 100644 --- a/workspace_configuration_keys_test.go +++ b/workspace_configuration_keys_test.go @@ -115,10 +115,31 @@ func TestWorkspaceConfigurationKeysListsInjectedConfiguration(t *testing.T) { assert.Equal(t, []string{"CREDENTIAL"}, secrets) } +// Listing a secret group opens no secret file and caches no secret value: +// the value read after the listing is what the file holds then, so the listing +// did not keep a copy of it. +func TestWorkspaceSecretKeysReadsNoSecretValue(t *testing.T) { + key := "CODEFLY__WORKSPACE_SECRET_CONFIGURATION__KEYS_SECRET_FILED__CREDENTIAL" + path := filepath.Join(t.TempDir(), "credential") + require.NoError(t, os.WriteFile(path, []byte("before listing"), 0o600)) + t.Cleanup(func() { _ = codefly.LoadEnvironmentVariables() }) + t.Setenv(resources.FileCarrierKey(key), path) + + keys, err := codefly.For(t.Context()).WorkspaceSecretKeys("keys-secret-filed") + + require.NoError(t, err) + assert.Equal(t, []string{"CREDENTIAL"}, keys) + require.NoError(t, os.WriteFile(path, []byte("after listing"), 0o600)) + value, err := codefly.For(t.Context()).WorkspaceSecret("keys-secret-filed", keys[0]) + require.NoError(t, err) + assert.Equal(t, "after listing", value, "the listing must not have read or cached the secret") +} + // Absence is an answer and a failed read is not: an absent group (or one // whose only carrier is empty) is an empty list with the error the per-key -// accessor gives an absent key, and a delivery that cannot be read fails the -// listing with an error that is not absence. +// accessor gives an absent key, and a delivery that cannot be read is listed +// (never silently dropped) and fails where its value is read, with an error +// that is not absence. func TestWorkspaceConfigurationKeysSeparatesAbsenceFromFailure(t *testing.T) { ctx := t.Context() @@ -148,9 +169,11 @@ func TestWorkspaceConfigurationKeysSeparatesAbsenceFromFailure(t *testing.T) { keys, err := codefly.For(ctx).WorkspaceConfigurationKeys("keys-unreadable") + require.NoError(t, err) + assert.Equal(t, []string{"BINDING", "READABLE"}, keys, "an unreadable delivery is listed, never silently omitted") + _, err = codefly.For(ctx).WorkspaceConfiguration("keys-unreadable", "BINDING") require.Error(t, err) assert.NotErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent, "an unreadable delivery is not absence") assert.ErrorIs(t, err, resources.ErrFileCarrier) - assert.Empty(t, keys, "a partial list would silently omit the unreadable key") }) } From 3af0d1de442dca1561b87d2decef152755d01d0c Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 23:49:06 -0400 Subject: [PATCH 3/4] fix: an unreadable public value is refused, never answered from the secret namespace (r139) WorkspaceValue, ReadAuthority and Recheck read through workspaceValueLive. When the public read failed (a missing file carrier, a value delivered both inline and by file), workspaceValueLive discarded the error and returned whatever the secret namespace held under the same name. Review r139 probed this and got "secretvalue" back with a nil error. An authority read could pin, or a Recheck could pass, on a value from a delivery the host never made for that name. workspaceValueLive now goes through workspaceValueIfSetLive, which moves on to the secret namespace only when the public value is absent and returns any other failure. WorkspaceValue returns that failure as it is; ReadAuthority and Recheck wrap it under ErrAuthorityValueMissing / ErrAuthorityValueChanged with "cannot be read", so the refusal names the delivery's reason. The new test fails against the old code at the WorkspaceValue assertion. The listing doc comments now say what the code does: a live file carrier is listed by its path, so one naming an empty file is listed and then reads as absent. TestWorkspaceConfigurationKeysSeparatesAbsenceFromFailure covers that case and a value delivered both inline and by file. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit a08a110eb90d31679814325077e134207562100a) --- authority.go | 6 +++ codefly.go | 13 ++++--- file_carrier_test.go | 46 +++++++++++++++++++++++ for.go | 55 ++++++++++++++++++---------- workspace_configuration_keys_test.go | 32 ++++++++++++++++ 5 files changed, 127 insertions(+), 25 deletions(-) diff --git a/authority.go b/authority.go index bd01a4d..11cd9e9 100644 --- a/authority.go +++ b/authority.go @@ -95,6 +95,9 @@ func ReadAuthority(ctx context.Context, names ...AuthorityValueName) (*Authority continue } value, err := query.workspaceValueLive(name.Name, name.Key) + if err != nil && !isWorkspaceValueAbsent(err) { + return nil, fmt.Errorf("%w: %s cannot be read: %w", ErrAuthorityValueMissing, name, err) + } if err != nil || strings.TrimSpace(value) == "" { return nil, fmt.Errorf("%w: %s", ErrAuthorityValueMissing, name) } @@ -164,6 +167,9 @@ func (a *Authority) Recheck(ctx context.Context) error { query := For(ctx) for _, name := range a.Names() { value, err := query.workspaceValueLive(name.Name, name.Key) + if err != nil && !isWorkspaceValueAbsent(err) { + return fmt.Errorf("%w: %s cannot be read: %w", ErrAuthorityValueChanged, name, err) + } if err != nil || strings.TrimSpace(value) == "" { return fmt.Errorf("%w: %s is no longer configured", ErrAuthorityValueChanged, name) } diff --git a/codefly.go b/codefly.go index d435e9c..e479e7f 100644 --- a/codefly.go +++ b/codefly.go @@ -269,12 +269,13 @@ func codeflyEnvironmentVariables() []string { return append([]string(nil), environmentVariablesSnapshot...) } -// deliveredNames lists, from the carriers the SDK already holds, the name of -// every value that is delivered non-empty: the snapshot (process values, the -// values file carriers delivered at load, and injected values) and the live -// process environment, where an inline value is listed by its own name and a -// file carrier by the key it carries. It reads no file: a carrier's path is -// evidence of delivery, and its content is read only by the per-key lookup, so +// deliveredNames lists the name of every carrier the SDK already holds with a +// non-empty value: the snapshot (process values, the values file carriers +// delivered at load, and injected values) and the live process environment, +// where an inline value is listed by its own name and a file carrier by the +// key it carries. For a live file carrier the non-empty value is its PATH: no +// file is opened, so a carrier naming an empty file is listed, and the per-key +// lookup then answers absent. Its content is read only by that lookup, so // listing a secret namespace neither materialises nor caches a secret value. func deliveredNames() []string { names := map[string]struct{}{} diff --git a/file_carrier_test.go b/file_carrier_test.go index 91a7bfe..3051879 100644 --- a/file_carrier_test.go +++ b/file_carrier_test.go @@ -129,3 +129,49 @@ func TestAnUnreadableFileCarrierFailsClosed(t *testing.T) { require.ErrorIs(t, err, resources.ErrFileCarrier) require.Error(t, codefly.LoadEnvironmentVariables()) } + +// A public delivery that cannot be read is refused with its reason. It is +// never answered from the secret namespace under the same name: that is a +// different delivery, and answering with it would silently replace the value +// the host meant this process to read (review r139 probed exactly this and got +// "secretvalue" back with no error). The same holds where the value is read as +// authority: ReadAuthority refuses, and Recheck refuses a pinned value whose +// public delivery broke even when the secret namespace holds the pinned value. +func TestAnUnreadablePublicValueIsNeverAnsweredFromTheSecretNamespace(t *testing.T) { + ctx := context.Background() + query := codefly.For(ctx) + public := resources.WorkspaceConfigurationPrefix + "__PROBE_FB__BINDING" + t.Setenv(resources.FileCarrierKey(public), filepath.Join(t.TempDir(), "absent")) + t.Setenv(resources.WorkspaceSecretConfigurationPrefix+"__PROBE_FB__BINDING", "secretvalue") + + // The premise: the public read fails, and the secret namespace answers. + _, err := query.WorkspaceConfiguration("probe-fb", "BINDING") + require.ErrorIs(t, err, resources.ErrFileCarrier) + secret, err := query.WorkspaceSecret("probe-fb", "BINDING") + require.NoError(t, err) + require.Equal(t, "secretvalue", secret) + + value, err := query.WorkspaceValue("probe-fb", "BINDING") + require.ErrorIs(t, err, resources.ErrFileCarrier, "the refusal names the public delivery's failure") + require.NotErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent) + require.Empty(t, value) + require.NotContains(t, err.Error(), "secretvalue") + + _, err = codefly.ReadAuthority(ctx, codefly.AuthorityValueName{Name: "probe-fb", Key: "BINDING"}) + require.ErrorIs(t, err, codefly.ErrAuthorityValueMissing) + require.ErrorIs(t, err, resources.ErrFileCarrier) + + pinned := resources.WorkspaceConfigurationPrefix + "__PROBE_FB_PINNED__BINDING" + t.Setenv(pinned, "pinnedvalue") + authority, err := codefly.ReadAuthority(ctx, codefly.AuthorityValueName{Name: "probe-fb-pinned", Key: "BINDING"}) + require.NoError(t, err) + require.NoError(t, os.Unsetenv(pinned)) + t.Setenv(resources.FileCarrierKey(pinned), filepath.Join(t.TempDir(), "absent")) + t.Setenv(resources.WorkspaceSecretConfigurationPrefix+"__PROBE_FB_PINNED__BINDING", "pinnedvalue") + + err = authority.Recheck(ctx) + require.ErrorIs(t, err, codefly.ErrAuthorityValueChanged) + require.ErrorIs(t, err, resources.ErrFileCarrier) + _, err = query.WorkspaceValue("probe-fb-pinned", "BINDING") + require.ErrorIs(t, err, resources.ErrFileCarrier) +} diff --git a/for.go b/for.go index ac4bd5d..43c7293 100644 --- a/for.go +++ b/for.go @@ -396,14 +396,17 @@ var ErrWorkspaceConfigurationAbsent = errors.New("no workspace configuration val // Codefly's environment variable encoding. // // Each key is spelled so that WorkspaceConfiguration(name, key) reads its -// value, and only keys a carrier delivers non-empty are listed. A group no -// carrier holds returns an empty list and an error wrapping +// value. A key is listed when a carrier names it: an inline value that is +// non-empty, or a file carrier whose path is non-empty. A group no carrier +// holds returns an empty list and an error wrapping // ErrWorkspaceConfigurationAbsent. // // Listing reads names, never values: no file a carrier names is opened, and -// no authority pin is read. A value is read through the per-key accessors, -// which apply their own rules, so a listed key whose delivery cannot be read -// fails there, naming its reason, rather than being dropped from the list. +// no authority pin is read. A file's content is judged only by the per-key +// accessors, which apply their own rules, so a listed key can still read as +// absent (its file is empty) or fail with a named reason (its file is missing, +// or it is delivered both inline and by file). It is never dropped from the +// list. func (q *Query) WorkspaceConfigurationKeys(name string) ([]string, error) { return workspaceConfigurationKeys(resources.WorkspaceConfigurationPrefix, name) } @@ -448,6 +451,11 @@ func workspaceConfigurationKeys(prefix string, name string) ([]string, error) { // then the secret namespace. It is intended for settings whose sensitivity is // deployment-defined while preserving a single SDK-only lookup boundary. // +// Only an absent public value moves the lookup on to the secret namespace. A +// public delivery that cannot be read (a missing file, a value delivered both +// inline and by file) is refused with that reason, and the secret namespace +// is not consulted. +// // A value this process read as authority-bearing (see ReadAuthority) answers // from what was read at boot, and a live value that has since drifted is // refused with ErrAuthorityValueChanged rather than returned. Without that, a @@ -457,6 +465,9 @@ func (q *Query) WorkspaceValue(name string, key string) (string, error) { addressed := AuthorityValueName{Name: name, Key: key}.canonical() pinned, isPinned := authorityPin(addressed) value, err := q.workspaceValueLive(name, key) + if err != nil && !isWorkspaceValueAbsent(err) { + return "", err + } if !isPinned { return value, err } @@ -469,16 +480,16 @@ func (q *Query) WorkspaceValue(name string, key string) (string, error) { return pinned, nil } -// WorkspaceValueIfSet resolves a workspace value and distinguishes the three -// answers WorkspaceValue flattens into two: the value is set, the value is -// genuinely not configured, or the lookup FAILED. +// WorkspaceValueIfSet resolves a workspace value and answers absence without +// an error: the value is set, the value is genuinely not configured, or the +// lookup FAILED. // -// WorkspaceValue reports "not configured" and "the delivery could not be read" -// with the same error, because its callers want a value or nothing. A caller -// deciding a posture question cannot live with that: a setting that must be -// asserted before something is permitted has to treat an unreadable delivery, -// and an authority-bearing value that has drifted, as a refusal rather than as -// "the operator did not assert it". Absence is an answer; a failed read is not. +// WorkspaceValue answers absence with an error, because its callers want a +// value or nothing. A caller deciding a posture question needs absence as an +// answer: a setting that must be asserted before something is permitted has to +// treat an unreadable delivery, and an authority-bearing value that has +// drifted, as a refusal rather than as "the operator did not assert it". +// Absence is an answer; a failed read is not. // // So an unreadable file carrier, a value delivered both inline and by file, and // ErrAuthorityValueChanged all come back as errors here, and only a value no @@ -530,14 +541,20 @@ func isWorkspaceValueAbsent(err error) bool { return errors.Is(err, ErrWorkspaceConfigurationAbsent) } +// workspaceValueLive is workspaceValueIfSetLive with absence as an error. A +// public delivery that cannot be read is returned as the failure it is, never +// stepped over to the secret namespace: a value under the same name there is a +// different delivery, and answering with it would let an unreadable public +// value be silently replaced. func (q *Query) workspaceValueLive(name string, key string) (string, error) { - if value, err := q.WorkspaceConfiguration(name, key); err == nil && value != "" { - return value, nil + value, found, err := q.workspaceValueIfSetLive(name, key) + if err != nil { + return "", err } - if value, err := q.WorkspaceSecret(name, key); err == nil && value != "" { - return value, nil + if !found { + return "", fmt.Errorf("%w for %s/%s", ErrWorkspaceConfigurationAbsent, name, key) } - return "", fmt.Errorf("%w for %s/%s", ErrWorkspaceConfigurationAbsent, name, key) + return value, nil } func (q *Query) workspaceConfigurationValue(prefix string, name string, key string) (string, error) { diff --git a/workspace_configuration_keys_test.go b/workspace_configuration_keys_test.go index 0c410d2..49027eb 100644 --- a/workspace_configuration_keys_test.go +++ b/workspace_configuration_keys_test.go @@ -176,4 +176,36 @@ func TestWorkspaceConfigurationKeysSeparatesAbsenceFromFailure(t *testing.T) { assert.NotErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent, "an unreadable delivery is not absence") assert.ErrorIs(t, err, resources.ErrFileCarrier) }) + + t.Run("a file carrier naming an empty file", func(t *testing.T) { + key := "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_EMPTY_FILE__BINDING" + path := filepath.Join(t.TempDir(), "empty") + require.NoError(t, os.WriteFile(path, nil, 0o600)) + t.Setenv(resources.FileCarrierKey(key), path) + + keys, err := codefly.For(ctx).WorkspaceConfigurationKeys("keys-empty-file") + + require.NoError(t, err) + assert.Equal(t, []string{"BINDING"}, keys, "listing opens no file, so the carrier's path is what it sees") + _, err = codefly.For(ctx).WorkspaceConfiguration("keys-empty-file", "BINDING") + assert.ErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent, "an empty file reads as absent") + }) + + t.Run("a value delivered both inline and by file", func(t *testing.T) { + key := "CODEFLY__WORKSPACE_CONFIGURATION__KEYS_TWICE__BINDING" + path := filepath.Join(t.TempDir(), "binding") + require.NoError(t, os.WriteFile(path, []byte("by file"), 0o600)) + t.Setenv(key, "inline") + t.Setenv(resources.FileCarrierKey(key), path) + + keys, err := codefly.For(ctx).WorkspaceConfigurationKeys("keys-twice") + + require.NoError(t, err) + assert.Equal(t, []string{"BINDING"}, keys) + _, err = codefly.For(ctx).WorkspaceConfiguration("keys-twice", "BINDING") + require.Error(t, err) + assert.NotErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent, "two sources for one fact is not absence") + assert.ErrorIs(t, err, resources.ErrFileCarrier) + assert.ErrorContains(t, err, "delivered both inline and by file") + }) } From fcbc475fa8d157d8a091ec2630deda18fa1f1c14 Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte Date: Sun, 18 Oct 2026 01:15:04 -0400 Subject: [PATCH 4/4] fix: a reachability tag is not a published version; the two pseudo-versions released modules pinned from compat/v0.1.65 are retracted by name (#81) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compat/v0.1.65 branch was replaced by the tag compat-v0.1.65-pins (annotations 0.2.15, robin 0.1.23 and record-store 0.2.2 pin two of its commits and must keep building). Go reads only vX.Y.Z tags as versions, so the published-tags gate now reads only those; what that tag keeps reachable is two pseudo-versions carrying the old Work Context implementation, outside the [v0.1.51, v0.1.65] retraction — retracted here by name. Co-authored-by: Antoine Toussaint Co-authored-by: Claude Opus 5.5 (cherry picked from commit e73dfc66597512a4a24ca4869ca7e14ae65e02dc) --- go.mod | 12 +++++++++++- go.sum | 2 ++ published_tags_test.go | 13 ++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 1d9cffd..13457d4 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/codefly-dev/core v0.19.1 github.com/jackc/pgx/v5 v5.10.0 github.com/stretchr/testify v1.12.1 + golang.org/x/mod v0.42.0 google.golang.org/grpc v1.84.0 google.golang.org/protobuf v1.36.12 ) @@ -103,4 +104,13 @@ require ( // retraction takes effect only once a NEW version is tagged carrying this // go.mod, and nothing here deletes anything from the proxy, because nothing // can. -retract [v0.1.51, v0.1.65] // a second Work Context implementation at the root +retract ( + v0.1.66-0.20260917202218-97dfea9e20d8 // a second Work Context implementation, pinned from compat/v0.1.65 + [v0.1.51, v0.1.65] // a second Work Context implementation at the root + // Two pseudo-versions released modules pinned from the compat/v0.1.65 branch + // (annotations 0.2.15, robin 0.1.23, record-store 0.2.2): they carry the same + // implementation and sort outside the range above (a prerelease of v0.1.51, and + // one of v0.1.66). The branch is gone; the tag compat-v0.1.65-pins keeps them + // reachable so those releases still build, retracted so nothing new picks them. + v0.1.51-0.20260927165945-8157d3abecb2 // a second Work Context implementation, pinned from compat/v0.1.65 +) diff --git a/go.sum b/go.sum index 29d57e6..85d8749 100644 --- a/go.sum +++ b/go.sum @@ -236,6 +236,8 @@ go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= golang.org/x/exp v0.0.0-20260908205506-85c1c2202aba h1:Ck8QetSgk912qxWLMCKxd0in+aiyBQyDSMae6e/xmpU= golang.org/x/exp v0.0.0-20260908205506-85c1c2202aba/go.mod h1:50RgIsmK7OwqzTTeqcSXQW8SswW0o8fRcDxmqGluJ8E= +golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg= +golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs= golang.org/x/net v0.60.0 h1:79p50tfZlm0J9YfoDsSi639qSXNGVwEzOPLCxM2FsYU= golang.org/x/net v0.60.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg= golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= diff --git a/published_tags_test.go b/published_tags_test.go index 1032da2..2c669e8 100644 --- a/published_tags_test.go +++ b/published_tags_test.go @@ -3,6 +3,7 @@ package codefly_test import ( "encoding/json" "fmt" + "golang.org/x/mod/semver" "os" "os/exec" "path/filepath" @@ -202,7 +203,11 @@ func localTags(t *testing.T) []string { require.NoError(t, err) var tags []string for _, line := range strings.Split(strings.TrimSpace(string(output)), "\n") { - if line != "" { + // A VERSION tag is a published version; any other tag is not one. Go reads only + // `vX.Y.Z` (and a module subdirectory prefix) as a version, so a reachability + // tag such as `compat-v0.1.65-pins` publishes no version of its own — what it + // keeps reachable is pseudo-versions, which go.mod retracts by name below. + if line != "" && semver.IsValid(strings.TrimPrefix(line, "workcontext/")) { tags = append(tags, line) } } @@ -244,6 +249,12 @@ func retractedVersions(t *testing.T, dir string) map[string]bool { covered := map[string]bool{} for _, tag := range localTags(t) { for _, span := range spans { + // A single retracted PSEUDO-version (a commit a release pinned, never a + // tag) cannot be any tag: tags are plain releases here, and the ordering + // below refuses a prerelease by design. It is what go.mod retracts by name. + if span.Low == span.High && semver.Prerelease(span.Low) != "" { + continue + } // `go mod edit -json` writes a single version as Low == High, so // one comparison covers both shapes. if semverAtMost(t, span.Low, tag) && semverAtMost(t, tag, span.High) {