diff --git a/.claude/skills/runtime-carrier-accessor/SKILL.md b/.claude/skills/runtime-carrier-accessor/SKILL.md index b30212e..33e3ba0 100644 --- a/.claude/skills/runtime-carrier-accessor/SKILL.md +++ b/.claude/skills/runtime-carrier-accessor/SKILL.md @@ -47,6 +47,12 @@ because a composition templating an unset variable ships the name with an empty value; treating that as present would shadow the fallback a caller relies on. New accessors do the same. +**An empty delivered file is not absent; it is broken.** A file carrier exists +only for a value above `resources.FileCarrierThreshold`, so `readFileCarrier` +refuses an empty file with `resources.ErrFileCarrier`, like a missing one. Read +file carriers through `processValue` and you inherit that; never read the file +yourself. + **Two spellings, exact first.** Capability and configuration names historically preserved `-` while newer emitters normalize it to `_`. `For`'s accessors try the canonical key, then the legacy one, so an SDK user is independent of the diff --git a/AGENTS.md b/AGENTS.md index dcf92ad..2d16ce1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -125,9 +125,9 @@ see the skill below. rebuilds it; a test that sets a carrier and forgets the reload asserts against the previous snapshot and passes for the wrong reason. Tests are `package codefly_test` and use `t.Setenv`. -- **An empty injected value is not a value.** A composition templating an unset - variable ships the name with an empty string; `RuntimeValue` reports `false` - so it cannot shadow the configuration a caller falls back to. Keep that. +- **An empty injected value is absent** (an unset template variable; it must not + shadow a fallback). An empty delivered *file*, and a name in both workspace + namespaces, are refused instead ([README](README.md#workspace-values)). - **The Work Context has exactly one implementation and it is not here.** `core/workcontext` signs, verifies, and answers the *structural* question through `Decode`. Nothing here may sign, check a signature, encode a diff --git a/README.md b/README.md index 50aa8ac..8e58c31 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,27 @@ is the authority for these values, and a value that drifts under a running process is an error rather than a reload — the process has already minted a credential sealed to the old one. `For(ctx).WorkspaceValue` answers from that boot read for a pinned name, so the ordinary accessor is not a way around it. +A pinned value whose live delivery breaks is refused with +`ErrAuthorityValueChanged` wrapping the reason, by `WorkspaceValue`, +`WorkspaceValueIfSet` and `Authority.Recheck` alike. + +## Workspace values + +`WorkspaceValue` and `WorkspaceValueIfSet` read a name that may sit in either +the public or the secret workspace namespace, and it must sit in exactly one: + +- a name carried in **both** namespaces is refused with + `ErrWorkspaceValueInBothNamespaces`, as two sources for one value, the same + way a value delivered both inline and by file is refused; +- a delivery that **cannot be read** (a missing file, an empty file, a value + delivered both inline and by file) is refused with that reason + (`resources.ErrFileCarrier`). The other namespace never answers in its place. + +An **empty delivered file** is a broken delivery, not absence. Codefly +delivers a value by file only when it is too large for the process +environment, so an empty file is a truncated write or a mount race. Every +accessor refuses it, and so does `LoadEnvironmentVariables`. An empty +*inline* value is still absent. ## Tenant-scoped effect receipts diff --git a/codefly.go b/codefly.go index e479e7f..f7c84aa 100644 --- a/codefly.go +++ b/codefly.go @@ -275,8 +275,9 @@ func codeflyEnvironmentVariables() []string { // 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. +// lookup then refuses it as a broken delivery. 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{}{} for _, entry := range codeflyEnvironmentVariables() { diff --git a/docs/configuration-documents.md b/docs/configuration-documents.md index 0c86e19..94c9d70 100644 --- a/docs/configuration-documents.md +++ b/docs/configuration-documents.md @@ -18,7 +18,9 @@ For typed decoding, use `DecodeConfigurationDocument(name, &destination)`, instead of rounding large integers through `float64`. Lookups use the injected configuration snapshot, then the process environment, -then the SDK environment snapshot. Empty carriers are absent. Outcomes are +then the SDK environment snapshot. An empty inline carrier is absent. A file +carrier naming an empty or missing file is a broken delivery and reads as +`ErrConfigurationDocumentUnreadable`, never as missing. Outcomes are distinguishable with `errors.Is`: | Outcome | Sentinel | diff --git a/file_carrier.go b/file_carrier.go index 596fe61..3612dfb 100644 --- a/file_carrier.go +++ b/file_carrier.go @@ -27,8 +27,8 @@ var ( // processValue is key's value as the process received it: inline in its // environment, or in the file its carrier names. An inline value and a // carrier for the same key are two sources for one fact and are refused, and -// a carrier that cannot be read is an error, never an absent value: an -// unreadable credential must not read as an unset one. +// a carrier that cannot be read, or whose file is empty, is an error, never an +// absent value: an unreadable credential must not read as an unset one. func processValue(key string) (string, bool, error) { value, inline := os.LookupEnv(key) path, carried := os.LookupEnv(resources.FileCarrierKey(key)) @@ -42,10 +42,17 @@ func processValue(key string) (string, bool, error) { if err != nil { return "", false, err } - return content, content != "", nil + return content, true, nil } // readFileCarrier reads key's file once and keeps it. +// +// An empty file is refused like a missing one. Codefly delivers a value by +// file only when it is too large for the process environment (core's +// resources.FileCarrierThreshold), so a carrier naming an empty file is a +// truncated write or a mount race, never a value the host chose to leave +// unset. Reading it as absent would let a lookup step past a broken delivery +// to another source. func readFileCarrier(key, path string) (string, error) { fileCarrierMu.Lock() defer fileCarrierMu.Unlock() @@ -57,6 +64,9 @@ func readFileCarrier(key, path string) (string, error) { if err != nil { return "", err } + if content == "" { + return "", fmt.Errorf("%w: %s: delivered file is empty", resources.ErrFileCarrier, key) + } fileCarrierValues[cacheKey] = content return content, nil } diff --git a/file_carrier_test.go b/file_carrier_test.go index 3051879..7728e62 100644 --- a/file_carrier_test.go +++ b/file_carrier_test.go @@ -172,6 +172,96 @@ func TestAnUnreadablePublicValueIsNeverAnsweredFromTheSecretNamespace(t *testing err = authority.Recheck(ctx) require.ErrorIs(t, err, codefly.ErrAuthorityValueChanged) require.ErrorIs(t, err, resources.ErrFileCarrier) + // The ordinary accessors report a pinned value's broken delivery exactly as + // Recheck does: ErrAuthorityValueChanged, wrapping the reason. _, err = query.WorkspaceValue("probe-fb-pinned", "BINDING") + require.ErrorIs(t, err, codefly.ErrAuthorityValueChanged) + require.ErrorIs(t, err, resources.ErrFileCarrier) + _, found, err := query.WorkspaceValueIfSet("probe-fb-pinned", "BINDING") + require.ErrorIs(t, err, codefly.ErrAuthorityValueChanged) + require.ErrorIs(t, err, resources.ErrFileCarrier) + require.False(t, found) + require.NotContains(t, err.Error(), "pinnedvalue") +} + +// A file carrier exists only for a value too large for the environment, so a +// carrier naming an empty file is a broken delivery (a truncated write, a mount +// race), refused like a missing file. It never reads as absent, so the secret +// namespace cannot answer in its place (review r144 probed this and got "sec1" +// back with no error), and the process refuses to load it. +func TestAnEmptyDeliveredFileIsABrokenDelivery(t *testing.T) { + ctx := context.Background() + query := codefly.For(ctx) + public := resources.WorkspaceConfigurationPrefix + "__PROBE_EMPTY__K" + path := filepath.Join(t.TempDir(), "empty") + require.NoError(t, os.WriteFile(path, nil, 0o600)) + t.Cleanup(func() { _ = codefly.LoadEnvironmentVariables() }) + t.Setenv(resources.FileCarrierKey(public), path) + t.Setenv(resources.WorkspaceSecretConfigurationPrefix+"__PROBE_EMPTY__K", "sec1") + + _, err := query.WorkspaceConfiguration("probe-empty", "K") require.ErrorIs(t, err, resources.ErrFileCarrier) + require.ErrorContains(t, err, "delivered file is empty") + require.NotErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent) + + value, err := query.WorkspaceValue("probe-empty", "K") + require.ErrorIs(t, err, resources.ErrFileCarrier) + require.Empty(t, value) + _, found, err := query.WorkspaceValueIfSet("probe-empty", "K") + require.ErrorIs(t, err, resources.ErrFileCarrier) + require.False(t, found) + _, err = codefly.ReadAuthority(ctx, codefly.AuthorityValueName{Name: "probe-empty", Key: "K"}) + require.ErrorIs(t, err, codefly.ErrAuthorityValueMissing) + require.ErrorIs(t, err, resources.ErrFileCarrier) + + require.ErrorIs(t, codefly.LoadEnvironmentVariables(), resources.ErrFileCarrier) +} + +// A name carried in both the public and the secret namespace is two sources +// for one value, refused like a value delivered both inline and by file. Review +// r144 probed public "pub" plus secret "sec" and got "pub" back with no error, +// so a value moved to the secret namespace with a stale public copy left behind +// kept answering the stale copy. The per-namespace accessors still answer: each +// asks one namespace by name. +func TestAValueInBothNamespacesIsRefused(t *testing.T) { + ctx := context.Background() + query := codefly.For(ctx) + t.Setenv(resources.WorkspaceConfigurationPrefix+"__PROBE_BOTH__K", "pub") + t.Setenv(resources.WorkspaceSecretConfigurationPrefix+"__PROBE_BOTH__K", "sec") + + value, err := query.WorkspaceValue("probe-both", "K") + require.ErrorIs(t, err, codefly.ErrWorkspaceValueInBothNamespaces) + require.NotErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent) + require.Empty(t, value) + require.NotContains(t, err.Error(), "pub\"") + require.NotContains(t, err.Error(), "sec\"") + require.ErrorContains(t, err, "probe-both/K") + + _, found, err := query.WorkspaceValueIfSet("probe-both", "K") + require.ErrorIs(t, err, codefly.ErrWorkspaceValueInBothNamespaces) + require.False(t, found) + + _, err = codefly.ReadAuthority(ctx, codefly.AuthorityValueName{Name: "probe-both", Key: "K"}) + require.ErrorIs(t, err, codefly.ErrAuthorityValueMissing) + require.ErrorIs(t, err, codefly.ErrWorkspaceValueInBothNamespaces) + + public, err := query.WorkspaceConfiguration("probe-both", "K") + require.NoError(t, err) + require.Equal(t, "pub", public) + secret, err := query.WorkspaceSecret("probe-both", "K") + require.NoError(t, err) + require.Equal(t, "sec", secret) + + // A value pinned from one namespace whose name then appears in the other + // is refused as a changed authority, by Recheck and by the accessor alike. + t.Setenv(resources.WorkspaceSecretConfigurationPrefix+"__PROBE_BOTH_PINNED__K", "sec") + authority, err := codefly.ReadAuthority(ctx, codefly.AuthorityValueName{Name: "probe-both-pinned", Key: "K"}) + require.NoError(t, err) + t.Setenv(resources.WorkspaceConfigurationPrefix+"__PROBE_BOTH_PINNED__K", "sec") + err = authority.Recheck(ctx) + require.ErrorIs(t, err, codefly.ErrAuthorityValueChanged) + require.ErrorIs(t, err, codefly.ErrWorkspaceValueInBothNamespaces) + _, err = query.WorkspaceValue("probe-both-pinned", "K") + require.ErrorIs(t, err, codefly.ErrAuthorityValueChanged) + require.ErrorIs(t, err, codefly.ErrWorkspaceValueInBothNamespaces) } diff --git a/for.go b/for.go index 43c7293..e7afe2c 100644 --- a/for.go +++ b/for.go @@ -403,10 +403,9 @@ var ErrWorkspaceConfigurationAbsent = errors.New("no workspace configuration val // // Listing reads names, never values: no file a carrier names is opened, and // 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. +// accessors, which apply their own rules, so a listed key can still fail with +// a named reason (its file is missing or empty, 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) } @@ -451,22 +450,25 @@ 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 lives in exactly one namespace. A public delivery that cannot be read +// (a missing or empty file, a value delivered both inline and by file) is +// refused with that reason, and the secret namespace does not answer in its +// place. A name carried in both namespaces is refused with +// ErrWorkspaceValueInBothNamespaces: two sources for one value, neither of +// which can be preferred silently. // // 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 -// process could seal its credential to the boot value and then authorize -// against a different one through the ordinary accessor. +// from what was read at boot. A live value that has since drifted, been +// withdrawn, or become unreadable is refused with ErrAuthorityValueChanged +// (wrapping the read failure, if there was one) rather than returned. Without +// that, a process could seal its credential to the boot value and then +// authorize against a different one through the ordinary accessor. 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 + return "", pinnedReadFailure(addressed, isPinned, err) } if !isPinned { return value, err @@ -491,15 +493,17 @@ func (q *Query) WorkspaceValue(name string, key string) (string, error) { // 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 -// namespace carries comes back as (", false, nil"). +// So an unreadable or empty file carrier, a value delivered both inline and by +// file, a name carried in both namespaces, and ErrAuthorityValueChanged all +// come back as errors here, and only a value no namespace carries comes back +// as ("", false, nil). For a pinned name, a read failure is +// ErrAuthorityValueChanged wrapping it, as Authority.Recheck reports it. func (q *Query) WorkspaceValueIfSet(name string, key string) (string, bool, error) { addressed := AuthorityValueName{Name: name, Key: key}.canonical() pinned, isPinned := authorityPin(addressed) value, found, err := q.workspaceValueIfSetLive(name, key) if err != nil { - return "", false, err + return "", false, pinnedReadFailure(addressed, isPinned, err) } if !isPinned { return value, found, nil @@ -513,28 +517,63 @@ func (q *Query) WorkspaceValueIfSet(name string, key string) (string, bool, erro return pinned, true, nil } -// workspaceValueIfSetLive reads the public namespace and then the secret one, -// returning a lookup failure rather than continuing past it. Continuing is what -// makes an unreadable value indistinguishable from an absent one. +// ErrWorkspaceValueInBothNamespaces reports a workspace value carried in both +// the public and the secret namespace under one name. They are two sources for +// one value, like a value delivered both inline and by file, and the SDK +// refuses rather than choose: preferring either would let a stale copy left in +// the other answer, with nothing reported. +var ErrWorkspaceValueInBothNamespaces = errors.New("workspace value is carried in both the public and the secret namespace") + +// workspaceValueIfSetLive reads the public and the secret namespace and +// answers from the one that carries the value. A lookup failure in either is +// returned rather than continued past, because continuing is what makes an +// unreadable value indistinguishable from an absent one, and a value carried +// in both is refused. func (q *Query) workspaceValueIfSetLive(name string, key string) (string, bool, error) { - for _, prefix := range []string{ - resources.WorkspaceConfigurationPrefix, - resources.WorkspaceSecretConfigurationPrefix, - } { - value, err := q.workspaceConfigurationValue(prefix, name, key) - if err != nil { - if isWorkspaceValueAbsent(err) { - continue - } - return "", false, err - } - if value != "" { - return value, true, nil - } + public, publicFound, err := q.workspaceNamespaceValue(resources.WorkspaceConfigurationPrefix, name, key) + if err != nil { + return "", false, err + } + secret, secretFound, err := q.workspaceNamespaceValue(resources.WorkspaceSecretConfigurationPrefix, name, key) + if err != nil { + return "", false, err + } + switch { + case publicFound && secretFound: + return "", false, fmt.Errorf("%w: %s/%s", ErrWorkspaceValueInBothNamespaces, name, key) + case publicFound: + return public, true, nil + case secretFound: + return secret, true, nil } return "", false, nil } +// workspaceNamespaceValue is one namespace's answer with absence as a result +// rather than an error. +func (q *Query) workspaceNamespaceValue(prefix string, name string, key string) (string, bool, error) { + value, err := q.workspaceConfigurationValue(prefix, name, key) + if err != nil { + if isWorkspaceValueAbsent(err) { + return "", false, nil + } + return "", false, err + } + return value, value != "", nil +} + +// pinnedReadFailure is what a failed live read means for name: for a value +// pinned as authority it is ErrAuthorityValueChanged wrapping the failure, as +// Authority.Recheck reports it, so a caller that stops or re-seals on that +// sentinel sees a broken delivery of a pinned value; otherwise it is the +// failure itself. +func pinnedReadFailure(name AuthorityValueName, isPinned bool, err error) error { + if !isPinned { + return err + } + return fmt.Errorf("%w: %s cannot be read: %w", ErrAuthorityValueChanged, name, err) +} + // 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 { @@ -542,10 +581,10 @@ func isWorkspaceValueAbsent(err error) bool { } // 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. +// delivery that cannot be read is returned as the failure it is, never +// stepped over to the other namespace: a value under the same name there is a +// different delivery, and answering with it would let an unreadable value be +// silently replaced. func (q *Query) workspaceValueLive(name string, key string) (string, error) { value, found, err := q.workspaceValueIfSetLive(name, key) if err != nil { diff --git a/workspace_configuration_keys_test.go b/workspace_configuration_keys_test.go index 49027eb..728d276 100644 --- a/workspace_configuration_keys_test.go +++ b/workspace_configuration_keys_test.go @@ -188,7 +188,10 @@ func TestWorkspaceConfigurationKeysSeparatesAbsenceFromFailure(t *testing.T) { 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") + require.Error(t, err) + assert.NotErrorIs(t, err, codefly.ErrWorkspaceConfigurationAbsent, "an empty delivered file is a broken delivery, not absence") + assert.ErrorIs(t, err, resources.ErrFileCarrier) + assert.ErrorContains(t, err, "delivered file is empty") }) t.Run("a value delivered both inline and by file", func(t *testing.T) {