Detach callback values before publishing configuration
This commit is contained in:
52
callback_ownership_test.go
Normal file
52
callback_ownership_test.go
Normal file
@@ -0,0 +1,52 @@
|
|||||||
|
package configkit
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
func TestCallbackAssignmentsAreDetachedAcrossLoads(t *testing.T) {
|
||||||
|
type object struct{ Value string }
|
||||||
|
type cfg struct {
|
||||||
|
Map map[string][]*object
|
||||||
|
Pointer *object
|
||||||
|
Secret Secret
|
||||||
|
}
|
||||||
|
for _, validatorAssignment := range []bool{false, true} {
|
||||||
|
name := "resolver"
|
||||||
|
if validatorAssignment {
|
||||||
|
name = "validator"
|
||||||
|
}
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
external := &object{Value: "original"}
|
||||||
|
shared := map[string][]*object{"key": {external}}
|
||||||
|
definition := Definition[cfg]{Resolve: func(c *cfg, r *Resolution) error {
|
||||||
|
Normalize(r, Keys("map"), &c.Map, shared, "populate")
|
||||||
|
c.Pointer = external
|
||||||
|
r.Record(Keys("pointer"), ChangeNormalization, "populate")
|
||||||
|
c.Secret = SecretEnv("CONFIGKIT_CALLBACK_SECRET")
|
||||||
|
r.Record(Keys("secret"), ChangeNormalization, "reference")
|
||||||
|
return nil
|
||||||
|
}}
|
||||||
|
if validatorAssignment {
|
||||||
|
definition.Validate = func(c *cfg, v *Validation) { c.Map = shared; c.Pointer = external }
|
||||||
|
}
|
||||||
|
calls := 0
|
||||||
|
lookup := func(string) (string, bool) { calls++; return "credential", true }
|
||||||
|
first, err := loadWithSecretLookup(NoSource(), lookup, Options[cfg]{Definition: definition})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
second, err := loadWithSecretLookup(NoSource(), lookup, Options[cfg]{Definition: definition})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
first.Config.Map["key"][0].Value = "changed"
|
||||||
|
first.Config.Pointer.Value = "changed pointer"
|
||||||
|
external.Value = "external change"
|
||||||
|
if second.Config.Map["key"][0].Value != "original" || second.Config.Pointer.Value != "original" {
|
||||||
|
t.Fatal("published configuration shares callback state")
|
||||||
|
}
|
||||||
|
if second.Config.Secret.Value() != "credential" || !second.Config.Secret.IsResolved() || calls != 2 {
|
||||||
|
t.Fatal("publication changed secret state or repeated environment lookup")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -34,7 +34,10 @@ root `T` must satisfy the application schema contract below. With no options it
|
|||||||
starts from the Go zero value. A `Definition[T].Defaults` factory, when supplied,
|
starts from the Go zero value. A `Definition[T].Defaults` factory, when supplied,
|
||||||
runs exactly once after successful source parsing, schema construction, optional
|
runs exactly once after successful source parsing, schema construction, optional
|
||||||
schema-version checking, and selected-profile loading. Its result is detached before
|
schema-version checking, and selected-profile loading. Its result is detached before
|
||||||
input is applied.
|
input is applied. Resolver-provided mutable values are detached before secret lookup
|
||||||
|
and validation, and a final detached copy prevents equal-valued aliases introduced
|
||||||
|
by validators from escaping into the returned configuration. The publication copy
|
||||||
|
preserves already validated secret state without rereading the environment.
|
||||||
|
|
||||||
The current lifecycle is:
|
The current lifecycle is:
|
||||||
|
|
||||||
|
|||||||
@@ -76,6 +76,11 @@ regular-file, permitted symlink, containment, and permission tests passing.
|
|||||||
|
|
||||||
## 3. P1 — Detach mutable values introduced by resolution before publication
|
## 3. P1 — Detach mutable values introduced by resolution before publication
|
||||||
|
|
||||||
|
Status: complete. Added post-resolution and publication ownership copies; the latter
|
||||||
|
preserves already validated secret state. Regressions cover nested maps/slices,
|
||||||
|
pointers, direct/helper assignments, validator aliases, and one lookup per load.
|
||||||
|
Full tests pass with the issue-8 TMPDIR workaround; vet, build and whitespace pass.
|
||||||
|
|
||||||
Evidence: [resolution.go](../../resolution.go), `Default`/`Normalize`, directly
|
Evidence: [resolution.go](../../resolution.go), `Default`/`Normalize`, directly
|
||||||
assign supplied values; `runResolution` projects them but does not clone the typed
|
assign supplied values; `runResolution` projects them but does not clone the typed
|
||||||
object. [load.go](../../load.go) publishes that same object in `Result.Config`.
|
object. [load.go](../../load.go) publishes that same object in `Result.Config`.
|
||||||
|
|||||||
16
load.go
16
load.go
@@ -252,6 +252,14 @@ func loadWithSecretLookup[T any](source Source, lookup secretLookup, supplied ..
|
|||||||
accumulator.add(Diagnostic{Code: CodeExplainLimitExceeded, Severity: SeverityError, Message: "retained configuration history exceeds the node limit"})
|
accumulator.add(Diagnostic{Code: CodeExplainLimitExceeded, Severity: SeverityError, Message: "retained configuration history exceeds the node limit"})
|
||||||
return nil, accumulator.loadError()
|
return nil, accumulator.loadError()
|
||||||
}
|
}
|
||||||
|
// Resolver assignments may borrow application-owned mutable values. Detach
|
||||||
|
// before secret resolution mutates reference fields or validation sees them.
|
||||||
|
detached, ownershipIssue := cloneConfiguration(schema, reflect.ValueOf(configuration), defaultProjectionLimits())
|
||||||
|
if ownershipIssue != nil {
|
||||||
|
accumulator.add(diagnosticFromDecodeIssue(decodeIssueFromProjection(ownershipIssue)))
|
||||||
|
return nil, accumulator.loadError()
|
||||||
|
}
|
||||||
|
configuration = detached.Interface().(T)
|
||||||
secretResolution := resolveSecretReferences(reflect.ValueOf(&configuration).Elem(), schema, inputDocument, lookup)
|
secretResolution := resolveSecretReferences(reflect.ValueOf(&configuration).Elem(), schema, inputDocument, lookup)
|
||||||
accumulator.add(secretResolution.diagnostics...)
|
accumulator.add(secretResolution.diagnostics...)
|
||||||
if hasErrors(secretResolution.diagnostics) {
|
if hasErrors(secretResolution.diagnostics) {
|
||||||
@@ -264,6 +272,14 @@ func loadWithSecretLookup[T any](source Source, lookup secretLookup, supplied ..
|
|||||||
return nil, accumulator.loadError()
|
return nil, accumulator.loadError()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// A read-only validator can replace a value with an equal borrowed object.
|
||||||
|
// Projected mutation checks cannot detect that alias, so detach at publication.
|
||||||
|
published, ownershipIssue := cloneConfigurationState(schema, reflect.ValueOf(configuration), defaultProjectionLimits(), true)
|
||||||
|
if ownershipIssue != nil {
|
||||||
|
accumulator.add(diagnosticFromDecodeIssue(decodeIssueFromProjection(ownershipIssue)))
|
||||||
|
return nil, accumulator.loadError()
|
||||||
|
}
|
||||||
|
configuration = published.Interface().(T)
|
||||||
secrets := classifySecretPaths(schema, options.Definition.Secret, metadata, final, applications, decoded.baseline, decoded.final)
|
secrets := classifySecretPaths(schema, options.Definition.Secret, metadata, final, applications, decoded.baseline, decoded.final)
|
||||||
return &Result[T]{
|
return &Result[T]{
|
||||||
Config: configuration,
|
Config: configuration,
|
||||||
|
|||||||
@@ -34,10 +34,11 @@ type valueVisit struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
type valueOperation struct {
|
type valueOperation struct {
|
||||||
schema *schemaDescriptor
|
preserveSecrets bool
|
||||||
limits projectionLimits
|
schema *schemaDescriptor
|
||||||
active map[valueVisit]bool
|
limits projectionLimits
|
||||||
nodes int
|
active map[valueVisit]bool
|
||||||
|
nodes int
|
||||||
}
|
}
|
||||||
|
|
||||||
func projectConfiguration(schema *schemaDescriptor, value reflect.Value, limits projectionLimits) (*document.Node, *projectionIssue) {
|
func projectConfiguration(schema *schemaDescriptor, value reflect.Value, limits projectionLimits) (*document.Node, *projectionIssue) {
|
||||||
@@ -49,10 +50,15 @@ func projectConfiguration(schema *schemaDescriptor, value reflect.Value, limits
|
|||||||
}
|
}
|
||||||
|
|
||||||
func cloneConfiguration(schema *schemaDescriptor, value reflect.Value, limits projectionLimits) (reflect.Value, *projectionIssue) {
|
func cloneConfiguration(schema *schemaDescriptor, value reflect.Value, limits projectionLimits) (reflect.Value, *projectionIssue) {
|
||||||
|
return cloneConfigurationState(schema, value, limits, false)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Publication copies preserve only already-validated private reference state.
|
||||||
|
func cloneConfigurationState(schema *schemaDescriptor, value reflect.Value, limits projectionLimits, preserveSecrets bool) (reflect.Value, *projectionIssue) {
|
||||||
if schema == nil || schema.root == nil || !value.IsValid() || value.Type() != schema.root.typeOf {
|
if schema == nil || schema.root == nil || !value.IsValid() || value.Type() != schema.root.typeOf {
|
||||||
return reflect.Value{}, &projectionIssue{code: CodeSchemaUnsupportedType, message: "configuration value does not match its schema"}
|
return reflect.Value{}, &projectionIssue{code: CodeSchemaUnsupportedType, message: "configuration value does not match its schema"}
|
||||||
}
|
}
|
||||||
operation := valueOperation{schema: schema, limits: limits, active: make(map[valueVisit]bool)}
|
operation := valueOperation{schema: schema, limits: limits, active: make(map[valueVisit]bool), preserveSecrets: preserveSecrets}
|
||||||
return operation.clone(value, schema.root, Path{}, 0)
|
return operation.clone(value, schema.root, Path{}, 0)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -354,7 +360,9 @@ func (o *valueOperation) clone(value reflect.Value, descriptor *typeDescriptor,
|
|||||||
case descriptorSecret:
|
case descriptorSecret:
|
||||||
result := reflect.New(value.Type()).Elem()
|
result := reflect.New(value.Type()).Elem()
|
||||||
reference := value.Interface().(Secret)
|
reference := value.Interface().(Secret)
|
||||||
if reference.specified {
|
if o.preserveSecrets {
|
||||||
|
result.Set(value)
|
||||||
|
} else if reference.specified {
|
||||||
result.Set(reflect.ValueOf(SecretEnv(reference.EnvName())))
|
result.Set(reflect.ValueOf(SecretEnv(reference.EnvName())))
|
||||||
}
|
}
|
||||||
return result, nil
|
return result, nil
|
||||||
|
|||||||
Reference in New Issue
Block a user