From 348f4c3478183a727f1f4e462744beec4b0c21bc Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 23 Aug 2025 21:23:09 -0700 Subject: [PATCH 1/8] Replace check-team-member shared action with inlined JavaScript script (#22) * Initial plan * Replace check-team-member template with inlined JavaScript script Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Fix code formatting (remove trailing whitespace) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Add check_team_member.cjs to tsconfig.json and create comprehensive tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux --- pkg/workflow/compiler.go | 36 +-- pkg/workflow/js.go | 3 + pkg/workflow/js/check_team_member.cjs | 30 ++ pkg/workflow/js/check_team_member.test.cjs | 271 ++++++++++++++++++ pkg/workflow/templates/check_team_member.yaml | 39 --- tsconfig.json | 1 + 6 files changed, 316 insertions(+), 64 deletions(-) create mode 100644 pkg/workflow/js/check_team_member.cjs create mode 100644 pkg/workflow/js/check_team_member.test.cjs delete mode 100644 pkg/workflow/templates/check_team_member.yaml diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 97cae893c0..9314a5efd7 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -35,9 +35,6 @@ var reactionActionTemplate string //go:embed templates/compute_text_action.yaml var computeTextActionTemplate string -//go:embed templates/check_team_member.yaml -var checkTeamMemberTemplate string - // Compiler handles converting markdown workflows to GitHub Actions YAML type Compiler struct { verbose bool @@ -281,22 +278,6 @@ func (c *Compiler) CompileWorkflow(markdownPath string) error { } } - // Write shared check-team-member action (only for alias workflows) - if workflowData.Alias != "" { - if err := c.writeCheckTeamMemberAction(markdownPath); err != nil { - formattedErr := console.FormatError(console.CompilerError{ - Position: console.ErrorPosition{ - File: markdownPath, - Line: 1, - Column: 1, - }, - Type: "error", - Message: fmt.Sprintf("failed to write check-team-member action: %v", err), - }) - return errors.New(formattedErr) - } - } - // Generate the YAML content if c.verbose { fmt.Println(console.FormatInfoMessage("Generating GitHub Actions YAML...")) @@ -1594,8 +1575,18 @@ func (c *Compiler) buildTaskJob(data *WorkflowData) (*Job, error) { steps = append(steps, " - name: Check team membership for alias workflow\n") steps = append(steps, " id: check-team-member\n") - steps = append(steps, " uses: ./.github/actions/check-team-member\n") steps = append(steps, fmt.Sprintf(" if: %s\n", aliasConditionStr)) + steps = append(steps, " uses: actions/github-script@v7\n") + steps = append(steps, " with:\n") + steps = append(steps, " script: |\n") + + // Inline the JavaScript code with proper indentation + scriptLines := strings.Split(checkTeamMemberScript, "\n") + for _, line := range scriptLines { + if strings.TrimSpace(line) != "" { + steps = append(steps, fmt.Sprintf(" %s\n", line)) + } + } steps = append(steps, " - name: Validate team membership\n") steps = append(steps, fmt.Sprintf(" if: %s\n", validationConditionStr)) steps = append(steps, " run: |\n") @@ -2093,11 +2084,6 @@ func (c *Compiler) writeComputeTextAction(markdownPath string) error { return c.writeSharedAction(markdownPath, "compute-text", computeTextActionTemplate, "compute-text") } -// writeCheckTeamMemberAction writes the shared check-team-member action -func (c *Compiler) writeCheckTeamMemberAction(markdownPath string) error { - return c.writeSharedAction(markdownPath, "check-team-member", checkTeamMemberTemplate, "check-team-member") -} - // generateMainJobSteps generates the steps section for the main job func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowData) { // Add custom steps or default checkout step diff --git a/pkg/workflow/js.go b/pkg/workflow/js.go index 1d057f92c6..6970e0f88d 100644 --- a/pkg/workflow/js.go +++ b/pkg/workflow/js.go @@ -24,6 +24,9 @@ var addLabelsScript string //go:embed js/setup_agent_output.cjs var setupAgentOutputScript string +//go:embed js/check_team_member.cjs +var checkTeamMemberScript string + // FormatJavaScriptForYAML formats a JavaScript script with proper indentation for embedding in YAML func FormatJavaScriptForYAML(script string) []string { var formattedLines []string diff --git a/pkg/workflow/js/check_team_member.cjs b/pkg/workflow/js/check_team_member.cjs new file mode 100644 index 0000000000..e4e7e4a238 --- /dev/null +++ b/pkg/workflow/js/check_team_member.cjs @@ -0,0 +1,30 @@ +async function main() { + const actor = context.actor; + const { owner, repo } = context.repo; + + // Check if the actor has repository access (admin, maintain permissions) + try { + console.log(`Checking if user '${actor}' is admin or maintainer of ${owner}/${repo}`); + + const repoPermission = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: owner, + repo: repo, + username: actor + }); + + const permission = repoPermission.data.permission; + console.log(`Repository permission level: ${permission}`); + + if (permission === 'admin' || permission === 'maintain') { + console.log(`User has ${permission} access to repository`); + core.setOutput('is_team_member', 'true'); + return; + } + } catch (repoError) { + const errorMessage = repoError instanceof Error ? repoError.message : String(repoError); + console.log(`Repository permission check failed: ${errorMessage}`); + } + + core.setOutput('is_team_member', 'false'); +} +await main(); \ No newline at end of file diff --git a/pkg/workflow/js/check_team_member.test.cjs b/pkg/workflow/js/check_team_member.test.cjs new file mode 100644 index 0000000000..0cd95ac556 --- /dev/null +++ b/pkg/workflow/js/check_team_member.test.cjs @@ -0,0 +1,271 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import fs from 'fs'; +import path from 'path'; + +// Mock the global objects that GitHub Actions provides +const mockCore = { + setOutput: vi.fn() +}; + +const mockGithub = { + rest: { + repos: { + getCollaboratorPermissionLevel: vi.fn() + } + } +}; + +const mockContext = { + actor: 'testuser', + repo: { + owner: 'testowner', + repo: 'testrepo' + } +}; + +// Set up global variables +global.core = mockCore; +global.github = mockGithub; +global.context = mockContext; + +describe('check_team_member.cjs', () => { + let checkTeamMemberScript; + + beforeEach(() => { + // Reset all mocks + vi.clearAllMocks(); + + // Reset context to default state + global.context.actor = 'testuser'; + global.context.repo = { + owner: 'testowner', + repo: 'testrepo' + }; + + // Read the script content + const scriptPath = path.join(process.cwd(), 'pkg/workflow/js/check_team_member.cjs'); + checkTeamMemberScript = fs.readFileSync(scriptPath, 'utf8'); + }); + + it('should set is_team_member to true for admin permission', async () => { + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'admin' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: 'testowner', + repo: 'testrepo', + username: 'testuser' + }); + + expect(consoleSpy).toHaveBeenCalledWith('Checking if user \'testuser\' is admin or maintainer of testowner/testrepo'); + expect(consoleSpy).toHaveBeenCalledWith('Repository permission level: admin'); + expect(consoleSpy).toHaveBeenCalledWith('User has admin access to repository'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'true'); + + consoleSpy.mockRestore(); + }); + + it('should set is_team_member to true for maintain permission', async () => { + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'maintain' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: 'testowner', + repo: 'testrepo', + username: 'testuser' + }); + + expect(consoleSpy).toHaveBeenCalledWith('Checking if user \'testuser\' is admin or maintainer of testowner/testrepo'); + expect(consoleSpy).toHaveBeenCalledWith('Repository permission level: maintain'); + expect(consoleSpy).toHaveBeenCalledWith('User has maintain access to repository'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'true'); + + consoleSpy.mockRestore(); + }); + + it('should set is_team_member to false for write permission', async () => { + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'write' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: 'testowner', + repo: 'testrepo', + username: 'testuser' + }); + + expect(consoleSpy).toHaveBeenCalledWith('Checking if user \'testuser\' is admin or maintainer of testowner/testrepo'); + expect(consoleSpy).toHaveBeenCalledWith('Repository permission level: write'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'false'); + + consoleSpy.mockRestore(); + }); + + it('should set is_team_member to false for read permission', async () => { + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'read' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: 'testowner', + repo: 'testrepo', + username: 'testuser' + }); + + expect(consoleSpy).toHaveBeenCalledWith('Checking if user \'testuser\' is admin or maintainer of testowner/testrepo'); + expect(consoleSpy).toHaveBeenCalledWith('Repository permission level: read'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'false'); + + consoleSpy.mockRestore(); + }); + + it('should set is_team_member to false for none permission', async () => { + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'none' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: 'testowner', + repo: 'testrepo', + username: 'testuser' + }); + + expect(consoleSpy).toHaveBeenCalledWith('Checking if user \'testuser\' is admin or maintainer of testowner/testrepo'); + expect(consoleSpy).toHaveBeenCalledWith('Repository permission level: none'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'false'); + + consoleSpy.mockRestore(); + }); + + it('should handle API errors and set is_team_member to false', async () => { + const apiError = new Error('API Error: Not Found'); + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(apiError); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: 'testowner', + repo: 'testrepo', + username: 'testuser' + }); + + expect(consoleSpy).toHaveBeenCalledWith('Checking if user \'testuser\' is admin or maintainer of testowner/testrepo'); + expect(consoleSpy).toHaveBeenCalledWith('Repository permission check failed: API Error: Not Found'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'false'); + + consoleSpy.mockRestore(); + }); + + it('should handle different actor names correctly', async () => { + global.context.actor = 'different-user'; + + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'admin' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: 'testowner', + repo: 'testrepo', + username: 'different-user' + }); + + expect(consoleSpy).toHaveBeenCalledWith('Checking if user \'different-user\' is admin or maintainer of testowner/testrepo'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'true'); + + consoleSpy.mockRestore(); + }); + + it('should handle different repository contexts correctly', async () => { + global.context.repo = { + owner: 'different-owner', + repo: 'different-repo' + }; + + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'maintain' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: 'different-owner', + repo: 'different-repo', + username: 'testuser' + }); + + expect(consoleSpy).toHaveBeenCalledWith('Checking if user \'testuser\' is admin or maintainer of different-owner/different-repo'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'true'); + + consoleSpy.mockRestore(); + }); + + it('should handle authentication errors gracefully', async () => { + const authError = new Error('Bad credentials'); + authError.status = 401; + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(authError); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('Repository permission check failed: Bad credentials'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'false'); + + consoleSpy.mockRestore(); + }); + + it('should handle rate limiting errors gracefully', async () => { + const rateLimitError = new Error('API rate limit exceeded'); + rateLimitError.status = 403; + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(rateLimitError); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + // Execute the script + await eval(`(async () => { ${checkTeamMemberScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('Repository permission check failed: API rate limit exceeded'); + expect(mockCore.setOutput).toHaveBeenCalledWith('is_team_member', 'false'); + + consoleSpy.mockRestore(); + }); +}); \ No newline at end of file diff --git a/pkg/workflow/templates/check_team_member.yaml b/pkg/workflow/templates/check_team_member.yaml deleted file mode 100644 index 6fe7c681d5..0000000000 --- a/pkg/workflow/templates/check_team_member.yaml +++ /dev/null @@ -1,39 +0,0 @@ -name: "Check if actor is team member" -description: "Checks if the current GitHub actor is a member of the repository's organization/team" -outputs: - is_team_member: - description: "Boolean indicating if the actor is a team member (true/false)" -runs: - using: "composite" - steps: - - name: Check team membership - uses: actions/github-script@v7 - with: - script: | - const actor = context.actor; - const { owner, repo } = context.repo; - - // Check if the actor has repository access (admin, maintain permissions) - try { - console.log(`Checking if user '${actor}' is admin or maintainer of ${owner}/${repo}`); - - const repoPermission = await github.rest.repos.getCollaboratorPermissionLevel({ - owner: owner, - repo: repo, - username: actor - }); - - const permission = repoPermission.data.permission; - console.log(`Repository permission level: ${permission}`); - - if (permission === 'admin' || permission === 'maintain') { - console.log(`User has ${permission} access to repository`); - core.setOutput('is_team_member', 'true'); - return; - } - } catch (repoError) { - console.log(`Repository permission check failed: ${repoError.message}`); - } - - core.setOutput('is_team_member', 'false'); - diff --git a/tsconfig.json b/tsconfig.json index e3fefd8069..4b6dc6412c 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -33,6 +33,7 @@ }, "include": [ "pkg/workflow/js/add_labels.cjs", + "pkg/workflow/js/check_team_member.cjs", "pkg/workflow/js/create_comment.cjs", "pkg/workflow/js/create_issue.cjs", "pkg/workflow/js/create_pull_request.cjs", From ff2426901907f8e508085bb0ac6de8f1e256533b Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 Aug 2025 08:27:01 -0700 Subject: [PATCH 2/8] Refactor compute_text_action.yaml as embedded JavaScript with sanitization (#25) --- pkg/workflow/compiler.go | 25 +- pkg/workflow/js.go | 3 + pkg/workflow/js/compute_text.cjs | 218 ++++++++++++ pkg/workflow/js/compute_text.test.cjs | 312 ++++++++++++++++++ .../templates/compute_text_action.yaml | 90 ----- tsconfig.json | 1 + 6 files changed, 555 insertions(+), 94 deletions(-) create mode 100644 pkg/workflow/js/compute_text.cjs create mode 100644 pkg/workflow/js/compute_text.test.cjs delete mode 100644 pkg/workflow/templates/compute_text_action.yaml diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 9314a5efd7..981d4e5cef 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -32,9 +32,6 @@ type FileTracker interface { //go:embed templates/reaction_action.yaml var reactionActionTemplate string -//go:embed templates/compute_text_action.yaml -var computeTextActionTemplate string - // Compiler handles converting markdown workflows to GitHub Actions YAML type Compiler struct { verbose bool @@ -2081,7 +2078,27 @@ func (c *Compiler) writeReactionAction(markdownPath string) error { // writeComputeTextAction writes the shared compute-text action func (c *Compiler) writeComputeTextAction(markdownPath string) error { - return c.writeSharedAction(markdownPath, "compute-text", computeTextActionTemplate, "compute-text") + // Generate the action content with embedded JavaScript + var actionContent strings.Builder + + actionContent.WriteString("name: \"Compute current body text\"\n") + actionContent.WriteString("description: \"Computes the current body text based on the GitHub event context\"\n") + actionContent.WriteString("outputs:\n") + actionContent.WriteString(" text:\n") + actionContent.WriteString(" description: \"The computed current body text based on event type\"\n") + actionContent.WriteString("runs:\n") + actionContent.WriteString(" using: \"composite\"\n") + actionContent.WriteString(" steps:\n") + actionContent.WriteString(" - name: Compute current body text\n") + actionContent.WriteString(" id: compute-text\n") + actionContent.WriteString(" uses: actions/github-script@v7\n") + actionContent.WriteString(" with:\n") + actionContent.WriteString(" script: |\n") + + // Embed the JavaScript with proper indentation + WriteJavaScriptToYAML(&actionContent, computeTextScript) + + return c.writeSharedAction(markdownPath, "compute-text", actionContent.String(), "compute-text") } // generateMainJobSteps generates the steps section for the main job diff --git a/pkg/workflow/js.go b/pkg/workflow/js.go index 6970e0f88d..cbf6ef8582 100644 --- a/pkg/workflow/js.go +++ b/pkg/workflow/js.go @@ -15,6 +15,9 @@ var createIssueScript string //go:embed js/create_comment.cjs var createCommentScript string +//go:embed js/compute_text.cjs +var computeTextScript string + //go:embed js/sanitize_output.cjs var sanitizeOutputScript string diff --git a/pkg/workflow/js/compute_text.cjs b/pkg/workflow/js/compute_text.cjs new file mode 100644 index 0000000000..94a5205cce --- /dev/null +++ b/pkg/workflow/js/compute_text.cjs @@ -0,0 +1,218 @@ +/** + * Sanitizes content for safe output in GitHub Actions + * @param {string} content - The content to sanitize + * @returns {string} The sanitized content + */ +function sanitizeContent(content) { + if (!content || typeof content !== 'string') { + return ''; + } + + // Read allowed domains from environment variable + const allowedDomainsEnv = process.env.GITHUB_AW_ALLOWED_DOMAINS; + const defaultAllowedDomains = [ + 'github.com', + 'github.io', + 'githubusercontent.com', + 'githubassets.com', + 'github.dev', + 'codespaces.new' + ]; + + const allowedDomains = allowedDomainsEnv + ? allowedDomainsEnv.split(',').map(d => d.trim()).filter(d => d) + : defaultAllowedDomains; + + let sanitized = content; + + // Neutralize @mentions to prevent unintended notifications + sanitized = neutralizeMentions(sanitized); + + // Remove control characters (except newlines and tabs) + sanitized = sanitized.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, ''); + + // XML character escaping + sanitized = sanitized + .replace(/&/g, '&') // Must be first to avoid double-escaping + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + + // URI filtering - replace non-https protocols with "(redacted)" + // Step 1: Temporarily mark HTTPS URLs to protect them + sanitized = sanitizeUrlProtocols(sanitized); + + // Domain filtering for HTTPS URIs + // Match https:// URIs and check if domain is in allowlist + sanitized = sanitizeUrlDomains(sanitized); + + // Limit total length to prevent DoS (0.5MB max) + const maxLength = 524288; + if (sanitized.length > maxLength) { + sanitized = sanitized.substring(0, maxLength) + '\n[Content truncated due to length]'; + } + + // Limit number of lines to prevent log flooding (65k max) + const lines = sanitized.split('\n'); + const maxLines = 65000; + if (lines.length > maxLines) { + sanitized = lines.slice(0, maxLines).join('\n') + '\n[Content truncated due to line count]'; + } + + // Remove ANSI escape sequences + sanitized = sanitized.replace(/\x1b\[[0-9;]*[mGKH]/g, ''); + + // Neutralize common bot trigger phrases + sanitized = neutralizeBotTriggers(sanitized); + + // Trim excessive whitespace + return sanitized.trim(); + + /** + * Remove unknown domains + * @param {string} s - The string to process + * @returns {string} The string with unknown domains redacted + */ + function sanitizeUrlDomains(s) { + s = s.replace(/\bhttps:\/\/([^\/\s\])}'"<>&\x00-\x1f]+)/gi, (match, domain) => { + // Extract the hostname part (before first slash, colon, or other delimiter) + const hostname = domain.split(/[\/:\?#]/)[0].toLowerCase(); + + // Check if this domain or any parent domain is in the allowlist + const isAllowed = allowedDomains.some(allowedDomain => { + const normalizedAllowed = allowedDomain.toLowerCase(); + return hostname === normalizedAllowed || hostname.endsWith('.' + normalizedAllowed); + }); + + return isAllowed ? match : '(redacted)'; + }); + + return s; + } + + /** + * Remove unknown protocols except https + * @param {string} s - The string to process + * @returns {string} The string with non-https protocols redacted + */ + function sanitizeUrlProtocols(s) { + // Match both protocol:// and protocol: patterns + // This covers URLs like https://example.com, javascript:alert(), mailto:user@domain.com, etc. + return s.replace(/\b(\w+):(?:\/\/)?[^\s\])}'"<>&\x00-\x1f]+/gi, (match, protocol) => { + // Allow https (case insensitive), redact everything else + return protocol.toLowerCase() === 'https' ? match : '(redacted)'; + }); + } + + /** + * Neutralizes @mentions by wrapping them in backticks + * @param {string} s - The string to process + * @returns {string} The string with neutralized mentions + */ + function neutralizeMentions(s) { + // Replace @name or @org/team outside code with `@name` + return s.replace(/(^|[^\w`])@([A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, + (_m, p1, p2) => `${p1}\`@${p2}\``); + } + + /** + * Neutralizes bot trigger phrases by wrapping them in backticks + * @param {string} s - The string to process + * @returns {string} The string with neutralized bot triggers + */ + function neutralizeBotTriggers(s) { + // Neutralize common bot trigger phrases like "fixes #123", "closes #asdfs", etc. + return s.replace(/\b(fixes?|closes?|resolves?|fix|close|resolve)\s+#(\w+)/gi, + (match, action, ref) => `\`${action} #${ref}\``); + } +} + +async function main() { + let text = ''; + + const actor = context.actor; + const { owner, repo } = context.repo; + + // Check if the actor has repository access (admin, maintain permissions) + const repoPermission = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: owner, + repo: repo, + username: actor + }); + + const permission = repoPermission.data.permission; + console.log(`Repository permission level: ${permission}`); + + if (permission !== 'admin' && permission !== 'maintain') { + core.setOutput('text', ''); + return; + } + + // Determine current body text based on event context + switch (context.eventName) { + case 'issues': + // For issues: title + body + if (context.payload.issue) { + const title = context.payload.issue.title || ''; + const body = context.payload.issue.body || ''; + text = `${title}\n\n${body}`; + } + break; + + case 'pull_request': + // For pull requests: title + body + if (context.payload.pull_request) { + const title = context.payload.pull_request.title || ''; + const body = context.payload.pull_request.body || ''; + text = `${title}\n\n${body}`; + } + break; + + case 'pull_request_target': + // For pull request target events: title + body + if (context.payload.pull_request) { + const title = context.payload.pull_request.title || ''; + const body = context.payload.pull_request.body || ''; + text = `${title}\n\n${body}`; + } + break; + + case 'issue_comment': + // For issue comments: comment body + if (context.payload.comment) { + text = context.payload.comment.body || ''; + } + break; + + case 'pull_request_review_comment': + // For PR review comments: comment body + if (context.payload.comment) { + text = context.payload.comment.body || ''; + } + break; + + case 'pull_request_review': + // For PR reviews: review body + if (context.payload.review) { + text = context.payload.review.body || ''; + } + break; + + default: + // Default: empty text + text = ''; + break; + } + + // Sanitize the text before output + const sanitizedText = sanitizeContent(text); + + // Display sanitized text in logs + console.log(`text: ${sanitizedText}`); + + // Set the sanitized text as output + core.setOutput('text', sanitizedText); +} + +await main(); \ No newline at end of file diff --git a/pkg/workflow/js/compute_text.test.cjs b/pkg/workflow/js/compute_text.test.cjs new file mode 100644 index 0000000000..73a492b1a4 --- /dev/null +++ b/pkg/workflow/js/compute_text.test.cjs @@ -0,0 +1,312 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'fs'; +import path from 'path'; + +// Mock the global objects that GitHub Actions provides +const mockCore = { + setOutput: vi.fn() +}; + +const mockGithub = { + rest: { + repos: { + getCollaboratorPermissionLevel: vi.fn() + } + } +}; + +const mockContext = { + actor: 'test-user', + repo: { + owner: 'test-owner', + repo: 'test-repo' + }, + eventName: 'issues', + payload: {} +}; + +// Set up global variables +global.core = mockCore; +global.github = mockGithub; +global.context = mockContext; + +describe('compute_text.cjs', () => { + let computeTextScript; + let sanitizeContentFunction; + + beforeEach(() => { + // Reset all mocks + vi.clearAllMocks(); + + // Reset context + mockContext.eventName = 'issues'; + mockContext.payload = {}; + + // Reset environment variables + delete process.env.GITHUB_AW_ALLOWED_DOMAINS; + + // Read the script content + const scriptPath = path.join(process.cwd(), 'pkg/workflow/js/compute_text.cjs'); + computeTextScript = fs.readFileSync(scriptPath, 'utf8'); + + // Extract sanitizeContent function for unit testing + // We need to eval the script to get access to the function + const scriptWithExport = computeTextScript.replace( + 'await main();', + 'global.testSanitizeContent = sanitizeContent; global.testMain = main;' + ); + eval(scriptWithExport); + sanitizeContentFunction = global.testSanitizeContent; + }); + + describe('sanitizeContent function', () => { + it('should handle null and undefined inputs', () => { + expect(sanitizeContentFunction(null)).toBe(''); + expect(sanitizeContentFunction(undefined)).toBe(''); + expect(sanitizeContentFunction('')).toBe(''); + }); + + it('should neutralize @mentions by wrapping in backticks', () => { + const input = 'Hello @user and @org/team'; + const result = sanitizeContentFunction(input); + expect(result).toContain('`@user`'); + expect(result).toContain('`@org/team`'); + }); + + it('should neutralize bot trigger phrases', () => { + const input = 'This fixes #123 and closes #456'; + const result = sanitizeContentFunction(input); + expect(result).toContain('`fixes #123`'); + expect(result).toContain('`closes #456`'); + }); + + it('should remove control characters', () => { + const input = 'Hello\x00\x01\x08world\x7F'; + const result = sanitizeContentFunction(input); + expect(result).toBe('Helloworld'); + }); + + it('should escape XML characters', () => { + const input = 'Test content & "quotes"'; + const result = sanitizeContentFunction(input); + expect(result).toContain('<tag>'); + expect(result).toContain('&'); + expect(result).toContain('"quotes"'); + }); + + it('should redact non-https protocols', () => { + const input = 'Visit http://example.com or ftp://files.com'; + const result = sanitizeContentFunction(input); + expect(result).toContain('(redacted)'); + expect(result).not.toContain('http://example.com'); + }); + + it('should allow github.com domains', () => { + const input = 'Visit https://github.com/user/repo'; + const result = sanitizeContentFunction(input); + expect(result).toContain('https://github.com/user/repo'); + }); + + it('should redact unknown domains', () => { + const input = 'Visit https://evil.com/malware'; + const result = sanitizeContentFunction(input); + expect(result).toContain('(redacted)'); + expect(result).not.toContain('evil.com'); + }); + + it('should truncate long content', () => { + const longContent = 'a'.repeat(600000); // Exceed 524288 limit + const result = sanitizeContentFunction(longContent); + expect(result.length).toBeLessThan(600000); + expect(result).toContain('[Content truncated due to length]'); + }); + + it('should truncate too many lines', () => { + const manyLines = Array(70000).fill('line').join('\n'); // Exceed 65000 limit + const result = sanitizeContentFunction(manyLines); + expect(result.split('\n').length).toBeLessThan(70000); + expect(result).toContain('[Content truncated due to line count]'); + }); + + it('should remove ANSI escape sequences', () => { + const input = 'Hello \u001b[31mred\u001b[0m world'; + const result = sanitizeContentFunction(input); + // ANSI sequences should be removed, allowing for possible differences in regex matching + expect(result).toMatch(/Hello.*red.*world/); + expect(result).not.toMatch(/\u001b\[/); + }); + + it('should respect custom allowed domains', () => { + process.env.GITHUB_AW_ALLOWED_DOMAINS = 'example.com,trusted.org'; + const input = 'Visit https://example.com and https://trusted.org and https://evil.com'; + const result = sanitizeContentFunction(input); + expect(result).toContain('https://example.com'); + expect(result).toContain('https://trusted.org'); + expect(result).toContain('(redacted)'); // for evil.com + }); + }); + + describe('main function', () => { + let testMain; + + beforeEach(() => { + // Set up default successful permission check + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'admin' } + }); + + // Get the main function from global scope + testMain = global.testMain; + }); + + it('should extract text from issue payload', async () => { + mockContext.eventName = 'issues'; + mockContext.payload = { + issue: { + title: 'Test Issue', + body: 'Issue description' + } + }; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', 'Test Issue\n\nIssue description'); + }); + + it('should extract text from pull request payload', async () => { + mockContext.eventName = 'pull_request'; + mockContext.payload = { + pull_request: { + title: 'Test PR', + body: 'PR description' + } + }; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', 'Test PR\n\nPR description'); + }); + + it('should extract text from issue comment payload', async () => { + mockContext.eventName = 'issue_comment'; + mockContext.payload = { + comment: { + body: 'This is a comment' + } + }; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', 'This is a comment'); + }); + + it('should extract text from pull request target payload', async () => { + mockContext.eventName = 'pull_request_target'; + mockContext.payload = { + pull_request: { + title: 'Test PR Target', + body: 'PR target description' + } + }; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', 'Test PR Target\n\nPR target description'); + }); + + it('should extract text from pull request review comment payload', async () => { + mockContext.eventName = 'pull_request_review_comment'; + mockContext.payload = { + comment: { + body: 'Review comment' + } + }; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', 'Review comment'); + }); + + it('should extract text from pull request review payload', async () => { + mockContext.eventName = 'pull_request_review'; + mockContext.payload = { + review: { + body: 'Review body' + } + }; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', 'Review body'); + }); + + it('should handle unknown event types', async () => { + mockContext.eventName = 'unknown_event'; + mockContext.payload = {}; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', ''); + }); + + it('should deny access for non-admin/maintain users', async () => { + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: 'read' } + }); + + mockContext.eventName = 'issues'; + mockContext.payload = { + issue: { + title: 'Test Issue', + body: 'Issue description' + } + }; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', ''); + }); + + it('should sanitize extracted text before output', async () => { + mockContext.eventName = 'issues'; + mockContext.payload = { + issue: { + title: 'Test @user fixes #123', + body: 'Visit https://evil.com' + } + }; + + await testMain(); + + const outputCall = mockCore.setOutput.mock.calls[0]; + expect(outputCall[1]).toContain('`@user`'); + expect(outputCall[1]).toContain('`fixes #123`'); + expect(outputCall[1]).toContain('(redacted)'); + }); + + it('should handle missing title and body gracefully', async () => { + mockContext.eventName = 'issues'; + mockContext.payload = { + issue: {} // No title or body + }; + + await testMain(); + + // Since empty strings get sanitized/trimmed, expect empty string + expect(mockCore.setOutput).toHaveBeenCalledWith('text', ''); + }); + + it('should handle null values in payload', async () => { + mockContext.eventName = 'issue_comment'; + mockContext.payload = { + comment: { + body: null + } + }; + + await testMain(); + + expect(mockCore.setOutput).toHaveBeenCalledWith('text', ''); + }); + }); +}); \ No newline at end of file diff --git a/pkg/workflow/templates/compute_text_action.yaml b/pkg/workflow/templates/compute_text_action.yaml deleted file mode 100644 index 5e0f3f14ea..0000000000 --- a/pkg/workflow/templates/compute_text_action.yaml +++ /dev/null @@ -1,90 +0,0 @@ -name: "Compute current body text" -description: "Computes the current body text based on the GitHub event context" -outputs: - text: - description: "The computed current body text based on event type" -runs: - using: "composite" - steps: - - name: Compute current body text - id: compute-text - uses: actions/github-script@v7 - with: - script: | - let text = ''; - - const actor = context.actor; - const { owner, repo } = context.repo; - - // Check if the actor has repository access (admin, maintain permissions) - try { - const repoPermission = await github.rest.repos.getCollaboratorPermissionLevel({ - owner: owner, - repo: repo, - username: actor - }); - - const permission = repoPermission.data.permission; - console.log(`Repository permission level: ${permission}`); - - if (permission !== 'admin' && permission !== 'maintain') { - return; - } - } catch (repoError) { - console.log(`Repository permission check failed: ${repoError.message}`); - core.setOutput('text', ''); - return; - } - - // Determine current body text based on event context - switch (context.eventName) { - case 'issues': - // For issues: title + body - if (context.payload.issue) { - const title = context.payload.issue.title || ''; - const body = context.payload.issue.body || ''; - text = `${title}\n\n${body}`; - } - break; - - case 'pull_request': - // For pull requests: title + body - if (context.payload.pull_request) { - const title = context.payload.pull_request.title || ''; - const body = context.payload.pull_request.body || ''; - text = `${title}\n\n${body}`; - } - break; - - case 'issue_comment': - // For issue comments: comment body - if (context.payload.comment) { - text = context.payload.comment.body || ''; - } - break; - - case 'pull_request_review_comment': - // For PR review comments: comment body - if (context.payload.comment) { - text = context.payload.comment.body || ''; - } - break; - - case 'pull_request_review': - // For PR reviews: review body - if (context.payload.review) { - text = context.payload.review.body || ''; - } - break; - - default: - // Default: empty text - text = ''; - break; - } - - // display in logs - console.log(`text: ${text}`); - - // Set the text as output - core.setOutput('text', text); \ No newline at end of file diff --git a/tsconfig.json b/tsconfig.json index 4b6dc6412c..d51267efcd 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -34,6 +34,7 @@ "include": [ "pkg/workflow/js/add_labels.cjs", "pkg/workflow/js/check_team_member.cjs", + "pkg/workflow/js/compute_text.cjs", "pkg/workflow/js/create_comment.cjs", "pkg/workflow/js/create_issue.cjs", "pkg/workflow/js/create_pull_request.cjs", From 8f28fe495a83a252c99490150cb32bcec047308c Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 Aug 2025 08:39:42 -0700 Subject: [PATCH 3/8] Simplify add-reaction feature by removing fallback and try/catch logic (#24) * Initial plan * Reimplement add-reaction feature as inlined JavaScript Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Remove "remove" functionality from add-reaction feature, simplify to only support adding reactions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Remove fallback and try/catch clauses from add-reaction feature Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux --- pkg/workflow/compiler.go | 42 +-- .../compiler_additional_simple_test.go | 38 --- pkg/workflow/compiler_test.go | 5 +- pkg/workflow/js.go | 3 + pkg/workflow/js/add_reaction.cjs | 99 ++++++ pkg/workflow/js/add_reaction.test.cjs | 319 ++++++++++++++++++ tsconfig.json | 1 + 7 files changed, 438 insertions(+), 69 deletions(-) create mode 100644 pkg/workflow/js/add_reaction.cjs create mode 100644 pkg/workflow/js/add_reaction.test.cjs diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 981d4e5cef..16b736cf8a 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -29,6 +29,9 @@ type FileTracker interface { TrackCreated(filePath string) } +//go:embed templates/compute_text_action.yaml +var computeTextActionTemplate string + //go:embed templates/reaction_action.yaml var reactionActionTemplate string @@ -243,22 +246,6 @@ func (c *Compiler) CompileWorkflow(markdownPath string) error { } } - // Write shared reaction action only if ai-reaction is configured - if workflowData.AIReaction != "" { - if err := c.writeReactionAction(markdownPath); err != nil { - formattedErr := console.FormatError(console.CompilerError{ - Position: console.ErrorPosition{ - File: markdownPath, - Line: 1, - Column: 1, - }, - Type: "error", - Message: fmt.Sprintf("failed to write reaction action: %v", err), - }) - return errors.New(formattedErr) - } - } - // Write shared compute-text action (only if needed for task job) if workflowData.NeedsTextOutput { if err := c.writeComputeTextAction(markdownPath); err != nil { @@ -1618,16 +1605,20 @@ func (c *Compiler) buildAddReactionJob(data *WorkflowData, taskJobCreated bool) reactionCondition := buildReactionCondition() var steps []string - steps = append(steps, " - uses: actions/checkout@v5\n") - steps = append(steps, " with:\n") - steps = append(steps, " sparse-checkout: .github\n") steps = append(steps, fmt.Sprintf(" - name: Add %s reaction to the triggering item\n", data.AIReaction)) steps = append(steps, " id: react\n") - steps = append(steps, " uses: ./.github/actions/reaction\n") + steps = append(steps, " uses: actions/github-script@v7\n") + + // Add environment variables + steps = append(steps, " env:\n") + steps = append(steps, fmt.Sprintf(" GITHUB_AW_REACTION: %s\n", data.AIReaction)) + steps = append(steps, " with:\n") - steps = append(steps, " github-token: ${{ secrets.GITHUB_TOKEN }}\n") - steps = append(steps, " mode: add\n") - steps = append(steps, fmt.Sprintf(" reaction: %s\n", data.AIReaction)) + steps = append(steps, " script: |\n") + + // Add each line of the script with proper indentation + formattedScript := FormatJavaScriptForYAML(addReactionScript) + steps = append(steps, formattedScript...) outputs := map[string]string{ "reaction_id": "${{ steps.react.outputs.reaction-id }}", @@ -2071,11 +2062,6 @@ func (c *Compiler) writeSharedAction(markdownPath string, actionPath string, con return nil } -// writeReactionAction writes the shared reaction action if ai-reaction is used -func (c *Compiler) writeReactionAction(markdownPath string) error { - return c.writeSharedAction(markdownPath, "reaction", reactionActionTemplate, "reaction") -} - // writeComputeTextAction writes the shared compute-text action func (c *Compiler) writeComputeTextAction(markdownPath string) error { // Generate the action content with embedded JavaScript diff --git a/pkg/workflow/compiler_additional_simple_test.go b/pkg/workflow/compiler_additional_simple_test.go index fe25518d1c..584d36e5a6 100644 --- a/pkg/workflow/compiler_additional_simple_test.go +++ b/pkg/workflow/compiler_additional_simple_test.go @@ -1,8 +1,6 @@ package workflow import ( - "os" - "path/filepath" "testing" ) @@ -35,42 +33,6 @@ func TestCompiler_SetFileTracker_Basic(t *testing.T) { } } -func TestCompiler_WriteReactionAction_Basic(t *testing.T) { - // Create compiler - compiler := NewCompiler(false, "", "test-version") - - // Create temporary directory for testing - tmpDir := t.TempDir() - - // Create a test markdown file path (doesn't need to actually exist) - markdownPath := filepath.Join(tmpDir, "test.md") - - // Set up file tracker to verify file creation - mockTracker := &SimpleBasicMockFileTracker{} - compiler.SetFileTracker(mockTracker) - - // Test that writeReactionAction succeeds - err := compiler.writeReactionAction(markdownPath) - if err != nil { - t.Errorf("Expected no error, got: %v", err) - } - - // Verify that the action file was created - expectedActionFile := filepath.Join(tmpDir, ".github", "actions", "reaction", "action.yml") - if _, err := os.Stat(expectedActionFile); os.IsNotExist(err) { - t.Errorf("Expected action file to be created at: %s", expectedActionFile) - } - - // Verify that file tracker was called - if len(mockTracker.tracked) != 1 { - t.Errorf("Expected file tracker to track 1 file, got %d", len(mockTracker.tracked)) - } - - if len(mockTracker.tracked) > 0 && mockTracker.tracked[0] != expectedActionFile { - t.Errorf("Expected tracker to track %s, got %s", expectedActionFile, mockTracker.tracked[0]) - } -} - // SimpleBasicMockFileTracker is a basic implementation for testing type SimpleBasicMockFileTracker struct { tracked []string diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index 08ca6e1a0e..1cab9d0002 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -3340,9 +3340,8 @@ Test workflow with ai-reaction. // Check for reaction-specific content in generated YAML expectedStrings := []string{ "add_reaction:", - "mode: add", - "reaction: eyes", - "uses: ./.github/actions/reaction", + "GITHUB_AW_REACTION: eyes", + "uses: actions/github-script@v7", } for _, expected := range expectedStrings { diff --git a/pkg/workflow/js.go b/pkg/workflow/js.go index cbf6ef8582..6bfdf3bb1b 100644 --- a/pkg/workflow/js.go +++ b/pkg/workflow/js.go @@ -30,6 +30,9 @@ var setupAgentOutputScript string //go:embed js/check_team_member.cjs var checkTeamMemberScript string +//go:embed js/add_reaction.cjs +var addReactionScript string + // FormatJavaScriptForYAML formats a JavaScript script with proper indentation for embedding in YAML func FormatJavaScriptForYAML(script string) []string { var formattedLines []string diff --git a/pkg/workflow/js/add_reaction.cjs b/pkg/workflow/js/add_reaction.cjs new file mode 100644 index 0000000000..e456db7b79 --- /dev/null +++ b/pkg/workflow/js/add_reaction.cjs @@ -0,0 +1,99 @@ +async function main() { + // Read inputs from environment variables + const reaction = process.env.GITHUB_AW_REACTION || 'eyes'; + + console.log('Reaction type:', reaction); + + // Validate reaction type + const validReactions = ['+1', '-1', 'laugh', 'confused', 'heart', 'hooray', 'rocket', 'eyes']; + if (!validReactions.includes(reaction)) { + core.setFailed(`Invalid reaction type: ${reaction}. Valid reactions are: ${validReactions.join(', ')}`); + return; + } + + // Determine the API endpoint based on the event type + let endpoint; + const eventName = context.eventName; + const owner = context.repo.owner; + const repo = context.repo.repo; + + try { + switch (eventName) { + case 'issues': + const issueNumber = context.payload?.issue?.number; + if (!issueNumber) { + core.setFailed('Issue number not found in event payload'); + return; + } + endpoint = `/repos/${owner}/${repo}/issues/${issueNumber}/reactions`; + break; + + case 'issue_comment': + const commentId = context.payload?.comment?.id; + if (!commentId) { + core.setFailed('Comment ID not found in event payload'); + return; + } + endpoint = `/repos/${owner}/${repo}/issues/comments/${commentId}/reactions`; + break; + + case 'pull_request': + case 'pull_request_target': + const prNumber = context.payload?.pull_request?.number; + if (!prNumber) { + core.setFailed('Pull request number not found in event payload'); + return; + } + // PRs are "issues" for the reactions endpoint + endpoint = `/repos/${owner}/${repo}/issues/${prNumber}/reactions`; + break; + + case 'pull_request_review_comment': + const reviewCommentId = context.payload?.comment?.id; + if (!reviewCommentId) { + core.setFailed('Review comment ID not found in event payload'); + return; + } + endpoint = `/repos/${owner}/${repo}/pulls/comments/${reviewCommentId}/reactions`; + break; + + default: + core.setFailed(`Unsupported event type: ${eventName}`); + return; + } + + console.log('API endpoint:', endpoint); + + await addReaction(endpoint, reaction); + + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + console.error('Failed to add reaction:', errorMessage); + core.setFailed(`Failed to add reaction: ${errorMessage}`); + } +} + +/** + * Add a reaction to a GitHub issue, PR, or comment + * @param {string} endpoint - The GitHub API endpoint to add the reaction to + * @param {string} reaction - The reaction type to add + */ +async function addReaction(endpoint, reaction) { + const response = await github.request('POST ' + endpoint, { + content: reaction, + headers: { + 'Accept': 'application/vnd.github+json' + } + }); + + const reactionId = response.data?.id; + if (reactionId) { + console.log(`Successfully added reaction: ${reaction} (id: ${reactionId})`); + core.setOutput('reaction-id', reactionId.toString()); + } else { + console.log(`Successfully added reaction: ${reaction}`); + core.setOutput('reaction-id', ''); + } +} + +await main(); \ No newline at end of file diff --git a/pkg/workflow/js/add_reaction.test.cjs b/pkg/workflow/js/add_reaction.test.cjs new file mode 100644 index 0000000000..0f2c334ce2 --- /dev/null +++ b/pkg/workflow/js/add_reaction.test.cjs @@ -0,0 +1,319 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import fs from 'fs'; +import path from 'path'; + +// Mock the global objects that GitHub Actions provides +const mockCore = { + setFailed: vi.fn(), + setOutput: vi.fn(), + summary: { + addRaw: vi.fn().mockReturnThis(), + write: vi.fn() + } +}; + +const mockGithub = { + request: vi.fn() +}; + +const mockContext = { + eventName: 'issues', + repo: { + owner: 'testowner', + repo: 'testrepo' + }, + payload: { + issue: { + number: 123 + } + } +}; + +// Set up global variables +global.core = mockCore; +global.github = mockGithub; +global.context = mockContext; + +describe('add_reaction.cjs', () => { + let addReactionScript; + + beforeEach(() => { + // Reset all mocks + vi.clearAllMocks(); + + // Reset environment variables + delete process.env.GITHUB_AW_REACTION; + + // Reset context to default + global.context = { + eventName: 'issues', + repo: { + owner: 'testowner', + repo: 'testrepo' + }, + payload: { + issue: { + number: 123 + } + } + }; + + // Load the script content + const scriptPath = path.join(process.cwd(), 'pkg/workflow/js/add_reaction.cjs'); + addReactionScript = fs.readFileSync(scriptPath, 'utf8'); + }); + + describe('Environment variable validation', () => { + it('should use default values when environment variables are not set', async () => { + mockGithub.request.mockResolvedValue({ + data: { id: 123, content: 'eyes' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('Reaction type:', 'eyes'); + + consoleSpy.mockRestore(); + }); + + it('should fail with invalid reaction type', async () => { + process.env.GITHUB_AW_REACTION = 'invalid'; + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(mockCore.setFailed).toHaveBeenCalledWith( + 'Invalid reaction type: invalid. Valid reactions are: +1, -1, laugh, confused, heart, hooray, rocket, eyes' + ); + }); + + it('should accept all valid reaction types', async () => { + const validReactions = ['+1', '-1', 'laugh', 'confused', 'heart', 'hooray', 'rocket', 'eyes']; + + for (const reaction of validReactions) { + vi.clearAllMocks(); + process.env.GITHUB_AW_REACTION = reaction; + + mockGithub.request.mockResolvedValue({ + data: { id: 123, content: reaction } + }); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(mockCore.setOutput).toHaveBeenCalledWith('reaction-id', '123'); + } + }); + }); + + describe('Event context handling', () => { + it('should handle issues event', async () => { + global.context.eventName = 'issues'; + global.context.payload = { issue: { number: 123 } }; + + mockGithub.request.mockResolvedValue({ + data: { id: 456, content: 'eyes' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('API endpoint:', '/repos/testowner/testrepo/issues/123/reactions'); + expect(mockGithub.request).toHaveBeenCalledWith('POST /repos/testowner/testrepo/issues/123/reactions', { + content: 'eyes', + headers: { 'Accept': 'application/vnd.github+json' } + }); + + consoleSpy.mockRestore(); + }); + + it('should handle issue_comment event', async () => { + global.context.eventName = 'issue_comment'; + global.context.payload = { comment: { id: 789 } }; + + mockGithub.request.mockResolvedValue({ + data: { id: 456, content: 'eyes' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('API endpoint:', '/repos/testowner/testrepo/issues/comments/789/reactions'); + expect(mockGithub.request).toHaveBeenCalledWith('POST /repos/testowner/testrepo/issues/comments/789/reactions', { + content: 'eyes', + headers: { 'Accept': 'application/vnd.github+json' } + }); + + consoleSpy.mockRestore(); + }); + + it('should handle pull_request event', async () => { + global.context.eventName = 'pull_request'; + global.context.payload = { pull_request: { number: 456 } }; + + mockGithub.request.mockResolvedValue({ + data: { id: 789, content: 'eyes' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('API endpoint:', '/repos/testowner/testrepo/issues/456/reactions'); + expect(mockGithub.request).toHaveBeenCalledWith('POST /repos/testowner/testrepo/issues/456/reactions', { + content: 'eyes', + headers: { 'Accept': 'application/vnd.github+json' } + }); + + consoleSpy.mockRestore(); + }); + + it('should handle pull_request_review_comment event', async () => { + global.context.eventName = 'pull_request_review_comment'; + global.context.payload = { comment: { id: 321 } }; + + mockGithub.request.mockResolvedValue({ + data: { id: 654, content: 'eyes' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('API endpoint:', '/repos/testowner/testrepo/pulls/comments/321/reactions'); + expect(mockGithub.request).toHaveBeenCalledWith('POST /repos/testowner/testrepo/pulls/comments/321/reactions', { + content: 'eyes', + headers: { 'Accept': 'application/vnd.github+json' } + }); + + consoleSpy.mockRestore(); + }); + + it('should fail on unsupported event type', async () => { + global.context.eventName = 'unsupported'; + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(mockCore.setFailed).toHaveBeenCalledWith('Unsupported event type: unsupported'); + }); + + it('should fail when issue number is missing', async () => { + global.context.eventName = 'issues'; + global.context.payload = {}; + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(mockCore.setFailed).toHaveBeenCalledWith('Issue number not found in event payload'); + }); + + it('should fail when comment ID is missing', async () => { + global.context.eventName = 'issue_comment'; + global.context.payload = {}; + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(mockCore.setFailed).toHaveBeenCalledWith('Comment ID not found in event payload'); + }); + }); + + describe('Add reaction functionality', () => { + it('should successfully add reaction with direct response', async () => { + process.env.GITHUB_AW_REACTION = 'heart'; + + mockGithub.request.mockResolvedValue({ + data: { id: 123, content: 'heart' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('Successfully added reaction: heart (id: 123)'); + expect(mockCore.setOutput).toHaveBeenCalledWith('reaction-id', '123'); + + consoleSpy.mockRestore(); + }); + + it('should handle response without ID', async () => { + process.env.GITHUB_AW_REACTION = 'rocket'; + + mockGithub.request.mockResolvedValue({ + data: { content: 'rocket' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('Successfully added reaction: rocket'); + expect(mockCore.setOutput).toHaveBeenCalledWith('reaction-id', ''); + + consoleSpy.mockRestore(); + }); + }); + + describe('Error handling', () => { + it('should handle API errors gracefully', async () => { + // Mock the GitHub request to fail + mockGithub.request.mockRejectedValue(new Error('API Error')); + + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('Failed to add reaction:', 'API Error'); + expect(mockCore.setFailed).toHaveBeenCalledWith('Failed to add reaction: API Error'); + + consoleSpy.mockRestore(); + }); + + it('should handle non-Error objects in catch block', async () => { + // Mock the GitHub request to fail with string error + mockGithub.request.mockRejectedValue('String error'); + + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('Failed to add reaction:', 'String error'); + expect(mockCore.setFailed).toHaveBeenCalledWith('Failed to add reaction: String error'); + + consoleSpy.mockRestore(); + }); + }); + + describe('Output and logging', () => { + it('should log reaction type', async () => { + process.env.GITHUB_AW_REACTION = 'rocket'; + + mockGithub.request.mockResolvedValue({ + data: { id: 123, content: 'rocket' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('Reaction type:', 'rocket'); + + consoleSpy.mockRestore(); + }); + + it('should log API endpoint', async () => { + mockGithub.request.mockResolvedValue({ + data: { id: 123, content: 'eyes' } + }); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await eval(`(async () => { ${addReactionScript} })()`); + + expect(consoleSpy).toHaveBeenCalledWith('API endpoint:', '/repos/testowner/testrepo/issues/123/reactions'); + + consoleSpy.mockRestore(); + }); + }); +}); \ No newline at end of file diff --git a/tsconfig.json b/tsconfig.json index d51267efcd..515f0eb610 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -33,6 +33,7 @@ }, "include": [ "pkg/workflow/js/add_labels.cjs", + "pkg/workflow/js/add_reaction.cjs", "pkg/workflow/js/check_team_member.cjs", "pkg/workflow/js/compute_text.cjs", "pkg/workflow/js/create_comment.cjs", From 16ac4cf18a663e6a652570a477feabcca5bfb1c4 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 Aug 2025 13:03:12 -0700 Subject: [PATCH 4/8] Remove compute-text action dependency and template files from workflow system (#29) --- .github/workflows/test-claude.lock.yml | 105 +++++++++++++++++++++++++ .github/workflows/test-claude.md | 3 + pkg/cli/file_tracker_test.go | 44 ++--------- pkg/workflow/compiler.go | 3 - 4 files changed, 114 insertions(+), 41 deletions(-) diff --git a/.github/workflows/test-claude.lock.yml b/.github/workflows/test-claude.lock.yml index f9b967bfc3..462e6eb151 100644 --- a/.github/workflows/test-claude.lock.yml +++ b/.github/workflows/test-claude.lock.yml @@ -21,6 +21,109 @@ concurrency: run-name: "Test Claude" jobs: + add_reaction: + if: github.event_name == 'issues' || github.event_name == 'pull_request' || github.event_name == 'issue_comment' || github.event_name == 'pull_request_comment' || github.event_name == 'pull_request_review_comment' + runs-on: ubuntu-latest + permissions: + contents: write # Read .github + issues: write + pull-requests: write + outputs: + reaction_id: ${{ steps.react.outputs.reaction-id }} + steps: + - name: Add eyes reaction to the triggering item + id: react + uses: actions/github-script@v7 + env: + GITHUB_AW_REACTION: eyes + with: + script: | + async function main() { + // Read inputs from environment variables + const reaction = process.env.GITHUB_AW_REACTION || 'eyes'; + console.log('Reaction type:', reaction); + // Validate reaction type + const validReactions = ['+1', '-1', 'laugh', 'confused', 'heart', 'hooray', 'rocket', 'eyes']; + if (!validReactions.includes(reaction)) { + core.setFailed(`Invalid reaction type: ${reaction}. Valid reactions are: ${validReactions.join(', ')}`); + return; + } + // Determine the API endpoint based on the event type + let endpoint; + const eventName = context.eventName; + const owner = context.repo.owner; + const repo = context.repo.repo; + try { + switch (eventName) { + case 'issues': + const issueNumber = context.payload?.issue?.number; + if (!issueNumber) { + core.setFailed('Issue number not found in event payload'); + return; + } + endpoint = `/repos/${owner}/${repo}/issues/${issueNumber}/reactions`; + break; + case 'issue_comment': + const commentId = context.payload?.comment?.id; + if (!commentId) { + core.setFailed('Comment ID not found in event payload'); + return; + } + endpoint = `/repos/${owner}/${repo}/issues/comments/${commentId}/reactions`; + break; + case 'pull_request': + case 'pull_request_target': + const prNumber = context.payload?.pull_request?.number; + if (!prNumber) { + core.setFailed('Pull request number not found in event payload'); + return; + } + // PRs are "issues" for the reactions endpoint + endpoint = `/repos/${owner}/${repo}/issues/${prNumber}/reactions`; + break; + case 'pull_request_review_comment': + const reviewCommentId = context.payload?.comment?.id; + if (!reviewCommentId) { + core.setFailed('Review comment ID not found in event payload'); + return; + } + endpoint = `/repos/${owner}/${repo}/pulls/comments/${reviewCommentId}/reactions`; + break; + default: + core.setFailed(`Unsupported event type: ${eventName}`); + return; + } + console.log('API endpoint:', endpoint); + await addReaction(endpoint, reaction); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + console.error('Failed to add reaction:', errorMessage); + core.setFailed(`Failed to add reaction: ${errorMessage}`); + } + } + /** + * Add a reaction to a GitHub issue, PR, or comment + * @param {string} endpoint - The GitHub API endpoint to add the reaction to + * @param {string} reaction - The reaction type to add + */ + async function addReaction(endpoint, reaction) { + const response = await github.request('POST ' + endpoint, { + content: reaction, + headers: { + 'Accept': 'application/vnd.github+json' + } + }); + const reactionId = response.data?.id; + if (reactionId) { + console.log(`Successfully added reaction: ${reaction} (id: ${reactionId})`); + core.setOutput('reaction-id', reactionId.toString()); + } else { + console.log(`Successfully added reaction: ${reaction}`); + core.setOutput('reaction-id', ''); + } + } + await main(); + test-claude: runs-on: ubuntu-latest permissions: @@ -108,6 +211,8 @@ jobs: **First, get the current time using the get_current_time tool to timestamp your analysis.** + **Important**: When analyzing the pull request content, gather context directly from the GitHub API to understand what triggered this workflow. + ### Analysis Tasks 1. **Review the Pull Request Details** diff --git a/.github/workflows/test-claude.md b/.github/workflows/test-claude.md index 1dcc7e6d66..5da1d4fdb2 100644 --- a/.github/workflows/test-claude.md +++ b/.github/workflows/test-claude.md @@ -11,6 +11,7 @@ engine: id: claude model: claude-3-5-sonnet-20241022 timeout_minutes: 10 +ai-reaction: eyes permissions: pull-requests: write actions: read @@ -47,6 +48,8 @@ You are a code review assistant powered by Claude. Your task is to analyze the c **First, get the current time using the get_current_time tool to timestamp your analysis.** +**Important**: When analyzing the pull request content, gather context directly from the GitHub API to understand what triggered this workflow. + ### Analysis Tasks 1. **Review the Pull Request Details** diff --git a/pkg/cli/file_tracker_test.go b/pkg/cli/file_tracker_test.go index 333c35c125..bf7e509100 100644 --- a/pkg/cli/file_tracker_test.go +++ b/pkg/cli/file_tracker_test.go @@ -323,23 +323,8 @@ This uses ai-reaction. t.Errorf("Lock file %s should be tracked", lockFile) } - // Should track the shared reaction action - reactionActionFile := filepath.Join(tempDir, ".github", "actions", "reaction", "action.yml") - found = false - for _, file := range allFiles { - if file == reactionActionFile { - found = true - break - } - } - if !found { - t.Errorf("Reaction action file %s should be tracked", reactionActionFile) - } - - // Verify the reaction action file was actually created - if _, err := os.Stat(reactionActionFile); os.IsNotExist(err) { - t.Errorf("Reaction action file %s should be created", reactionActionFile) - } + // Note: The reaction feature now uses inline GitHub Scripts instead of separate action files + // so we don't expect a separate reaction action file to be created // Test 2: Workflow WITHOUT ai-reaction should NOT create shared action workflowWithoutReaction := `--- @@ -366,30 +351,13 @@ This does NOT use ai-reaction. } // Remove the existing reaction action to test it's not created again - if err := os.RemoveAll(filepath.Join(tempDir, ".github", "actions", "reaction")); err != nil { - t.Fatalf("Failed to remove existing reaction action: %v", err) - } - + // (Note: Since reaction is now inline, this removal step is no longer needed) + // Compile the workflow with tracking if err := compileWorkflowWithTracking(workflowFileWithoutReaction, false, "", tracker2); err != nil { t.Fatalf("Failed to compile workflow: %v", err) } - // Check that reaction action was NOT created - if _, err := os.Stat(reactionActionFile); !os.IsNotExist(err) { - t.Errorf("Reaction action file %s should NOT be created when ai-reaction is not specified", reactionActionFile) - } - - // Check that reaction action is NOT tracked - allFiles2 := append(tracker2.CreatedFiles, tracker2.ModifiedFiles...) - found = false - for _, file := range allFiles2 { - if file == reactionActionFile { - found = true - break - } - } - if found { - t.Errorf("Reaction action file %s should NOT be tracked when ai-reaction is not specified", reactionActionFile) - } + // Note: Since reaction feature now uses inline GitHub Scripts instead of separate action files, + // we don't expect any reaction action files to be created or tracked } diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 16b736cf8a..62b49bd731 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -29,9 +29,6 @@ type FileTracker interface { TrackCreated(filePath string) } -//go:embed templates/compute_text_action.yaml -var computeTextActionTemplate string - //go:embed templates/reaction_action.yaml var reactionActionTemplate string From ffa3fac282197db7fe64c8a8f8c8f22d9e1c65ff Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 Aug 2025 13:46:21 -0700 Subject: [PATCH 5/8] Fix linter issues: Remove unused reactionActionTemplate variable (#30) --- pkg/cli/file_tracker_test.go | 2 +- pkg/workflow/compiler.go | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/cli/file_tracker_test.go b/pkg/cli/file_tracker_test.go index bf7e509100..ed32ae4e00 100644 --- a/pkg/cli/file_tracker_test.go +++ b/pkg/cli/file_tracker_test.go @@ -352,7 +352,7 @@ This does NOT use ai-reaction. // Remove the existing reaction action to test it's not created again // (Note: Since reaction is now inline, this removal step is no longer needed) - + // Compile the workflow with tracking if err := compileWorkflowWithTracking(workflowFileWithoutReaction, false, "", tracker2); err != nil { t.Fatalf("Failed to compile workflow: %v", err) diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 62b49bd731..95ad22ddc7 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -1,7 +1,6 @@ package workflow import ( - _ "embed" "encoding/json" "errors" "fmt" @@ -29,9 +28,6 @@ type FileTracker interface { TrackCreated(filePath string) } -//go:embed templates/reaction_action.yaml -var reactionActionTemplate string - // Compiler handles converting markdown workflows to GitHub Actions YAML type Compiler struct { verbose bool From 644b2238dcced58bc8674f1d0039ac9095542dc9 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 Aug 2025 15:16:45 -0700 Subject: [PATCH 6/8] Add output text to test-claude and inline JavaScript instead of shared actions (#32) --- .github/workflows/test-claude.lock.yml | 203 +++++++++++++++++++++++++ .github/workflows/test-claude.md | 2 + pkg/workflow/compiler.go | 112 ++------------ pkg/workflow/compute_text_lazy_test.go | 15 +- 4 files changed, 226 insertions(+), 106 deletions(-) diff --git a/.github/workflows/test-claude.lock.yml b/.github/workflows/test-claude.lock.yml index 462e6eb151..3f7c29066b 100644 --- a/.github/workflows/test-claude.lock.yml +++ b/.github/workflows/test-claude.lock.yml @@ -21,7 +21,207 @@ concurrency: run-name: "Test Claude" jobs: + task: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + text: ${{ steps.compute-text.outputs.text }} + steps: + - uses: actions/checkout@v5 + with: + sparse-checkout: .github + fetch-depth: 1 + - name: Compute current body text + id: compute-text + uses: actions/github-script@v7 + with: + script: | + /** + * Sanitizes content for safe output in GitHub Actions + * @param {string} content - The content to sanitize + * @returns {string} The sanitized content + */ + function sanitizeContent(content) { + if (!content || typeof content !== 'string') { + return ''; + } + // Read allowed domains from environment variable + const allowedDomainsEnv = process.env.GITHUB_AW_ALLOWED_DOMAINS; + const defaultAllowedDomains = [ + 'github.com', + 'github.io', + 'githubusercontent.com', + 'githubassets.com', + 'github.dev', + 'codespaces.new' + ]; + const allowedDomains = allowedDomainsEnv + ? allowedDomainsEnv.split(',').map(d => d.trim()).filter(d => d) + : defaultAllowedDomains; + let sanitized = content; + // Neutralize @mentions to prevent unintended notifications + sanitized = neutralizeMentions(sanitized); + // Remove control characters (except newlines and tabs) + sanitized = sanitized.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, ''); + // XML character escaping + sanitized = sanitized + .replace(/&/g, '&') // Must be first to avoid double-escaping + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + // URI filtering - replace non-https protocols with "(redacted)" + // Step 1: Temporarily mark HTTPS URLs to protect them + sanitized = sanitizeUrlProtocols(sanitized); + // Domain filtering for HTTPS URIs + // Match https:// URIs and check if domain is in allowlist + sanitized = sanitizeUrlDomains(sanitized); + // Limit total length to prevent DoS (0.5MB max) + const maxLength = 524288; + if (sanitized.length > maxLength) { + sanitized = sanitized.substring(0, maxLength) + '\n[Content truncated due to length]'; + } + // Limit number of lines to prevent log flooding (65k max) + const lines = sanitized.split('\n'); + const maxLines = 65000; + if (lines.length > maxLines) { + sanitized = lines.slice(0, maxLines).join('\n') + '\n[Content truncated due to line count]'; + } + // Remove ANSI escape sequences + sanitized = sanitized.replace(/\x1b\[[0-9;]*[mGKH]/g, ''); + // Neutralize common bot trigger phrases + sanitized = neutralizeBotTriggers(sanitized); + // Trim excessive whitespace + return sanitized.trim(); + /** + * Remove unknown domains + * @param {string} s - The string to process + * @returns {string} The string with unknown domains redacted + */ + function sanitizeUrlDomains(s) { + s = s.replace(/\bhttps:\/\/([^\/\s\])}'"<>&\x00-\x1f]+)/gi, (match, domain) => { + // Extract the hostname part (before first slash, colon, or other delimiter) + const hostname = domain.split(/[\/:\?#]/)[0].toLowerCase(); + // Check if this domain or any parent domain is in the allowlist + const isAllowed = allowedDomains.some(allowedDomain => { + const normalizedAllowed = allowedDomain.toLowerCase(); + return hostname === normalizedAllowed || hostname.endsWith('.' + normalizedAllowed); + }); + return isAllowed ? match : '(redacted)'; + }); + return s; + } + /** + * Remove unknown protocols except https + * @param {string} s - The string to process + * @returns {string} The string with non-https protocols redacted + */ + function sanitizeUrlProtocols(s) { + // Match both protocol:// and protocol: patterns + // This covers URLs like https://example.com, javascript:alert(), mailto:user@domain.com, etc. + return s.replace(/\b(\w+):(?:\/\/)?[^\s\])}'"<>&\x00-\x1f]+/gi, (match, protocol) => { + // Allow https (case insensitive), redact everything else + return protocol.toLowerCase() === 'https' ? match : '(redacted)'; + }); + } + /** + * Neutralizes @mentions by wrapping them in backticks + * @param {string} s - The string to process + * @returns {string} The string with neutralized mentions + */ + function neutralizeMentions(s) { + // Replace @name or @org/team outside code with `@name` + return s.replace(/(^|[^\w`])@([A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, + (_m, p1, p2) => `${p1}\`@${p2}\``); + } + /** + * Neutralizes bot trigger phrases by wrapping them in backticks + * @param {string} s - The string to process + * @returns {string} The string with neutralized bot triggers + */ + function neutralizeBotTriggers(s) { + // Neutralize common bot trigger phrases like "fixes #123", "closes #asdfs", etc. + return s.replace(/\b(fixes?|closes?|resolves?|fix|close|resolve)\s+#(\w+)/gi, + (match, action, ref) => `\`${action} #${ref}\``); + } + } + async function main() { + let text = ''; + const actor = context.actor; + const { owner, repo } = context.repo; + // Check if the actor has repository access (admin, maintain permissions) + const repoPermission = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: owner, + repo: repo, + username: actor + }); + const permission = repoPermission.data.permission; + console.log(`Repository permission level: ${permission}`); + if (permission !== 'admin' && permission !== 'maintain') { + core.setOutput('text', ''); + return; + } + // Determine current body text based on event context + switch (context.eventName) { + case 'issues': + // For issues: title + body + if (context.payload.issue) { + const title = context.payload.issue.title || ''; + const body = context.payload.issue.body || ''; + text = `${title}\n\n${body}`; + } + break; + case 'pull_request': + // For pull requests: title + body + if (context.payload.pull_request) { + const title = context.payload.pull_request.title || ''; + const body = context.payload.pull_request.body || ''; + text = `${title}\n\n${body}`; + } + break; + case 'pull_request_target': + // For pull request target events: title + body + if (context.payload.pull_request) { + const title = context.payload.pull_request.title || ''; + const body = context.payload.pull_request.body || ''; + text = `${title}\n\n${body}`; + } + break; + case 'issue_comment': + // For issue comments: comment body + if (context.payload.comment) { + text = context.payload.comment.body || ''; + } + break; + case 'pull_request_review_comment': + // For PR review comments: comment body + if (context.payload.comment) { + text = context.payload.comment.body || ''; + } + break; + case 'pull_request_review': + // For PR reviews: review body + if (context.payload.review) { + text = context.payload.review.body || ''; + } + break; + default: + // Default: empty text + text = ''; + break; + } + // Sanitize the text before output + const sanitizedText = sanitizeContent(text); + // Display sanitized text in logs + console.log(`text: ${sanitizedText}`); + // Set the sanitized text as output + core.setOutput('text', sanitizedText); + } + await main(); + add_reaction: + needs: task if: github.event_name == 'issues' || github.event_name == 'pull_request' || github.event_name == 'issue_comment' || github.event_name == 'pull_request_comment' || github.event_name == 'pull_request_review_comment' runs-on: ubuntu-latest permissions: @@ -125,6 +325,7 @@ jobs: await main(); test-claude: + needs: task runs-on: ubuntu-latest permissions: actions: read @@ -284,6 +485,8 @@ jobs: - Proper markdown formatting for readability - Clear structure with headers and bullet points + **Current Context**: You have access to the current pull request content via: "${{ needs.task.outputs.text }}" + ### Action Output: Create a Haiku **IMPORTANT**: After completing your PR analysis and posting your comment, please create a haiku about the changes you analyzed and write it to the action output. The haiku should capture the essence of the pull request in a creative and poetic way. diff --git a/.github/workflows/test-claude.md b/.github/workflows/test-claude.md index 5da1d4fdb2..95c795c4b4 100644 --- a/.github/workflows/test-claude.md +++ b/.github/workflows/test-claude.md @@ -121,6 +121,8 @@ Your comment should include: - Proper markdown formatting for readability - Clear structure with headers and bullet points +**Current Context**: You have access to the current pull request content via: "${{ needs.task.outputs.text }}" + ### Action Output: Create a Haiku **IMPORTANT**: After completing your PR analysis and posting your comment, please create a haiku about the changes you analyzed and write it to the action output. The haiku should capture the essence of the pull request in a creative and poetic way. diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 95ad22ddc7..13c5a0d40f 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -239,21 +239,8 @@ func (c *Compiler) CompileWorkflow(markdownPath string) error { } } - // Write shared compute-text action (only if needed for task job) - if workflowData.NeedsTextOutput { - if err := c.writeComputeTextAction(markdownPath); err != nil { - formattedErr := console.FormatError(console.CompilerError{ - Position: console.ErrorPosition{ - File: markdownPath, - Line: 1, - Column: 1, - }, - Type: "error", - Message: fmt.Sprintf("failed to write compute-text action: %v", err), - }) - return errors.New(formattedErr) - } - } + // Note: compute-text functionality is now inlined directly in the task job + // instead of using a shared action file // Generate the YAML content if c.verbose { @@ -1572,11 +1559,17 @@ func (c *Compiler) buildTaskJob(data *WorkflowData) (*Job, error) { steps = append(steps, " exit 1\n") } - // Use shared compute-text action only if needed + // Use inlined compute-text script only if needed (no shared action) if data.NeedsTextOutput { steps = append(steps, " - name: Compute current body text\n") steps = append(steps, " id: compute-text\n") - steps = append(steps, " uses: ./.github/actions/compute-text\n") + steps = append(steps, " uses: actions/github-script@v7\n") + steps = append(steps, " with:\n") + steps = append(steps, " script: |\n") + + // Inline the JavaScript directly instead of using shared action + steps = append(steps, FormatJavaScriptForYAML(computeTextScript)...) + // Set up outputs outputs["text"] = "${{ steps.compute-text.outputs.text }}" } @@ -1995,91 +1988,6 @@ func getGitHubDockerImageVersion(githubTool any) string { return githubDockerImageVersion } -// writeSharedAction writes a shared action file, creating directories as needed and updating only if content differs -func (c *Compiler) writeSharedAction(markdownPath string, actionPath string, content string, actionName string) error { - // Get git root to write action relative to it, fallback to markdown directory for tests - gitRoot := filepath.Dir(markdownPath) - for { - if _, err := os.Stat(filepath.Join(gitRoot, ".git")); err == nil { - break - } - parent := filepath.Dir(gitRoot) - if parent == gitRoot { - // Reached filesystem root without finding .git - use markdown directory as fallback - gitRoot = filepath.Dir(markdownPath) - break - } - gitRoot = parent - } - - actionsDir := filepath.Join(gitRoot, ".github", "actions", actionPath) - actionFile := filepath.Join(actionsDir, "action.yml") - - // Create directory if it doesn't exist - if err := os.MkdirAll(actionsDir, 0755); err != nil { - return fmt.Errorf("failed to create actions directory: %w", err) - } - - // Write the action file if it doesn't exist or is different - if _, err := os.Stat(actionFile); os.IsNotExist(err) { - if c.verbose { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Creating shared %s action: %s", actionName, actionFile))) - } - if err := os.WriteFile(actionFile, []byte(content), 0644); err != nil { - return fmt.Errorf("failed to write %s action: %w", actionName, err) - } - // Track the created file - if c.fileTracker != nil { - c.fileTracker.TrackCreated(actionFile) - } - } else { - // Check if the content is different and update if needed - existing, err := os.ReadFile(actionFile) - if err != nil { - return fmt.Errorf("failed to read existing action file: %w", err) - } - if string(existing) != content { - if c.verbose { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Updating shared %s action: %s", actionName, actionFile))) - } - if err := os.WriteFile(actionFile, []byte(content), 0644); err != nil { - return fmt.Errorf("failed to update %s action: %w", actionName, err) - } - // Track the updated file - if c.fileTracker != nil { - c.fileTracker.TrackCreated(actionFile) - } - } - } - - return nil -} - -// writeComputeTextAction writes the shared compute-text action -func (c *Compiler) writeComputeTextAction(markdownPath string) error { - // Generate the action content with embedded JavaScript - var actionContent strings.Builder - - actionContent.WriteString("name: \"Compute current body text\"\n") - actionContent.WriteString("description: \"Computes the current body text based on the GitHub event context\"\n") - actionContent.WriteString("outputs:\n") - actionContent.WriteString(" text:\n") - actionContent.WriteString(" description: \"The computed current body text based on event type\"\n") - actionContent.WriteString("runs:\n") - actionContent.WriteString(" using: \"composite\"\n") - actionContent.WriteString(" steps:\n") - actionContent.WriteString(" - name: Compute current body text\n") - actionContent.WriteString(" id: compute-text\n") - actionContent.WriteString(" uses: actions/github-script@v7\n") - actionContent.WriteString(" with:\n") - actionContent.WriteString(" script: |\n") - - // Embed the JavaScript with proper indentation - WriteJavaScriptToYAML(&actionContent, computeTextScript) - - return c.writeSharedAction(markdownPath, "compute-text", actionContent.String(), "compute-text") -} - // generateMainJobSteps generates the steps section for the main job func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowData) { // Add custom steps or default checkout step diff --git a/pkg/workflow/compute_text_lazy_test.go b/pkg/workflow/compute_text_lazy_test.go index 2fdb9f5f06..1c004ad557 100644 --- a/pkg/workflow/compute_text_lazy_test.go +++ b/pkg/workflow/compute_text_lazy_test.go @@ -76,13 +76,13 @@ Create a report based on repository analysis.` t.Fatalf("Failed to compile workflow with text: %v", err) } - // Check that compute-text action was created + // Check that compute-text action was NOT created (JavaScript is now inlined) actionPath := filepath.Join(tempDir, ".github", "actions", "compute-text", "action.yml") - if _, err := os.Stat(actionPath); os.IsNotExist(err) { - t.Error("Expected compute-text action to be created for workflow that uses text output") + if _, err := os.Stat(actionPath); !os.IsNotExist(err) { + t.Error("Expected compute-text action NOT to be created (JavaScript should be inlined)") } - // Check that the compiled YAML contains compute-text step + // Check that the compiled YAML contains inlined compute-text step lockPath := strings.TrimSuffix(workflowWithTextPath, ".md") + ".lock.yml" lockContent, err := os.ReadFile(lockPath) if err != nil { @@ -96,6 +96,13 @@ Create a report based on repository analysis.` if !strings.Contains(lockStr, "text: ${{ steps.compute-text.outputs.text }}") { t.Error("Expected compiled workflow to contain text output") } + // Check that JavaScript is inlined instead of using shared action + if !strings.Contains(lockStr, "uses: actions/github-script@v7") { + t.Error("Expected compute-text step to use inlined JavaScript") + } + if strings.Contains(lockStr, "uses: ./.github/actions/compute-text") { + t.Error("Expected compute-text step NOT to use shared action") + } }) // Remove compute-text action for next test From a06c9d0c000cbcb32f36d788755bc0bfb38f0fbe Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 Aug 2025 19:18:54 -0700 Subject: [PATCH 7/8] Remove obsolete reaction_action.yaml template file (#34) --- docs/frontmatter.md | 2 +- pkg/workflow/compiler_test.go | 4 +- pkg/workflow/templates/reaction_action.yaml | 130 -------------------- 3 files changed, 3 insertions(+), 133 deletions(-) delete mode 100644 pkg/workflow/templates/reaction_action.yaml diff --git a/docs/frontmatter.md b/docs/frontmatter.md index 381d387fb0..90d15cd5dd 100644 --- a/docs/frontmatter.md +++ b/docs/frontmatter.md @@ -173,7 +173,7 @@ ai-reaction: "eyes" - `rocket` (🚀) - `eyes` (👀) -**Note**: Using this feature results in the addition of `.github/actions/reaction/action.yml` file to the repository when the workflow is compiled. +**Note**: This feature uses inline JavaScript code with `actions/github-script@v7` to add reactions, so no additional action files are created in the repository. ## Output Configuration (`output:`) diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index 1cab9d0002..b099a97238 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -3412,8 +3412,8 @@ Test workflow without explicit ai-reaction (should not create reaction action). // Check that reaction-specific content is NOT in generated YAML unexpectedStrings := []string{ "add_reaction:", - "uses: ./.github/actions/reaction", - "mode: add", + "GITHUB_AW_REACTION:", + "Add eyes reaction to the triggering item", } for _, unexpected := range unexpectedStrings { diff --git a/pkg/workflow/templates/reaction_action.yaml b/pkg/workflow/templates/reaction_action.yaml deleted file mode 100644 index 9858b7290c..0000000000 --- a/pkg/workflow/templates/reaction_action.yaml +++ /dev/null @@ -1,130 +0,0 @@ -name: "Add/Remove reaction on triggering item" -description: "Adds or removes a reaction on the issue/PR/comment that triggered the workflow" -inputs: - github-token: - description: "Token with issues/pull-requests write (GITHUB_TOKEN is fine)" - required: true - mode: - description: "'add' or 'remove'" - required: true - reaction: - description: "One of +1, -1, laugh, confused, heart, hooray, rocket, eyes" - required: false - default: "eyes" - reaction-id: - description: "Optional reaction id to remove (if known)" - required: false -outputs: - reaction-id: - description: "ID of the reaction that was added (for later removal)" -runs: - using: "composite" - steps: - - name: Compute reactions API endpoint for the triggering payload - id: ctx - shell: bash - env: - GITHUB_EVENT_NAME: ${{ github.event_name }} - GITHUB_EVENT_PATH: ${{ github.event_path }} - GITHUB_REPOSITORY: ${{ github.repository }} - run: | - set -euo pipefail - owner="${GITHUB_REPOSITORY%%/*}" - repo="${GITHUB_REPOSITORY##*/}" - ev="$GITHUB_EVENT_PATH" - - case "$GITHUB_EVENT_NAME" in - issues) - number=$(jq -r '.issue.number' "$ev") - endpoint="/repos/$owner/$repo/issues/$number/reactions" - ;; - issue_comment) - cid=$(jq -r '.comment.id' "$ev") - endpoint="/repos/$owner/$repo/issues/comments/$cid/reactions" - ;; - pull_request|pull_request_target) - number=$(jq -r '.pull_request.number' "$ev") - # PRs are "issues" for the reactions endpoint - endpoint="/repos/$owner/$repo/issues/$number/reactions" - ;; - pull_request_review_comment) - cid=$(jq -r '.comment.id' "$ev") - endpoint="/repos/$owner/$repo/pulls/comments/$cid/reactions" - ;; - *) - echo "Unsupported event: $GITHUB_EVENT_NAME" >&2 - exit 1 - ;; - esac - - echo "endpoint=$endpoint" >> "$GITHUB_OUTPUT" - - - name: Add reaction - if: ${{ inputs.mode == 'add' }} - shell: bash - env: - GH_TOKEN: ${{ inputs.github-token }} - ENDPOINT: ${{ steps.ctx.outputs.endpoint }} - REACTION: ${{ inputs.reaction }} - run: | - set -euo pipefail - # Create (or fetch existing) reaction - # The API returns the reaction object (201 on create, 200 if it already existed) - resp=$(gh api \ - -H "Accept: application/vnd.github+json" \ - -X POST "$ENDPOINT" \ - -f content="$REACTION" \ - || true) - - # If a concurrent create happened, fall back to listing to find our reaction - if [ -z "${resp:-}" ] || [ "$resp" = "null" ]; then - resp=$(gh api -H "Accept: application/vnd.github+json" "$ENDPOINT") - rid=$(echo "$resp" | jq -r --arg r "$REACTION" \ - '.[] | select(.content==$r and .user.login=="github-actions[bot]") | .id' | head -n1) - else - rid=$(echo "$resp" | jq -r '.id') - if [ "$rid" = "null" ] || [ -z "$rid" ]; then - # fallback to list, just in case - list=$(gh api -H "Accept: application/vnd.github+json" "$ENDPOINT") - rid=$(echo "$list" | jq -r --arg r "$REACTION" \ - '.[] | select(.content==$r and .user.login=="github-actions[bot]") | .id' | head -n1) - fi - fi - - if [ -z "${rid:-}" ]; then - echo "Warning: could not determine reaction id; cleanup will list/filter." >&2 - fi - - echo "reaction-id=${rid:-}" >> "$GITHUB_OUTPUT" - - - name: Remove reaction - if: ${{ inputs.mode == 'remove' }} - shell: bash - env: - GH_TOKEN: ${{ inputs.github-token }} - ENDPOINT: ${{ steps.ctx.outputs.endpoint }} - REACTION: ${{ inputs.reaction }} - REACTION_ID_IN: ${{ inputs.reaction-id }} - run: | - set -euo pipefail - - delete_by_id () { - local rid="$1" - if [ -n "$rid" ] && [ "$rid" != "null" ]; then - gh api -H "Accept: application/vnd.github+json" -X DELETE "/reactions/$rid" || true - fi - } - - if [ -n "$REACTION_ID_IN" ]; then - # Fast path: we were given the id from the add step - delete_by_id "$REACTION_ID_IN" - exit 0 - fi - - # Fallback: list reactions on the same subject, and delete the bot's matching reaction(s) - list=$(gh api -H "Accept: application/vnd.github+json" "$ENDPOINT" || echo "[]") - echo "$list" | jq -r --arg r "$REACTION" ' - .[] | select(.content==$r and .user.login=="github-actions[bot]") | .id - ' | while read -r rid; do - delete_by_id "$rid" - done \ No newline at end of file From 65a9646269704e6d02ba57548cacc408390140e5 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 Aug 2025 22:34:59 -0700 Subject: [PATCH 8/8] Remove unnecessary content permissions and checkout steps from task and add_reaction jobs (#35) --- .github/workflows/test-claude.lock.yml | 7 - pkg/workflow/compiler.go | 10 +- .../task_and_reaction_permissions_test.go | 130 ++++++++++++++++++ 3 files changed, 132 insertions(+), 15 deletions(-) create mode 100644 pkg/workflow/task_and_reaction_permissions_test.go diff --git a/.github/workflows/test-claude.lock.yml b/.github/workflows/test-claude.lock.yml index 3f7c29066b..8fd97c1395 100644 --- a/.github/workflows/test-claude.lock.yml +++ b/.github/workflows/test-claude.lock.yml @@ -23,15 +23,9 @@ run-name: "Test Claude" jobs: task: runs-on: ubuntu-latest - permissions: - contents: read outputs: text: ${{ steps.compute-text.outputs.text }} steps: - - uses: actions/checkout@v5 - with: - sparse-checkout: .github - fetch-depth: 1 - name: Compute current body text id: compute-text uses: actions/github-script@v7 @@ -225,7 +219,6 @@ jobs: if: github.event_name == 'issues' || github.event_name == 'pull_request' || github.event_name == 'issue_comment' || github.event_name == 'pull_request_comment' || github.event_name == 'pull_request_review_comment' runs-on: ubuntu-latest permissions: - contents: write # Read .github issues: write pull-requests: write outputs: diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 13c5a0d40f..dcaf05eb13 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -1516,12 +1516,6 @@ func (c *Compiler) buildTaskJob(data *WorkflowData) (*Job, error) { outputs := map[string]string{} var steps []string - // Add shallow checkout step to access shared actions - steps = append(steps, " - uses: actions/checkout@v5\n") - steps = append(steps, " with:\n") - steps = append(steps, " sparse-checkout: .github\n") - steps = append(steps, " fetch-depth: 1\n") - // Add team member check for alias workflows, but only when triggered by alias mention if data.Alias != "" { // Build condition that only applies to alias mentions in comment-related events @@ -1578,7 +1572,7 @@ func (c *Compiler) buildTaskJob(data *WorkflowData) (*Job, error) { Name: "task", If: data.If, // Use the existing condition (which may include alias checks) RunsOn: "runs-on: ubuntu-latest", - Permissions: "permissions:\n contents: read", // Read permission for checkout + Permissions: "", // No permissions needed - task job does not require content access Steps: steps, Outputs: outputs, } @@ -1619,7 +1613,7 @@ func (c *Compiler) buildAddReactionJob(data *WorkflowData, taskJobCreated bool) Name: "add_reaction", If: fmt.Sprintf("if: %s", reactionCondition.Render()), RunsOn: "runs-on: ubuntu-latest", - Permissions: "permissions:\n contents: write # Read .github\n issues: write\n pull-requests: write", + Permissions: "permissions:\n issues: write\n pull-requests: write", Steps: steps, Outputs: outputs, Depends: depends, diff --git a/pkg/workflow/task_and_reaction_permissions_test.go b/pkg/workflow/task_and_reaction_permissions_test.go new file mode 100644 index 0000000000..bd687e27e1 --- /dev/null +++ b/pkg/workflow/task_and_reaction_permissions_test.go @@ -0,0 +1,130 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestTaskAndAddReactionJobsPermissions(t *testing.T) { + // Test that task and add_reaction jobs do not have contents permissions and checkout steps + tmpDir, err := os.MkdirTemp("", "permissions-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Create a test workflow with both task job and add_reaction job + testContent := `--- +on: + issues: + types: [opened] +ai-reaction: eyes +tools: + github: + allowed: [list_issues] +engine: claude +--- + +# Test Workflow for Task and Add Reaction + +This workflow should generate both task and add_reaction jobs without contents permissions. + +The task job references text output: "${{ needs.task.outputs.text }}" +` + + testFile := filepath.Join(tmpDir, "test-permissions.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler(false, "", "test") + + // Compile the workflow + err = compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Calculate the lock file path + lockFile := strings.TrimSuffix(testFile, ".md") + ".lock.yml" + + // Read the generated lock file + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + lockContentStr := string(lockContent) + + // Test 1: Verify task job exists and has no contents permission + if !strings.Contains(lockContentStr, "task:") { + t.Error("Expected task job to be present in generated workflow") + } + + // Test 2: Verify task job has no checkout step + taskJobSection := extractJobSection(lockContentStr, "task") + if strings.Contains(taskJobSection, "actions/checkout") { + t.Error("Task job should not contain actions/checkout step") + } + + // Test 3: Verify task job has no contents permission + if strings.Contains(taskJobSection, "contents:") { + t.Error("Task job should not have contents permission") + } + + // Test 4: Verify add_reaction job exists and has no contents permission + if !strings.Contains(lockContentStr, "add_reaction:") { + t.Error("Expected add_reaction job to be present in generated workflow") + } + + // Test 5: Verify add_reaction job has no checkout step + addReactionJobSection := extractJobSection(lockContentStr, "add_reaction") + if strings.Contains(addReactionJobSection, "actions/checkout") { + t.Error("Add_reaction job should not contain actions/checkout step") + } + + // Test 6: Verify add_reaction job has no contents permission + if strings.Contains(addReactionJobSection, "contents:") { + t.Error("Add_reaction job should not have contents permission") + } + + // Test 7: Verify add_reaction job still has required permissions + if !strings.Contains(addReactionJobSection, "issues: write") { + t.Error("Add_reaction job should still have issues: write permission") + } + if !strings.Contains(addReactionJobSection, "pull-requests: write") { + t.Error("Add_reaction job should still have pull-requests: write permission") + } +} + +// extractJobSection extracts a specific job section from the YAML content +func extractJobSection(yamlContent, jobName string) string { + lines := strings.Split(yamlContent, "\n") + var jobLines []string + inJob := false + jobPrefix := " " + jobName + ":" + + for i, line := range lines { + if strings.HasPrefix(line, jobPrefix) { + inJob = true + jobLines = append(jobLines, line) + continue + } + + if inJob { + // If we hit another job at the same level (starts with " " and ends with ":"), stop + if strings.HasPrefix(line, " ") && strings.HasSuffix(line, ":") && !strings.HasPrefix(line, " ") { + break + } + // If we hit the end of jobs section, stop + if strings.HasPrefix(line, "jobs:") && i > 0 { + break + } + jobLines = append(jobLines, line) + } + } + + return strings.Join(jobLines, "\n") +}