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()