From a06687255d5a6e4dc989d2656d4451e69d69c2d1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:17:46 +0000 Subject: [PATCH] =?UTF-8?q?chore:=20remove=20dead=20functions=20=E2=80=94?= =?UTF-8?q?=207=20functions=20removed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes unreachable Go functions identified by the deadcode static analyzer. Functions removed: - ResolveRefToSHA (pkg/parser/remote_fetch.go) - ExtractPermissionsFromContent (pkg/parser/content_extractor.go) - SemanticVersion.IsPrecise (pkg/semverutil/semverutil.go) - SemanticVersion.IsNewer (pkg/semverutil/semverutil.go) - writeArgsToYAMLInline (pkg/workflow/args.go) - higherIntegrityLevels (pkg/workflow/cache_integrity.go) - GenerateHeredocDelimiter (pkg/workflow/strings.go) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/parser/content_extractor.go | 5 - pkg/parser/remote_fetch.go | 7 -- pkg/semverutil/semverutil.go | 18 --- pkg/semverutil/semverutil_test.go | 53 -------- pkg/workflow/args.go | 10 -- pkg/workflow/cache_integrity.go | 15 --- pkg/workflow/cache_integrity_test.go | 22 ---- pkg/workflow/mcp_config_utils_test.go | 73 ----------- pkg/workflow/permissions_import_test.go | 61 ---------- pkg/workflow/strings.go | 39 ------ pkg/workflow/strings_test.go | 154 ------------------------ 11 files changed, 457 deletions(-) delete mode 100644 pkg/workflow/mcp_config_utils_test.go diff --git a/pkg/parser/content_extractor.go b/pkg/parser/content_extractor.go index 36b18c8a84b..5bf3d54a04b 100644 --- a/pkg/parser/content_extractor.go +++ b/pkg/parser/content_extractor.go @@ -54,11 +54,6 @@ func extractToolsFromContent(content string) (string, error) { return strings.TrimSpace(string(extractedJSON)), nil } -// ExtractPermissionsFromContent extracts permissions section from frontmatter as JSON string -func ExtractPermissionsFromContent(content string) (string, error) { - return extractFrontmatterField(content, "permissions", "{}") -} - // extractFrontmatterField extracts a specific field from frontmatter as JSON string func extractFrontmatterField(content, fieldName, emptyValue string) (string, error) { contentExtractorLog.Printf("Extracting field: %s", fieldName) diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go index 772f9c63ac6..f7de2025ee6 100644 --- a/pkg/parser/remote_fetch.go +++ b/pkg/parser/remote_fetch.go @@ -699,13 +699,6 @@ func DownloadFileFromGitHubForHost(owner, repo, path, ref, host string) ([]byte, return downloadFileFromGitHubWithDepth(owner, repo, path, ref, 0, host) } -// ResolveRefToSHA resolves a git ref (branch, tag, or short SHA) to its full commit SHA. -// This is the exported wrapper for resolveRefToSHA. -// If the ref is already a 40-character hex SHA, it returns it as-is. -func ResolveRefToSHA(owner, repo, ref string) (string, error) { - return resolveRefToSHA(owner, repo, ref, "") -} - // ResolveRefToSHAForHost resolves a git ref to its full commit SHA on a specific GitHub host. // Use this when the target repository is on a different host than the one configured via GH_HOST. // host is the hostname without scheme (e.g., "github.com", "myorg.ghe.com"). diff --git a/pkg/semverutil/semverutil.go b/pkg/semverutil/semverutil.go index 7dcdb3ba531..b5b539fa351 100644 --- a/pkg/semverutil/semverutil.go +++ b/pkg/semverutil/semverutil.go @@ -98,24 +98,6 @@ func ParseVersion(v string) *SemanticVersion { return ver } -// IsPrecise returns true if the version has explicit minor and patch components. -// For example, "v6.0.0" is precise, but "v6" is not. -func (v *SemanticVersion) IsPrecise() bool { - versionPart := strings.TrimPrefix(v.Raw, "v") - dotCount := strings.Count(versionPart, ".") - return dotCount >= 2 // Require at least major.minor.patch -} - -// IsNewer returns true if v is strictly newer than other. -// Uses golang.org/x/mod/semver.Compare for proper semantic version comparison. -func (v *SemanticVersion) IsNewer(other *SemanticVersion) bool { - v1 := EnsureVPrefix(v.Raw) - v2 := EnsureVPrefix(other.Raw) - isNewer := semver.Compare(v1, v2) > 0 - log.Printf("Version comparison: %s vs %s, isNewer=%v", v.Raw, other.Raw, isNewer) - return isNewer -} - // Compare compares two semantic versions and returns 1 if v1 > v2, -1 if v1 < v2, // or 0 if they are equal. A bare version without a leading "v" is accepted. func Compare(v1, v2 string) int { diff --git a/pkg/semverutil/semverutil_test.go b/pkg/semverutil/semverutil_test.go index 1c9952af1da..6dcb52a7b41 100644 --- a/pkg/semverutil/semverutil_test.go +++ b/pkg/semverutil/semverutil_test.go @@ -164,59 +164,6 @@ func TestParseVersion(t *testing.T) { } } -func TestSemanticVersionIsPrecise(t *testing.T) { - tests := []struct { - version string - want bool - }{ - {"v6", false}, - {"v6.0", false}, - {"v6.0.0", true}, - {"v1.2.3", true}, - {"6.0.0", true}, - } - for _, tt := range tests { - t.Run(tt.version, func(t *testing.T) { - v := ParseVersion(tt.version) - if v == nil { - t.Fatalf("failed to parse version: %s", tt.version) - } - got := v.IsPrecise() - if got != tt.want { - t.Errorf("ParseVersion(%q).IsPrecise() = %v, want %v", tt.version, got, tt.want) - } - }) - } -} - -func TestSemanticVersionIsNewer(t *testing.T) { - tests := []struct { - name string - version string - other string - want bool - }{ - {"newer major", "v2.0.0", "v1.0.0", true}, - {"older major", "v1.0.0", "v2.0.0", false}, - {"newer minor", "v1.2.0", "v1.1.0", true}, - {"same version", "v1.0.0", "v1.0.0", false}, - {"release vs prerelease", "v1.0.0", "v1.0.0-beta", true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - v := ParseVersion(tt.version) - other := ParseVersion(tt.other) - if v == nil || other == nil { - t.Fatalf("failed to parse versions %q or %q", tt.version, tt.other) - } - got := v.IsNewer(other) - if got != tt.want { - t.Errorf("(%q).IsNewer(%q) = %v, want %v", tt.version, tt.other, got, tt.want) - } - }) - } -} - func TestCompare(t *testing.T) { tests := []struct { v1 string diff --git a/pkg/workflow/args.go b/pkg/workflow/args.go index f5716ec9ecc..d8c1dee143e 100644 --- a/pkg/workflow/args.go +++ b/pkg/workflow/args.go @@ -63,13 +63,3 @@ func writeArgsToYAML(yaml *strings.Builder, args []string, indent string) { yaml.WriteString(indent + string(quotedArg)) } } - -// writeArgsToYAMLInline writes custom args to YAML inline with proper JSON quoting and escaping -// Used when args are written on the same line with comma-space separators -func writeArgsToYAMLInline(yaml *strings.Builder, args []string) { - for _, arg := range args { - // Use json.Marshal to properly quote and escape the argument - quotedArg, _ := json.Marshal(arg) - yaml.WriteString(", " + string(quotedArg)) - } -} diff --git a/pkg/workflow/cache_integrity.go b/pkg/workflow/cache_integrity.go index 97a8ce6c703..3662e740b86 100644 --- a/pkg/workflow/cache_integrity.go +++ b/pkg/workflow/cache_integrity.go @@ -218,18 +218,3 @@ func generateIntegrityAwareCacheKey(cacheID, integrityLevel, policyHash string) cacheIntegrityLog.Printf("Generated integrity-aware cache key: cacheID=%s, integrityLevel=%s, policyHash=%s", cacheID, integrityLevel, policyHash) return key } - -// higherIntegrityLevels returns the integrity levels that are higher than the given level, -// ordered from highest to lowest (merged → approved → unapproved → none). -// Used to determine which branches to merge down from. -func higherIntegrityLevels(level string) []string { - var result []string - for _, l := range integrityLevelOrder { - if l == level { - break - } - result = append(result, l) - } - cacheIntegrityLog.Printf("Higher integrity levels than %q: %v", level, result) - return result -} diff --git a/pkg/workflow/cache_integrity_test.go b/pkg/workflow/cache_integrity_test.go index d4e5bb6b67b..b9b40b73da8 100644 --- a/pkg/workflow/cache_integrity_test.go +++ b/pkg/workflow/cache_integrity_test.go @@ -308,28 +308,6 @@ func TestCacheIntegrityLevel(t *testing.T) { } } -// TestHigherIntegrityLevels verifies the merge-down logic helper. -func TestHigherIntegrityLevels(t *testing.T) { - tests := []struct { - name string - level string - expected []string - }{ - {name: "merged (highest) has no higher", level: "merged", expected: nil}, - {name: "approved has merged above", level: "approved", expected: []string{"merged"}}, - {name: "unapproved has merged+approved above", level: "unapproved", expected: []string{"merged", "approved"}}, - {name: "none (lowest) has all above", level: "none", expected: []string{"merged", "approved", "unapproved"}}, - {name: "unknown level has all levels above (no match = all)", level: "unknown", expected: []string{"merged", "approved", "unapproved", "none"}}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := higherIntegrityLevels(tt.level) - assert.Equal(t, tt.expected, result, "Higher integrity levels mismatch for '%s'", tt.level) - }) - } -} - // TestComputeIntegrityCacheKey_WithGitHubConfig verifies that computeIntegrityCacheKey // produces the correct key when a GitHub guard policy is configured. func TestComputeIntegrityCacheKey_WithGitHubConfig(t *testing.T) { diff --git a/pkg/workflow/mcp_config_utils_test.go b/pkg/workflow/mcp_config_utils_test.go deleted file mode 100644 index e2aabd44de8..00000000000 --- a/pkg/workflow/mcp_config_utils_test.go +++ /dev/null @@ -1,73 +0,0 @@ -//go:build !integration - -package workflow - -import ( - "strings" - "testing" -) - -func TestWriteArgsToYAMLInline(t *testing.T) { - tests := []struct { - name string - args []string - want string - }{ - { - name: "no args", - args: []string{}, - want: "", - }, - { - name: "single simple arg", - args: []string{"--verbose"}, - want: `, "--verbose"`, - }, - { - name: "multiple simple args", - args: []string{"--verbose", "--debug"}, - want: `, "--verbose", "--debug"`, - }, - { - name: "args with spaces", - args: []string{"--message", "hello world"}, - want: `, "--message", "hello world"`, - }, - { - name: "args with quotes", - args: []string{"--text", `say "hello"`}, - want: `, "--text", "say \"hello\""`, - }, - { - name: "args with special characters", - args: []string{"--path", "/tmp/test\n\t"}, - want: `, "--path", "/tmp/test\n\t"`, - }, - { - name: "args with backslashes", - args: []string{"--path", `C:\Windows\System32`}, - want: `, "--path", "C:\\Windows\\System32"`, - }, - { - name: "empty string arg", - args: []string{""}, - want: `, ""`, - }, - { - name: "unicode args", - args: []string{"--text", "Hello 世界 🌍"}, - want: `, "--text", "Hello 世界 🌍"`, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var builder strings.Builder - writeArgsToYAMLInline(&builder, tt.args) - got := builder.String() - if got != tt.want { - t.Errorf("writeArgsToYAMLInline() = %q, want %q", got, tt.want) - } - }) - } -} diff --git a/pkg/workflow/permissions_import_test.go b/pkg/workflow/permissions_import_test.go index da95c4c4302..1431fce5210 100644 --- a/pkg/workflow/permissions_import_test.go +++ b/pkg/workflow/permissions_import_test.go @@ -9,8 +9,6 @@ import ( "testing" "github.com/github/gh-aw/pkg/testutil" - - "github.com/github/gh-aw/pkg/parser" ) func TestValidateIncludedPermissions(t *testing.T) { @@ -292,62 +290,3 @@ tools: } }) } - -func TestExtractPermissionsFromContent(t *testing.T) { - tests := []struct { - name string - content string - expected string - wantErr bool - }{ - { - name: "Simple permissions", - content: `--- -on: push -permissions: - contents: read - issues: read - pull-requests: read ---- -# Content`, - expected: `{"contents":"read","issues":"read","pull-requests":"read"}`, - wantErr: false, - }, - { - name: "No permissions", - content: `--- -on: issues ---- -# Content`, - expected: "{}", - wantErr: false, - }, - { - name: "Empty frontmatter", - content: `--- ---- -# Content`, - expected: "{}", - wantErr: false, - }, - { - name: "No frontmatter", - content: "# Just markdown content", - expected: "{}", - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result, err := parser.ExtractPermissionsFromContent(tt.content) - if (err != nil) != tt.wantErr { - t.Errorf("extractPermissionsFromContent() error = %v, wantErr %v", err, tt.wantErr) - return - } - if result != tt.expected { - t.Errorf("extractPermissionsFromContent() = %v, want %v", result, tt.expected) - } - }) - } -} diff --git a/pkg/workflow/strings.go b/pkg/workflow/strings.go index ffbc08b49b1..bb4f32639a0 100644 --- a/pkg/workflow/strings.go +++ b/pkg/workflow/strings.go @@ -263,45 +263,6 @@ func ShortenCommand(command string) string { return shortened } -// GenerateHeredocDelimiter creates a randomized heredoc delimiter with the GH_AW prefix. -// All heredoc delimiters in compiled lock.yml files use this format for security. -// -// The function generates delimiters in the format: GH_AW__<16_HEX_CHARS>_EOF -// -// The 16-character cryptographically random hex suffix prevents heredoc delimiter -// injection attacks: an attacker who knows the name cannot predict the full delimiter, -// so they cannot close the heredoc early to inject shell commands. -// -// Parameters: -// - name: A descriptive identifier for the heredoc content (e.g., "PROMPT", "MCP_CONFIG", "TOOLS_JSON") -// The name should use SCREAMING_SNAKE_CASE without the _EOF suffix. -// -// Returns a delimiter string in the format "GH_AW__<16_HEX_CHARS>_EOF" -// -// Example: -// -// GenerateHeredocDelimiter("PROMPT") // returns "GH_AW_PROMPT_a1b2c3d4e5f6g7h8_EOF" -// GenerateHeredocDelimiter("MCP_CONFIG") // returns "GH_AW_MCP_CONFIG_a1b2c3d4e5f6g7h8_EOF" -// GenerateHeredocDelimiter("") // returns "GH_AW_a1b2c3d4e5f6g7h8_EOF" -// -// Usage in heredoc generation: -// -// delimiter := GenerateHeredocDelimiter("PROMPT") -// yaml.WriteString(fmt.Sprintf("cat << '%s' >> \"$GH_AW_PROMPT\"\n", delimiter)) -// yaml.WriteString("content here\n") -// yaml.WriteString(delimiter + "\n") -func GenerateHeredocDelimiter(name string) string { - b := make([]byte, 8) - if _, err := rand.Read(b); err != nil { - panic("crypto/rand failed: " + err.Error()) - } - tag := hex.EncodeToString(b) // 16 hex chars - if name == "" { - return "GH_AW_" + tag + "_EOF" - } - return "GH_AW_" + strings.ToUpper(name) + "_" + tag + "_EOF" -} - // GenerateHeredocDelimiterFromSeed creates a stable heredoc delimiter derived from a seed // (typically the workflow frontmatter hash hex string) so that repeated compilations of the // same workflow produce identical lock files. diff --git a/pkg/workflow/strings_test.go b/pkg/workflow/strings_test.go index 20796e120a7..b0839dd02a0 100644 --- a/pkg/workflow/strings_test.go +++ b/pkg/workflow/strings_test.go @@ -5,7 +5,6 @@ package workflow import ( "regexp" "sort" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -508,159 +507,6 @@ func TestSanitizeName_NilOptions(t *testing.T) { } } -func TestGenerateHeredocDelimiter(t *testing.T) { - // validPattern matches the new randomized format: GH_AW__<16hexchars>_EOF - validPattern := regexp.MustCompile(`^GH_AW_[A-Z0-9_]+_[0-9a-f]{16}_EOF$`) - // emptyPattern matches the no-name variant: GH_AW_<16hexchars>_EOF - emptyPattern := regexp.MustCompile(`^GH_AW_[0-9a-f]{16}_EOF$`) - - tests := []struct { - name string - input string - nameInDelim string // expected uppercase name segment embedded in the delimiter - empty bool // true when input is "" - }{ - { - name: "simple name", - input: "PROMPT", - nameInDelim: "PROMPT", - }, - { - name: "multi-word name with underscores", - input: "MCP_CONFIG", - nameInDelim: "MCP_CONFIG", - }, - { - name: "tools json", - input: "TOOLS_JSON", - nameInDelim: "TOOLS_JSON", - }, - { - name: "SRT config", - input: "SRT_CONFIG", - nameInDelim: "SRT_CONFIG", - }, - { - name: "SRT wrapper", - input: "SRT_WRAPPER", - nameInDelim: "SRT_WRAPPER", - }, - { - name: "file with hash", - input: "FILE_123ABC", - nameInDelim: "FILE_123ABC", - }, - { - name: "mcp-scripts", - input: "MCP_SCRIPTS", - nameInDelim: "MCP_SCRIPTS", - }, - { - name: "JS file suffix", - input: "EOFJS_TOOL_NAME", - nameInDelim: "EOFJS_TOOL_NAME", - }, - { - name: "shell file suffix", - input: "EOFSH_TOOL_NAME", - nameInDelim: "EOFSH_TOOL_NAME", - }, - { - name: "python file suffix", - input: "EOFPY_TOOL_NAME", - nameInDelim: "EOFPY_TOOL_NAME", - }, - { - name: "go file suffix", - input: "EOFGO_TOOL_NAME", - nameInDelim: "EOFGO_TOOL_NAME", - }, - { - name: "lowercase input gets uppercased", - input: "prompt", - nameInDelim: "PROMPT", - }, - { - name: "mixed case input", - input: "Mcp_Config", - nameInDelim: "MCP_CONFIG", - }, - { - name: "empty string returns default", - input: "", - empty: true, - }, - { - name: "single character", - input: "A", - nameInDelim: "A", - }, - { - name: "numbers only", - input: "123", - nameInDelim: "123", - }, - { - name: "alphanumeric with underscores", - input: "CONFIG_V2_TEST", - nameInDelim: "CONFIG_V2_TEST", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := GenerateHeredocDelimiter(tt.input) - if tt.empty { - assert.True(t, emptyPattern.MatchString(result), - "GenerateHeredocDelimiter(%q) = %q, want format GH_AW_<16hex>_EOF", tt.input, result) - } else { - assert.True(t, validPattern.MatchString(result), - "GenerateHeredocDelimiter(%q) = %q, want format GH_AW_%s_<16hex>_EOF", tt.input, result, tt.nameInDelim) - assert.Contains(t, result, "GH_AW_"+tt.nameInDelim+"_", - "Delimiter should embed the uppercased name %q", tt.nameInDelim) - } - }) - } -} - -func TestGenerateHeredocDelimiter_Usage(t *testing.T) { - // Test that the delimiter matches the expected randomized format - delimiter := GenerateHeredocDelimiter("TEST") - - // Verify format is correct for heredoc usage - assert.True(t, strings.HasPrefix(delimiter, "GH_AW_TEST_"), "Delimiter should start with GH_AW_TEST_") - assert.True(t, strings.HasSuffix(delimiter, "_EOF"), "Delimiter should end with _EOF") - - // Test that it contains only alphanumeric and underscores (valid for heredoc) - // The delimiter includes a lowercase hex token, so allow lowercase hex chars too - validPattern := regexp.MustCompile(`^[A-Z0-9_a-f]+$`) - assert.True(t, validPattern.MatchString(delimiter), "Delimiter should contain only alphanumeric and underscores") - - // Verify the random hex token is present (16 hex chars between name and _EOF) - randomizedPattern := regexp.MustCompile(`^GH_AW_TEST_[0-9a-f]{16}_EOF$`) - assert.True(t, randomizedPattern.MatchString(delimiter), "Delimiter should match GH_AW_TEST_<16hex>_EOF format") -} - -func TestGenerateHeredocDelimiter_Uniqueness(t *testing.T) { - // Test that calling the function multiple times with same input produces unique delimiters - // (this is the security property: attacker cannot predict the delimiter) - input := "PROMPT" - result1 := GenerateHeredocDelimiter(input) - result2 := GenerateHeredocDelimiter(input) - result3 := GenerateHeredocDelimiter(input) - - // All should match the expected format - pattern := regexp.MustCompile(`^GH_AW_PROMPT_[0-9a-f]{16}_EOF$`) - assert.True(t, pattern.MatchString(result1), "result1 should match pattern, got %q", result1) - assert.True(t, pattern.MatchString(result2), "result2 should match pattern, got %q", result2) - assert.True(t, pattern.MatchString(result3), "result3 should match pattern, got %q", result3) - - // All three should be unique (probability of collision is 1/2^64 ≈ negligible) - assert.NotEqual(t, result1, result2, "GenerateHeredocDelimiter should produce unique delimiters") - assert.NotEqual(t, result2, result3, "GenerateHeredocDelimiter should produce unique delimiters") - assert.NotEqual(t, result1, result3, "GenerateHeredocDelimiter should produce unique delimiters") -} - func TestGenerateHeredocDelimiterFromSeed_Stability(t *testing.T) { // Sample SHA-256 hex string representing a typical workflow frontmatter hash. seed := "49266e50774d7e6a8b1c50f64b2f790c214dcdcf7b75b6bc8478bb43257b9863"