From 6a1b41b4a4fa8d73d026c054fc29be797286851f Mon Sep 17 00:00:00 2001 From: zack-nova <226822369+zack-nova@users.noreply.github.com> Date: Tue, 12 May 2026 21:32:45 +0800 Subject: [PATCH] Enforce sensitive package variable policy Reject sensitive Package Variable defaults across template manifests, install snapshots, declaration merging, and runtime resolution. Share sensitive value redaction for vars explain output and cover doctor diagnostics against inline sensitive value leaks. Validation: mise run fix; mise run ci; sh ./scripts/test_release_surface_hyard.sh --- cmd/hyard/cli/vars_integration_test.go | 20 ++++++++++++ cmd/orbit/cli/bindings/compatibility.go | 21 ++++++++++++- cmd/orbit/cli/bindings/compatibility_test.go | 18 +++++++++++ cmd/orbit/cli/bindings/redaction.go | 12 +++++++ cmd/orbit/cli/bindings/runtime_resolution.go | 4 +++ .../cli/bindings/runtime_resolution_test.go | 20 ++++++++++++ cmd/orbit/cli/harness/bindings_explain.go | 6 +--- cmd/orbit/cli/harness/manifest.go | 10 ++++++ cmd/orbit/cli/harness/manifest_test.go | 27 ++++++++++++++++ cmd/orbit/cli/harness/template_manifest.go | 10 ++++++ .../cli/harness/template_manifest_test.go | 31 +++++++++++++++++++ cmd/orbit/cli/harness/template_merge.go | 22 +++++++++++++ cmd/orbit/cli/harness/template_merge_test.go | 21 +++++++++++++ cmd/orbit/cli/template/install.go | 3 ++ cmd/orbit/cli/template/install_test.go | 29 +++++++++++++++++ cmd/orbit/cli/template/manifest.go | 10 ++++++ cmd/orbit/cli/template/manifest_test.go | 27 ++++++++++++++++ 17 files changed, 285 insertions(+), 6 deletions(-) create mode 100644 cmd/orbit/cli/bindings/redaction.go diff --git a/cmd/hyard/cli/vars_integration_test.go b/cmd/hyard/cli/vars_integration_test.go index 554e6ee..c7fdc06 100644 --- a/cmd/hyard/cli/vars_integration_test.go +++ b/cmd/hyard/cli/vars_integration_test.go @@ -105,6 +105,26 @@ func TestHyardVarsDoctorReportsRuntimeBindingDiagnostics(t *testing.T) { require.Contains(t, varsDiagnosticKeys(payload.Warnings), "undeclared_binding:orphan_binding") } +func TestHyardVarsDoctorRejectsSensitiveInlineBindingWithoutLeakingValue(t *testing.T) { + t.Parallel() + + repo := seedHyardVarsInstallRuntime(t, map[string]bindings.VariableDeclaration{ + "github_token": {Description: "GitHub token", Required: true, Sensitive: true}, + }) + repo.WriteFile(t, ".harness/vars.yaml", ""+ + "schema_version: 2\n"+ + "variables:\n"+ + " github_token:\n"+ + " value: ghp_secret\n") + + stdout, stderr, err := executeHyardCLI(t, repo.Root, "vars", "doctor") + require.Error(t, err) + require.Empty(t, stderr) + require.Contains(t, stdout, "status: error\n") + require.Contains(t, stdout, "sensitive_value_source github_token") + require.NotContains(t, stdout, "ghp_secret") +} + func TestHyardVarsExplainReportsTextAndJSON(t *testing.T) { lockHyardProcessEnv(t) t.Setenv("GITHUB_TOKEN", "ghp_secret") diff --git a/cmd/orbit/cli/bindings/compatibility.go b/cmd/orbit/cli/bindings/compatibility.go index f00fcaa..6398ef8 100644 --- a/cmd/orbit/cli/bindings/compatibility.go +++ b/cmd/orbit/cli/bindings/compatibility.go @@ -18,8 +18,24 @@ func (err *DeclarationConflictError) Error() string { return fmt.Sprintf("variable conflict for %q (sources: %s)", err.Name, strings.Join(err.Sources, ", ")) } +// ValidateVariableDeclaration applies declaration-local Package Variable policy. +func ValidateVariableDeclaration(declaration VariableDeclaration) error { + if declaration.Sensitive && declaration.Default != nil { + return fmt.Errorf("sensitive variables must not define default") + } + + return nil +} + // MergeVariableDeclaration applies the shared compatibility policy for one variable declaration. func MergeVariableDeclaration(name string, current VariableDeclaration, next VariableDeclaration) (VariableDeclaration, error) { + if err := ValidateVariableDeclaration(current); err != nil { + return VariableDeclaration{}, fmt.Errorf("variable conflict for %q: %w", name, err) + } + if err := ValidateVariableDeclaration(next); err != nil { + return VariableDeclaration{}, fmt.Errorf("variable conflict for %q: %w", name, err) + } + switch { case current.Description == next.Description: case current.Description == "": @@ -40,7 +56,7 @@ func MergeVariableDeclaration(name string, current VariableDeclaration, next Var return VariableDeclaration{}, fmt.Errorf("variable conflict for %q", name) } if current.Sensitive && current.Default != nil { - return VariableDeclaration{}, fmt.Errorf("variable conflict for %q", name) + return VariableDeclaration{}, fmt.Errorf("variable conflict for %q: %w", name, ValidateVariableDeclaration(current)) } return current, nil @@ -55,6 +71,9 @@ func MergeDeclarations( ) error { for _, name := range contractutil.SortedKeys(next) { nextDeclaration := next[name] + if err := ValidateVariableDeclaration(nextDeclaration); err != nil { + return fmt.Errorf("validate variable %q declaration: %w", name, err) + } currentDeclaration, ok := merged[name] if !ok { merged[name] = nextDeclaration diff --git a/cmd/orbit/cli/bindings/compatibility_test.go b/cmd/orbit/cli/bindings/compatibility_test.go index 4a0b165..7e71a27 100644 --- a/cmd/orbit/cli/bindings/compatibility_test.go +++ b/cmd/orbit/cli/bindings/compatibility_test.go @@ -49,3 +49,21 @@ func TestMergeDeclarationsFailsOnConflictingDescriptions(t *testing.T) { require.Equal(t, "project_name", conflict.Name) require.Equal(t, []string{"orbit-template/cmd", "orbit-template/docs"}, conflict.Sources) } + +func TestMergeDeclarationsRejectsSensitiveDefault(t *testing.T) { + t.Parallel() + + defaultValue := "ghp_secret" + merged := map[string]VariableDeclaration{} + contributors := map[string][]string{} + + err := MergeDeclarations(merged, contributors, map[string]VariableDeclaration{ + "github_token": { + Required: true, + Sensitive: true, + Default: &defaultValue, + }, + }, "orbit-template/docs") + require.Error(t, err) + require.ErrorContains(t, err, "sensitive variables must not define default") +} diff --git a/cmd/orbit/cli/bindings/redaction.go b/cmd/orbit/cli/bindings/redaction.go new file mode 100644 index 0000000..a549994 --- /dev/null +++ b/cmd/orbit/cli/bindings/redaction.go @@ -0,0 +1,12 @@ +package bindings + +// RedactedSensitiveValue is the stable placeholder for resolved sensitive values. +const RedactedSensitiveValue = "" + +// RedactRuntimeBindingValue returns the public display value for one resolved binding. +func RedactRuntimeBindingValue(value string, sensitive bool) string { + if sensitive { + return RedactedSensitiveValue + } + return value +} diff --git a/cmd/orbit/cli/bindings/runtime_resolution.go b/cmd/orbit/cli/bindings/runtime_resolution.go index 02c0497..c41d9ee 100644 --- a/cmd/orbit/cli/bindings/runtime_resolution.go +++ b/cmd/orbit/cli/bindings/runtime_resolution.go @@ -43,6 +43,10 @@ type RuntimeBindingResolution struct { // ResolveRuntimeBinding selects a Runtime Binding using public P0 precedence: // scoped Runtime Binding, global Runtime Binding, declaration default, unresolved. func ResolveRuntimeBinding(input RuntimeBindingInput) (RuntimeBindingResolution, error) { + if err := ValidateVariableDeclaration(input.Declaration); err != nil { + return RuntimeBindingResolution{}, fmt.Errorf("%s: %w", input.Name, err) + } + result := RuntimeBindingResolution{ Name: input.Name, Required: input.Declaration.Required, diff --git a/cmd/orbit/cli/bindings/runtime_resolution_test.go b/cmd/orbit/cli/bindings/runtime_resolution_test.go index f302bba..6406c06 100644 --- a/cmd/orbit/cli/bindings/runtime_resolution_test.go +++ b/cmd/orbit/cli/bindings/runtime_resolution_test.go @@ -113,6 +113,26 @@ func TestResolveRuntimeBindingSelectsEmptyInlineValue(t *testing.T) { require.Empty(t, resolution.Value) } +func TestResolveRuntimeBindingRejectsSensitiveDefault(t *testing.T) { + t.Parallel() + + defaultValue := "ghp_secret" + _, err := ResolveRuntimeBinding(RuntimeBindingInput{ + Name: "github_token", + Declaration: VariableDeclaration{ + Required: true, + Sensitive: true, + Default: &defaultValue, + }, + VarsFile: VarsFile{ + SchemaVersion: VarsSchemaVersion, + Variables: map[string]VariableBinding{}, + }, + }) + require.Error(t, err) + require.ErrorContains(t, err, "sensitive variables must not define default") +} + func TestResolveRuntimeBindingValueSources(t *testing.T) { t.Parallel() diff --git a/cmd/orbit/cli/harness/bindings_explain.go b/cmd/orbit/cli/harness/bindings_explain.go index 821946c..2af3629 100644 --- a/cmd/orbit/cli/harness/bindings_explain.go +++ b/cmd/orbit/cli/harness/bindings_explain.go @@ -103,11 +103,7 @@ func ExplainRuntimeBinding(ctx context.Context, input RuntimeBindingExplainInput result.SelectedScope = selected.SelectedScope if selected.Resolved { result.Status = RuntimeBindingExplainStatusResolved - if result.Sensitive { - result.Value = "" - } else { - result.Value = selected.Value - } + result.Value = bindings.RedactRuntimeBindingValue(selected.Value, result.Sensitive) } return result, nil diff --git a/cmd/orbit/cli/harness/manifest.go b/cmd/orbit/cli/harness/manifest.go index 6c05fb8..c5459de 100644 --- a/cmd/orbit/cli/harness/manifest.go +++ b/cmd/orbit/cli/harness/manifest.go @@ -9,6 +9,7 @@ import ( "gopkg.in/yaml.v3" + "github.com/zack-nova/harnessyard/cmd/orbit/cli/bindings" gitpkg "github.com/zack-nova/harnessyard/cmd/orbit/cli/git" "github.com/zack-nova/harnessyard/cmd/orbit/cli/ids" "github.com/zack-nova/harnessyard/cmd/orbit/cli/internal/contractutil" @@ -368,6 +369,15 @@ func ValidateOrbitTemplateManifestFile(file ManifestFile) error { if err := contractutil.ValidateVariableName(name); err != nil { return fmt.Errorf("variables.%s: %w", name, err) } + spec := file.Variables[name] + if err := bindings.ValidateVariableDeclaration(bindings.VariableDeclaration{ + Description: spec.Description, + Required: spec.Required, + Sensitive: spec.Sensitive, + Default: spec.Default, + }); err != nil { + return fmt.Errorf("variables.%s: %w", name, err) + } } } diff --git a/cmd/orbit/cli/harness/manifest_test.go b/cmd/orbit/cli/harness/manifest_test.go index fa752c3..adf187c 100644 --- a/cmd/orbit/cli/harness/manifest_test.go +++ b/cmd/orbit/cli/harness/manifest_test.go @@ -232,6 +232,33 @@ func TestWriteAndLoadOrbitTemplateManifestFileRoundTrip(t *testing.T) { require.Equal(t, input, loaded) } +func TestLoadOrbitTemplateManifestFileRejectsSensitiveDefault(t *testing.T) { + t.Parallel() + + repoRoot := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Dir(ManifestPath(repoRoot)), 0o755)) + require.NoError(t, os.WriteFile(ManifestPath(repoRoot), []byte(""+ + "schema_version: 1\n"+ + "kind: orbit_template\n"+ + "template:\n"+ + " package:\n"+ + " type: orbit\n"+ + " name: docs\n"+ + " created_from_branch: main\n"+ + " created_from_commit: abc123\n"+ + " created_at: 2026-04-05T11:00:00Z\n"+ + "variables:\n"+ + " github_token:\n"+ + " required: true\n"+ + " sensitive: true\n"+ + " default: ghp_secret\n"), 0o600)) + + _, err := LoadManifestFile(repoRoot) + require.Error(t, err) + require.ErrorContains(t, err, "variables.github_token") + require.ErrorContains(t, err, "sensitive variables must not define default") +} + func TestWriteAndLoadHarnessTemplateManifestFileRoundTrip(t *testing.T) { t.Parallel() diff --git a/cmd/orbit/cli/harness/template_manifest.go b/cmd/orbit/cli/harness/template_manifest.go index 7559377..1b2f44f 100644 --- a/cmd/orbit/cli/harness/template_manifest.go +++ b/cmd/orbit/cli/harness/template_manifest.go @@ -8,6 +8,7 @@ import ( "gopkg.in/yaml.v3" + "github.com/zack-nova/harnessyard/cmd/orbit/cli/bindings" "github.com/zack-nova/harnessyard/cmd/orbit/cli/ids" "github.com/zack-nova/harnessyard/cmd/orbit/cli/internal/contractutil" ) @@ -168,6 +169,15 @@ func ValidateTemplateManifest(manifest TemplateManifest) error { if err := contractutil.ValidateVariableName(name); err != nil { return fmt.Errorf("variables.%s: %w", name, err) } + spec := manifest.Variables[name] + if err := bindings.ValidateVariableDeclaration(bindings.VariableDeclaration{ + Description: spec.Description, + Required: spec.Required, + Sensitive: spec.Sensitive, + Default: spec.Default, + }); err != nil { + return fmt.Errorf("variables.%s: %w", name, err) + } } return nil diff --git a/cmd/orbit/cli/harness/template_manifest_test.go b/cmd/orbit/cli/harness/template_manifest_test.go index a7b7359..9900a41 100644 --- a/cmd/orbit/cli/harness/template_manifest_test.go +++ b/cmd/orbit/cli/harness/template_manifest_test.go @@ -159,6 +159,37 @@ func TestLoadTemplateManifestAllowsEmptyMembersAndVariables(t *testing.T) { }, loaded) } +func TestLoadTemplateManifestRejectsSensitiveDefault(t *testing.T) { + t.Parallel() + + repoRoot := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Dir(TemplatePath(repoRoot)), 0o755)) + require.NoError(t, os.WriteFile(TemplatePath(repoRoot), []byte(""+ + "schema_version: 1\n"+ + "kind: harness_template\n"+ + "template:\n"+ + " harness_id: project_a\n"+ + " default_template: false\n"+ + " created_from_branch: main\n"+ + " created_from_commit: abc123\n"+ + " created_at: 2026-03-25T13:00:00Z\n"+ + " root_guidance:\n"+ + " agents: false\n"+ + " humans: false\n"+ + " bootstrap: false\n"+ + "members: []\n"+ + "variables:\n"+ + " github_token:\n"+ + " required: true\n"+ + " sensitive: true\n"+ + " default: ghp_secret\n"), 0o600)) + + _, err := LoadTemplateManifest(repoRoot) + require.Error(t, err) + require.ErrorContains(t, err, "variables.github_token") + require.ErrorContains(t, err, "sensitive variables must not define default") +} + func TestValidateTemplateManifestRejectsDuplicateMembers(t *testing.T) { t.Parallel() diff --git a/cmd/orbit/cli/harness/template_merge.go b/cmd/orbit/cli/harness/template_merge.go index a7393bc..ae3515f 100644 --- a/cmd/orbit/cli/harness/template_merge.go +++ b/cmd/orbit/cli/harness/template_merge.go @@ -6,6 +6,7 @@ import ( "sort" "strings" + "github.com/zack-nova/harnessyard/cmd/orbit/cli/bindings" orbittemplate "github.com/zack-nova/harnessyard/cmd/orbit/cli/template" ) @@ -74,6 +75,12 @@ func MergeTemplateMemberCandidates(candidates []TemplateMemberCandidate) (Templa } for name, next := range candidate.Variables { + if err := validateTemplateVariableSpec(next); err != nil { + return TemplateMergeResult{}, &TemplateVariableConflictError{ + Name: name, + Members: appendContributor(variableContributors[name], candidate.OrbitID), + } + } current, ok := variables[name] if !ok { variables[name] = next @@ -135,10 +142,25 @@ func mergeTemplateVariableSpec(name string, current TemplateVariableSpec, next T default: return TemplateVariableSpec{}, fmt.Errorf("variable conflict for %q", name) } + if err := validateTemplateVariableSpec(current); err != nil { + return TemplateVariableSpec{}, fmt.Errorf("variable conflict for %q", name) + } return current, nil } +func validateTemplateVariableSpec(spec TemplateVariableSpec) error { + if err := bindings.ValidateVariableDeclaration(bindings.VariableDeclaration{ + Description: spec.Description, + Required: spec.Required, + Sensitive: spec.Sensitive, + Default: spec.Default, + }); err != nil { + return fmt.Errorf("validate variable declaration: %w", err) + } + return nil +} + func appendContributor(existing []string, contributor string) []string { if contributor == "" { return sortedUniqueStrings(existing) diff --git a/cmd/orbit/cli/harness/template_merge_test.go b/cmd/orbit/cli/harness/template_merge_test.go index ff1cb60..62c40e8 100644 --- a/cmd/orbit/cli/harness/template_merge_test.go +++ b/cmd/orbit/cli/harness/template_merge_test.go @@ -156,3 +156,24 @@ func TestMergeTemplateMemberCandidatesFailsOnVariableDescriptionConflict(t *test require.ErrorContains(t, err, `variable conflict for "project_name"`) require.ErrorContains(t, err, `members: cmd, docs`) } + +func TestMergeTemplateMemberCandidatesRejectsSensitiveDefault(t *testing.T) { + t.Parallel() + + defaultValue := "ghp_secret" + _, err := MergeTemplateMemberCandidates([]TemplateMemberCandidate{ + { + OrbitID: "docs", + Variables: map[string]TemplateVariableSpec{ + "github_token": { + Required: true, + Sensitive: true, + Default: &defaultValue, + }, + }, + }, + }) + require.Error(t, err) + require.ErrorContains(t, err, `variable conflict for "github_token"`) + require.ErrorContains(t, err, `members: docs`) +} diff --git a/cmd/orbit/cli/template/install.go b/cmd/orbit/cli/template/install.go index f4b693b..cba982a 100644 --- a/cmd/orbit/cli/template/install.go +++ b/cmd/orbit/cli/template/install.go @@ -533,6 +533,9 @@ func validateInstallVariablesSnapshot(snapshot InstallVariablesSnapshot) error { if err := contractutil.ValidateVariableName(name); err != nil { return fmt.Errorf("declarations.%s: %w", name, err) } + if err := bindings.ValidateVariableDeclaration(snapshot.Declarations[name]); err != nil { + return fmt.Errorf("declarations.%s: %w", name, err) + } } for _, name := range contractutil.SortedKeys(snapshot.Namespaces) { if err := contractutil.ValidateVariableName(name); err != nil { diff --git a/cmd/orbit/cli/template/install_test.go b/cmd/orbit/cli/template/install_test.go index 7ec3e82..75a3488 100644 --- a/cmd/orbit/cli/template/install_test.go +++ b/cmd/orbit/cli/template/install_test.go @@ -184,6 +184,35 @@ func TestWriteAndLoadInstallRecordRoundTripWithEmptyVariableSnapshot(t *testing. require.Equal(t, input, loaded) } +func TestLoadInstallRecordRejectsSensitiveDefaultDeclaration(t *testing.T) { + t.Parallel() + + repoRoot := t.TempDir() + filename := filepath.Join(repoRoot, ".orbit", "installs", "docs.yaml") + require.NoError(t, os.MkdirAll(filepath.Dir(filename), 0o755)) + require.NoError(t, os.WriteFile(filename, []byte(""+ + "schema_version: 1\n"+ + "orbit_id: docs\n"+ + "template:\n"+ + " source_kind: local_branch\n"+ + " source_repo: \"\"\n"+ + " source_ref: orbit-template/docs\n"+ + " template_commit: abc123\n"+ + "applied_at: 2026-03-21T10:30:00Z\n"+ + "variables:\n"+ + " declarations:\n"+ + " github_token:\n"+ + " required: true\n"+ + " sensitive: true\n"+ + " default: ghp_secret\n"+ + " resolved_at_apply: {}\n"), 0o600)) + + _, err := LoadInstallRecord(repoRoot, "docs") + require.Error(t, err) + require.ErrorContains(t, err, "declarations.github_token") + require.ErrorContains(t, err, "sensitive variables must not define default") +} + func TestLoadInstallRecordRejectsMismatchedOrbitID(t *testing.T) { t.Parallel() diff --git a/cmd/orbit/cli/template/manifest.go b/cmd/orbit/cli/template/manifest.go index 0ecc0a7..01b988f 100644 --- a/cmd/orbit/cli/template/manifest.go +++ b/cmd/orbit/cli/template/manifest.go @@ -8,6 +8,7 @@ import ( "gopkg.in/yaml.v3" + "github.com/zack-nova/harnessyard/cmd/orbit/cli/bindings" "github.com/zack-nova/harnessyard/cmd/orbit/cli/ids" "github.com/zack-nova/harnessyard/cmd/orbit/cli/internal/contractutil" ) @@ -159,6 +160,15 @@ func ValidateManifest(manifest Manifest) error { if err := contractutil.ValidateVariableName(name); err != nil { return fmt.Errorf("variables.%s: %w", name, err) } + spec := manifest.Variables[name] + if err := bindings.ValidateVariableDeclaration(bindings.VariableDeclaration{ + Description: spec.Description, + Required: spec.Required, + Sensitive: spec.Sensitive, + Default: spec.Default, + }); err != nil { + return fmt.Errorf("variables.%s: %w", name, err) + } } seenSharedPaths := make(map[string]struct{}, len(manifest.SharedFiles)) for index, sharedFile := range manifest.SharedFiles { diff --git a/cmd/orbit/cli/template/manifest_test.go b/cmd/orbit/cli/template/manifest_test.go index 5ae8399..60f80e6 100644 --- a/cmd/orbit/cli/template/manifest_test.go +++ b/cmd/orbit/cli/template/manifest_test.go @@ -86,6 +86,33 @@ func TestLoadManifestRejectsMissingRequiredFlag(t *testing.T) { require.ErrorContains(t, err, "variables.project_name.required") } +func TestLoadManifestRejectsSensitiveDefault(t *testing.T) { + t.Parallel() + + repoRoot := t.TempDir() + filename := filepath.Join(repoRoot, ".orbit", "template.yaml") + require.NoError(t, os.MkdirAll(filepath.Dir(filename), 0o755)) + require.NoError(t, os.WriteFile(filename, []byte(""+ + "schema_version: 1\n"+ + "kind: template\n"+ + "template:\n"+ + " orbit_id: docs\n"+ + " default_template: false\n"+ + " created_from_branch: main\n"+ + " created_from_commit: abc123\n"+ + " created_at: 2026-03-21T10:00:00Z\n"+ + "variables:\n"+ + " github_token:\n"+ + " required: true\n"+ + " sensitive: true\n"+ + " default: ghp_secret\n"), 0o600)) + + _, err := LoadManifest(repoRoot) + require.Error(t, err) + require.ErrorContains(t, err, "variables.github_token") + require.ErrorContains(t, err, "sensitive variables must not define default") +} + func TestLoadManifestAllowsEmptyVariablesMap(t *testing.T) { t.Parallel()