diff --git a/pkg/context/safe_log_test.go b/pkg/context/safe_log_test.go index c6b2bf4..556bfc7 100644 --- a/pkg/context/safe_log_test.go +++ b/pkg/context/safe_log_test.go @@ -12,11 +12,13 @@ import ( // entries. A newline lets an attacker append a fabricated line; a carriage // return can overwrite one on a terminal; ANSI escapes can hide text entirely. func TestSafeLogValueStripsControlCharacters(t *testing.T) { - tests := []struct { + type testCase struct { name string input string want string - }{ + } + + tests := []testCase{ {"plain text passes through", "111111111111", "111111111111"}, {"newline forging a second entry", "111\nlevel=fatal msg=\"breach\"", "111level=fatal msg=\"breach\""}, {"carriage return overwriting a line", "111\rmalicious", "111malicious"}, @@ -26,16 +28,19 @@ func TestSafeLogValueStripsControlCharacters(t *testing.T) { {"delete character", "111\x7f222", "111222"}, {"empty stays empty", "", ""}, {"unicode is preserved", "café-eu-west-1", "café-eu-west-1"}, - - // Filtering only the ASCII range leaves these, and each breaks a log - // line as effectively as \n does. - {"U+2028 line separator", "111
forged", "111forged"}, - {"U+2029 paragraph separator", "111
forged", "111forged"}, - {"U+0085 next line", "111…forged", "111forged"}, - {"U+009B C1 control sequence introducer", "111›forged", "111forged"}, - {"U+200E bidi mark reordering display", "111‎forged", "111forged"}, - {"U+202E right-to-left override", "111‮forged", "111forged"}, - {"U+200B zero-width space", "111​forged", "111forged"}, + } + // Filtering only the ASCII range leaves these, and each breaks a log line + // as effectively as \n does. + for name, r := range map[string]rune{ + "U+2028 line separator": 0x2028, + "U+2029 paragraph separator": 0x2029, + "U+0085 next line": 0x0085, + "U+009B C1 control sequence introducer": 0x009B, + "U+200E bidi mark reordering display": 0x200E, + "U+202E right-to-left override": 0x202E, + "U+200B zero-width space": 0x200B, + } { + tests = append(tests, testCase{name, "111" + string(r) + "forged", "111forged"}) } for _, tc := range tests { diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 2be0cf4..f6a9954 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -354,6 +354,15 @@ func formatJSON(diff *PipelineDiff) string { return string(data) } +func writeChangeSummary(sb *strings.Builder, summary ChangeSummary) { + fmt.Fprintf(sb, " Added: %d\n", summary.Added) + fmt.Fprintf(sb, " Removed: %d\n", summary.Removed) + fmt.Fprintf(sb, " Modified: %d\n", summary.Modified) + fmt.Fprintf(sb, " Unchanged: %d\n", summary.Unchanged) + fmt.Fprintf(sb, " Total: %d\n", summary.Total) + sb.WriteString("\n") +} + func formatHuman(diff *PipelineDiff) string { var sb strings.Builder @@ -365,12 +374,7 @@ func formatHuman(diff *PipelineDiff) string { // Overall summary sb.WriteString("Pipeline Diff Summary\n") sb.WriteString("=====================\n") - sb.WriteString(fmt.Sprintf(" Added: %d\n", diff.Summary.Added)) - sb.WriteString(fmt.Sprintf(" Removed: %d\n", diff.Summary.Removed)) - sb.WriteString(fmt.Sprintf(" Modified: %d\n", diff.Summary.Modified)) - sb.WriteString(fmt.Sprintf(" Unchanged: %d\n", diff.Summary.Unchanged)) - sb.WriteString(fmt.Sprintf(" Total: %d\n", diff.Summary.Total)) - sb.WriteString("\n") + writeChangeSummary(&sb, diff.Summary) if diff.IsZeroSum() { sb.WriteString("✅ ZERO-SUM: No changes detected\n") @@ -385,7 +389,7 @@ func formatHuman(diff *PipelineDiff) string { continue } - sb.WriteString(fmt.Sprintf("Target: %s\n", td.Target)) + fmt.Fprintf(&sb, "Target: %s\n", td.Target) sb.WriteString(strings.Repeat("-", 40) + "\n") for _, c := range td.Changes { @@ -399,16 +403,16 @@ func formatHuman(diff *PipelineDiff) string { if c.DesiredVersion > 0 { versionInfo = fmt.Sprintf(" (v%d)", c.DesiredVersion) } - sb.WriteString(fmt.Sprintf(" + %s (new secret)%s\n", c.Path, versionInfo)) + fmt.Fprintf(&sb, " + %s (new secret)%s\n", c.Path, versionInfo) if len(c.DesiredKeys) > 0 { - sb.WriteString(fmt.Sprintf(" keys: %v\n", c.DesiredKeys)) + fmt.Fprintf(&sb, " keys: %v\n", c.DesiredKeys) } case ChangeTypeRemoved: versionInfo := "" if c.CurrentVersion > 0 { versionInfo = fmt.Sprintf(" (was v%d)", c.CurrentVersion) } - sb.WriteString(fmt.Sprintf(" - %s (removed)%s\n", c.Path, versionInfo)) + fmt.Fprintf(&sb, " - %s (removed)%s\n", c.Path, versionInfo) case ChangeTypeModified: versionInfo := "" if c.CurrentVersion > 0 && c.DesiredVersion > 0 { @@ -418,15 +422,15 @@ func formatHuman(diff *PipelineDiff) string { } else if c.DesiredVersion > 0 { versionInfo = fmt.Sprintf(" (→ v%d)", c.DesiredVersion) } - sb.WriteString(fmt.Sprintf(" ~ %s (modified)%s\n", c.Path, versionInfo)) + fmt.Fprintf(&sb, " ~ %s (modified)%s\n", c.Path, versionInfo) if len(c.KeysAdded) > 0 { - sb.WriteString(fmt.Sprintf(" + keys: %v\n", c.KeysAdded)) + fmt.Fprintf(&sb, " + keys: %v\n", c.KeysAdded) } if len(c.KeysRemoved) > 0 { - sb.WriteString(fmt.Sprintf(" - keys: %v\n", c.KeysRemoved)) + fmt.Fprintf(&sb, " - keys: %v\n", c.KeysRemoved) } if len(c.KeysModified) > 0 { - sb.WriteString(fmt.Sprintf(" ~ keys: %v\n", c.KeysModified)) + fmt.Fprintf(&sb, " ~ keys: %v\n", c.KeysModified) } } } @@ -442,9 +446,9 @@ func formatGitHub(diff *PipelineDiff) string { if diff.IsZeroSum() { sb.WriteString("::notice::✅ Zero-sum: No changes detected\n") } else { - sb.WriteString(fmt.Sprintf("::warning::⚠️ %d changes detected (%d added, %d removed, %d modified)\n", + fmt.Fprintf(&sb, "::warning::⚠️ %d changes detected (%d added, %d removed, %d modified)\n", diff.Summary.Added+diff.Summary.Removed+diff.Summary.Modified, - diff.Summary.Added, diff.Summary.Removed, diff.Summary.Modified)) + diff.Summary.Added, diff.Summary.Removed, diff.Summary.Modified) } // Group annotations by target @@ -453,18 +457,18 @@ func formatGitHub(diff *PipelineDiff) string { continue } - sb.WriteString(fmt.Sprintf("::group::Target: %s (%d changes)\n", escapeGitHubCommandData(td.Target), - td.Summary.Added+td.Summary.Removed+td.Summary.Modified)) + fmt.Fprintf(&sb, "::group::Target: %s (%d changes)\n", escapeGitHubCommandData(td.Target), + td.Summary.Added+td.Summary.Removed+td.Summary.Modified) for _, c := range td.Changes { safePath := escapeGitHubCommandData(c.Path) switch c.ChangeType { case ChangeTypeAdded: - sb.WriteString(fmt.Sprintf("::notice::+ %s (new secret)\n", safePath)) + fmt.Fprintf(&sb, "::notice::+ %s (new secret)\n", safePath) case ChangeTypeRemoved: - sb.WriteString(fmt.Sprintf("::warning::- %s (removed)\n", safePath)) + fmt.Fprintf(&sb, "::warning::- %s (removed)\n", safePath) case ChangeTypeModified: - sb.WriteString(fmt.Sprintf("::notice::~ %s (modified)\n", safePath)) + fmt.Fprintf(&sb, "::notice::~ %s (modified)\n", safePath) } } @@ -530,12 +534,7 @@ func formatSideBySide(diff *PipelineDiff, showValues bool) string { // Overall summary sb.WriteString("Pipeline Diff Summary (Side-by-Side)\n") sb.WriteString("====================================\n") - sb.WriteString(fmt.Sprintf(" Added: %d\n", diff.Summary.Added)) - sb.WriteString(fmt.Sprintf(" Removed: %d\n", diff.Summary.Removed)) - sb.WriteString(fmt.Sprintf(" Modified: %d\n", diff.Summary.Modified)) - sb.WriteString(fmt.Sprintf(" Unchanged: %d\n", diff.Summary.Unchanged)) - sb.WriteString(fmt.Sprintf(" Total: %d\n", diff.Summary.Total)) - sb.WriteString("\n") + writeChangeSummary(&sb, diff.Summary) if diff.IsZeroSum() { sb.WriteString("✅ ZERO-SUM: No changes detected\n") @@ -550,7 +549,7 @@ func formatSideBySide(diff *PipelineDiff, showValues bool) string { continue } - sb.WriteString(fmt.Sprintf("Target: %s\n", td.Target)) + fmt.Fprintf(&sb, "Target: %s\n", td.Target) sb.WriteString(strings.Repeat("=", 80) + "\n") for _, c := range td.Changes { @@ -583,33 +582,33 @@ func formatSecretChangeSideBySide(change SecretChange, showValues bool) string { switch change.ChangeType { case ChangeTypeAdded: - sb.WriteString(fmt.Sprintf("+ %s%s\n", change.Path, versionInfo)) + fmt.Fprintf(&sb, "+ %s%s\n", change.Path, versionInfo) sb.WriteString(" ┌─ NEW SECRET ─────────────────────────────────────────────────────────┐\n") if showValues && change.DesiredValues != nil { for key, value := range change.DesiredValues { maskedValue := maskValue(value, showValues) - sb.WriteString(fmt.Sprintf(" │ + %-20s: %s\n", key, maskedValue)) + fmt.Fprintf(&sb, " │ + %-20s: %s\n", key, maskedValue) } } else if len(change.DesiredKeys) > 0 { - sb.WriteString(fmt.Sprintf(" │ Keys: %v\n", change.DesiredKeys)) + fmt.Fprintf(&sb, " │ Keys: %v\n", change.DesiredKeys) } sb.WriteString(" └──────────────────────────────────────────────────────────────────────┘\n") case ChangeTypeRemoved: - sb.WriteString(fmt.Sprintf("- %s%s\n", change.Path, versionInfo)) + fmt.Fprintf(&sb, "- %s%s\n", change.Path, versionInfo) sb.WriteString(" ┌─ REMOVED SECRET ─────────────────────────────────────────────────────┐\n") if showValues && change.CurrentValues != nil { for key, value := range change.CurrentValues { maskedValue := maskValue(value, showValues) - sb.WriteString(fmt.Sprintf(" │ - %-20s: %s\n", key, maskedValue)) + fmt.Fprintf(&sb, " │ - %-20s: %s\n", key, maskedValue) } } else if len(change.CurrentKeys) > 0 { - sb.WriteString(fmt.Sprintf(" │ Keys: %v\n", change.CurrentKeys)) + fmt.Fprintf(&sb, " │ Keys: %v\n", change.CurrentKeys) } sb.WriteString(" └──────────────────────────────────────────────────────────────────────┘\n") case ChangeTypeModified: - sb.WriteString(fmt.Sprintf("~ %s%s\n", change.Path, versionInfo)) + fmt.Fprintf(&sb, "~ %s%s\n", change.Path, versionInfo) sb.WriteString(" ┌─ CURRENT ─────────────────┬─ DESIRED ─────────────────────────────────┐\n") // Show side-by-side comparison @@ -648,23 +647,23 @@ func formatSecretChangeSideBySide(change SecretChange, showValues bool) string { indicator = "~" } - sb.WriteString(fmt.Sprintf(" │%s%-10s: %-15s │%s%-10s: %-15s │\n", + fmt.Fprintf(&sb, " │%s%-10s: %-15s │%s%-10s: %-15s │\n", indicator, key, truncateString(currentStr, 15), - indicator, key, truncateString(desiredStr, 15))) + indicator, key, truncateString(desiredStr, 15)) } } else { // Show key-level changes if len(change.KeysAdded) > 0 { - sb.WriteString(fmt.Sprintf(" │ + Added keys: %-12s │ │\n", - strings.Join(change.KeysAdded, ", "))) + fmt.Fprintf(&sb, " │ + Added keys: %-12s │ │\n", + strings.Join(change.KeysAdded, ", ")) } if len(change.KeysRemoved) > 0 { - sb.WriteString(fmt.Sprintf(" │ - Removed keys: %-10s │ │\n", - strings.Join(change.KeysRemoved, ", "))) + fmt.Fprintf(&sb, " │ - Removed keys: %-10s │ │\n", + strings.Join(change.KeysRemoved, ", ")) } if len(change.KeysModified) > 0 { - sb.WriteString(fmt.Sprintf(" │ ~ Modified keys: %-9s │ │\n", - strings.Join(change.KeysModified, ", "))) + fmt.Fprintf(&sb, " │ ~ Modified keys: %-9s │ │\n", + strings.Join(change.KeysModified, ", ")) } } sb.WriteString(" └───────────────────────────┴───────────────────────────────────────────┘\n") diff --git a/pkg/observability/tracing.go b/pkg/observability/tracing.go index 56e2b05..80a840f 100644 --- a/pkg/observability/tracing.go +++ b/pkg/observability/tracing.go @@ -10,6 +10,8 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" "go.opentelemetry.io/otel/exporters/stdout/stdouttrace" + + //nolint:staticcheck // Zipkin remains a documented, supported compatibility exporter. "go.opentelemetry.io/otel/exporters/zipkin" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/sdk/resource" diff --git a/pkg/pipeline/aws_context.go b/pkg/pipeline/aws_context.go index e98006e..420d109 100644 --- a/pkg/pipeline/aws_context.go +++ b/pkg/pipeline/aws_context.go @@ -641,30 +641,30 @@ func (ec *AWSExecutionContext) Summary() string { var sb strings.Builder sb.WriteString("AWS Execution Context:\n") - sb.WriteString(fmt.Sprintf(" Account ID: %s\n", ec.CallerIdentity.AccountID)) - sb.WriteString(fmt.Sprintf(" ARN: %s\n", ec.CallerIdentity.ARN)) + fmt.Fprintf(&sb, " Account ID: %s\n", ec.CallerIdentity.AccountID) + fmt.Fprintf(&sb, " ARN: %s\n", ec.CallerIdentity.ARN) if ec.OrganizationInfo != nil { - sb.WriteString(fmt.Sprintf(" Organization ID: %s\n", ec.OrganizationInfo.ID)) - sb.WriteString(fmt.Sprintf(" Management Account: %s\n", ec.OrganizationInfo.MasterAccountID)) + fmt.Fprintf(&sb, " Organization ID: %s\n", ec.OrganizationInfo.ID) + fmt.Fprintf(&sb, " Management Account: %s\n", ec.OrganizationInfo.MasterAccountID) if ec.OrganizationInfo.IsManagementAccount { sb.WriteString(" Role: Management Account ⚠️\n") } else if ec.OrganizationInfo.IsDelegatedAdmin { sb.WriteString(" Role: Delegated Administrator ✓\n") - sb.WriteString(fmt.Sprintf(" Delegated Services: %v\n", ec.OrganizationInfo.DelegatedServices)) + fmt.Fprintf(&sb, " Delegated Services: %v\n", ec.OrganizationInfo.DelegatedServices) } else { sb.WriteString(" Role: Member Account\n") } } - sb.WriteString(fmt.Sprintf(" Control Tower: %v\n", ec.Config.ControlTower.Enabled)) + fmt.Fprintf(&sb, " Control Tower: %v\n", ec.Config.ControlTower.Enabled) if ec.Config.ControlTower.Enabled { - sb.WriteString(fmt.Sprintf(" Execution Role: %s\n", ec.Config.ControlTower.ExecutionRole.Name)) + fmt.Fprintf(&sb, " Execution Role: %s\n", ec.Config.ControlTower.ExecutionRole.Name) } - sb.WriteString(fmt.Sprintf(" Identity Center Access: %v\n", ec.CanAccessIdentityCenter())) - sb.WriteString(fmt.Sprintf(" Organizations Access: %v\n", ec.CanAccessOrganizations())) + fmt.Fprintf(&sb, " Identity Center Access: %v\n", ec.CanAccessIdentityCenter()) + fmt.Fprintf(&sb, " Organizations Access: %v\n", ec.CanAccessOrganizations()) return sb.String() } diff --git a/pkg/pipeline/graph.go b/pkg/pipeline/graph.go index 852d386..dc0e4e8 100644 --- a/pkg/pipeline/graph.go +++ b/pkg/pipeline/graph.go @@ -242,7 +242,7 @@ func (g *Graph) PrintGraph() string { sb.WriteString("Dependency Graph:\n") for i, level := range levels { - sb.WriteString(fmt.Sprintf(" Level %d: %v\n", i, level)) + fmt.Fprintf(&sb, " Level %d: %v\n", i, level) } sb.WriteString("\nInheritance:\n") @@ -257,7 +257,7 @@ func (g *Graph) PrintGraph() string { } } if len(targetDeps) > 0 { - sb.WriteString(fmt.Sprintf(" %s <- %v\n", name, targetDeps)) + fmt.Fprintf(&sb, " %s <- %v\n", name, targetDeps) } } } diff --git a/pkg/pipeline/pipeline.go b/pkg/pipeline/pipeline.go index 275ddda..adb333f 100644 --- a/pkg/pipeline/pipeline.go +++ b/pkg/pipeline/pipeline.go @@ -182,9 +182,9 @@ func NewWithContextAndRuntimeAuth(ctx context.Context, cfg *Config, auth *Runtim // Initialize AWS execution context if configured if runtimeCfg.AWS.ExecutionContext.Type != "" { - awsCtx, err := NewAWSExecutionContextWithRuntimeAuth(ctx, &runtimeCfg.AWS, p.runtimeAWSAuth()) - if err != nil { - log.WithError(err).Warn("Failed to initialize AWS execution context") + awsCtx, awsErr := NewAWSExecutionContextWithRuntimeAuth(ctx, &runtimeCfg.AWS, p.runtimeAWSAuth()) + if awsErr != nil { + log.WithError(awsErr).Warn("Failed to initialize AWS execution context") } else { p.awsCtx = awsCtx } @@ -194,15 +194,15 @@ func NewWithContextAndRuntimeAuth(ctx context.Context, cfg *Config, auth *Runtim // exists. Validate accepts a config whose targets are entirely dynamic, so // skipping this would build an empty dependency graph and the run would // report success having synced nothing. - if err := p.expandDynamicTargets(ctx, runtimeCfg); err != nil { - return nil, err + if expandErr := p.expandDynamicTargets(ctx, runtimeCfg); expandErr != nil { + return nil, expandErr } // Initialize S3 merge store if configured if runtimeCfg.MergeStore.S3 != nil { - s3Store, err := NewS3MergeStoreWithRuntimeAuth(ctx, runtimeCfg.MergeStore.S3, runtimeCfg.AWS.Region, p.runtimeAWSAuth()) - if err != nil { - log.WithError(err).Warn("Failed to initialize S3 merge store") + s3Store, s3Err := NewS3MergeStoreWithRuntimeAuth(ctx, runtimeCfg.MergeStore.S3, runtimeCfg.AWS.Region, p.runtimeAWSAuth()) + if s3Err != nil { + log.WithError(s3Err).Warn("Failed to initialize S3 merge store") } else { p.s3Store = s3Store // Build cross-region replicas if configured. diff --git a/scripts/go-packages.sh b/scripts/go-packages.sh index 210247a..a1d9817 100755 --- a/scripts/go-packages.sh +++ b/scripts/go-packages.sh @@ -9,6 +9,8 @@ mapfile -t package_dirs < <( ! -path './.tools/*' \ ! -path './bin/*' \ ! -path './dist/*' \ + ! -path '*/node_modules/*' \ + ! -path './docs/dist/*' \ ! -path './python/build/*' \ -exec dirname {} \; \ | sort -u