send workflow url from cli #2048
Conversation
Bug Summary ReportTotal Bugs Found: 3Bug Summaries
These bugs primarily affect URL handling and validation, which could impact user experience through broken links and missing workflow information. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements a workflow URL optimization feature to reduce GitHub API rate limiting by having the CLI provide workflow URLs directly rather than fetching them through API calls later. The changes span two key areas:
-
CLI-side enhancement (
libs/backendapi/diggerapi.go): Adds two helper functionsisGitHubActions()andgetWorkflowUrl()that detect when running in GitHub Actions and construct workflow URLs from environment variables (GITHUB_SERVER_URL,GITHUB_REPOSITORY,GITHUB_RUN_ID). The workflow URL is now included in theReportProjectJobStatuspayload sent to the backend. -
Backend-side enhancement (
backend/controllers/projects.go): Adds a new optionalWorkflowUrlfield to theSetJobStatusRequeststruct and updates the job'sWorkflowRunUrlfield when a workflow URL is provided during job startup (status = 'started').
This optimization fits into the existing codebase by leveraging the established job status reporting mechanism between CLI and backend. Previously, the system would make additional GitHub API calls after job completion to retrieve workflow URLs (as seen in the existing GetWorkflowIdAndUrlFromDiggerJobId function). Now, the CLI proactively provides this information, reducing API calls and improving rate limit management.
Confidence score: 4/5
- This is a safe optimization that adds functionality without breaking existing behavior.
- The implementation is straightforward and follows existing patterns, with proper fallback handling.
- The
backend/controllers/projects.gofile needs attention due to potential edge cases around URL validation and existing workflow URL handling logic.
2 files reviewed, 1 comment
backend/controllers/projects.go
Outdated
| if request.WorkflowUrl != "" { | ||
| job.WorkflowRunUrl = &request.WorkflowUrl | ||
| } |
There was a problem hiding this comment.
style: Consider adding debug logging here to track when workflow URL is provided by CLI versus fetched later from GitHub API
![bismuthdev[bot]](https://avatars.githubusercontent.com/in/957724?s=60&v=4){kind=small}
| func getWorkflowUrl() string { | ||
| if isGitHubActions() { | ||
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | ||
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | ||
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | ||
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | ||
| return workflowURL | ||
| } else { | ||
| return "#" | ||
| } |
There was a problem hiding this comment.
The getWorkflowUrl() function constructs a URL using environment variables (GITHUB_SERVER_URL, GITHUB_REPOSITORY, GITHUB_RUN_ID) without validating if they exist or have valid values. If any of these variables are missing or empty, the function will still attempt to construct a URL with empty values, resulting in an invalid URL like "//actions/runs/" or similar malformed URLs.
This could lead to broken links in the application UI or API responses. The fix adds validation to check if any of the required environment variables are missing or empty, and returns a fallback value ("#") in that case, which is consistent with the non-GitHub Actions case.
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } | |
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| // Validate that all required environment variables are present | |
| if githubServerURL == "" || githubRepository == "" || githubRunID == "" { | |
| slog.Warn("Missing required GitHub environment variables for workflow URL", | |
| "GITHUB_SERVER_URL", githubServerURL, | |
| "GITHUB_REPOSITORY", githubRepository, | |
| "GITHUB_RUN_ID", githubRunID) | |
| return "#" | |
| } | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } |
| func getWorkflowUrl() string { | ||
| if isGitHubActions() { | ||
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | ||
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | ||
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | ||
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | ||
| return workflowURL | ||
| } else { | ||
| return "#" | ||
| } | ||
| } |
There was a problem hiding this comment.
The getWorkflowUrl() function constructs a GitHub workflow URL using environment variables (GITHUB_SERVER_URL, GITHUB_REPOSITORY, and GITHUB_RUN_ID) without validating if these variables are actually set. If any of these environment variables are missing, the function will still attempt to construct a URL with empty values, potentially resulting in an invalid URL. This could lead to broken links in the UI or other unexpected behavior.
The fix adds validation to check if the required environment variables are present before constructing the URL. If any of them are missing, it logs a warning with the specific missing variables and returns the fallback value "#" instead.
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } | |
| } | |
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| // Validate environment variables are present | |
| if githubServerURL == "" || githubRepository == "" || githubRunID == "" { | |
| slog.Warn("Missing GitHub environment variables for workflow URL construction", | |
| "GITHUB_SERVER_URL", githubServerURL, | |
| "GITHUB_REPOSITORY", githubRepository, | |
| "GITHUB_RUN_ID", githubRunID, | |
| ) | |
| return "#" | |
| } | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } | |
| } |
backend/controllers/projects.go
Outdated
| if request.WorkflowUrl != "" { | ||
| job.WorkflowRunUrl = &request.WorkflowUrl | ||
| } |
There was a problem hiding this comment.
The code doesn't validate the URL format before assigning request.WorkflowUrl to job.WorkflowRunUrl. This could lead to storing invalid URLs in the database, which might cause issues when these URLs are later used (e.g., in UI displays, API responses, or when attempting to make requests to these URLs).
The fix adds URL validation using Go's standard url.Parse() function, which checks if the URL format is valid. If the URL is invalid, it logs a warning and doesn't store the URL. This prevents invalid URLs from being stored in the database while maintaining the existing behavior for valid URLs.
| if request.WorkflowUrl != "" { | |
| job.WorkflowRunUrl = &request.WorkflowUrl | |
| } | |
| if request.WorkflowUrl != "" { | |
| // Validate URL format before storing | |
| _, err := url.Parse(request.WorkflowUrl) | |
| if err != nil { | |
| slog.Warn("Invalid workflow URL format", | |
| "jobId", jobId, | |
| "workflowUrl", request.WorkflowUrl, | |
| "error", err, | |
| ) | |
| } else { | |
| job.WorkflowRunUrl = &request.WorkflowUrl | |
| } | |
| } |
| func getWorkflowUrl() string { | ||
| if isGitHubActions() { | ||
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | ||
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | ||
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | ||
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | ||
| return workflowURL | ||
| } else { | ||
| return "#" | ||
| } |
There was a problem hiding this comment.
The getWorkflowUrl() function only handles GitHub Actions environments and returns a placeholder "#" for all other CI/CD environments. This means that when the code runs in other CI/CD environments like GitLab CI, Jenkins, or CircleCI, users won't get proper links to their workflow runs.
Each CI/CD platform provides environment variables that can be used to construct workflow URLs:
- GitLab CI provides
CI_PROJECT_URLandCI_PIPELINE_ID - Jenkins provides
JENKINS_URL,JOB_NAME, andBUILD_NUMBER - CircleCI provides
CIRCLE_BUILD_URL
The fix adds support for these common CI/CD platforms by checking for their specific environment variables and constructing appropriate workflow URLs for each platform.
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } | |
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else if os.Getenv("GITLAB_CI") == "true" { | |
| // GitLab CI environment | |
| projectURL := os.Getenv("CI_PROJECT_URL") | |
| pipelineID := os.Getenv("CI_PIPELINE_ID") | |
| return fmt.Sprintf("%s/-/pipelines/%s", projectURL, pipelineID) | |
| } else if os.Getenv("JENKINS_URL") != "" { | |
| // Jenkins environment | |
| jenkinsURL := os.Getenv("JENKINS_URL") | |
| jobName := os.Getenv("JOB_NAME") | |
| buildNumber := os.Getenv("BUILD_NUMBER") | |
| return fmt.Sprintf("%s/job/%s/%s", jenkinsURL, jobName, buildNumber) | |
| } else if os.Getenv("CIRCLECI") == "true" { | |
| // CircleCI environment | |
| buildURL := os.Getenv("CIRCLE_BUILD_URL") | |
| if buildURL != "" { | |
| return buildURL | |
| } | |
| return "#" | |
| } else { | |
| return "#" | |
| } |
Bug Summary ReportTotal Bugs Found: 4Summary of Bugs
The most critical bugs are the security vulnerabilities in the URL construction process, which could potentially allow for injection attacks if environment variables contain malicious values. |
| if isGitHubActions() { | ||
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | ||
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | ||
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | ||
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | ||
| return workflowURL |
There was a problem hiding this comment.
The getWorkflowUrl() function uses environment variables (GITHUB_SERVER_URL, GITHUB_REPOSITORY, and GITHUB_RUN_ID) to construct a URL without any validation or sanitization. This could lead to security issues if these environment variables are manipulated or contain malicious values.
Potential security risks include:
- URL injection attacks if the environment variables contain characters that could change the URL structure
- XSS vulnerabilities if the constructed URL is later rendered in a web context
- Potential server-side request forgery if the URL is used for internal requests
The fix adds validation to:
- Check if the environment variables are present
- Validate the format of the components before constructing the URL
- Log warnings when invalid values are detected
Note: The implementation requires adding two helper functions isValidURLComponent and isValidRunID which should be added to the file to validate the URL components and run ID format.
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| // Validate environment variables before using them | |
| if githubServerURL == "" || githubRepository == "" || githubRunID == "" { | |
| slog.Warn("Missing GitHub environment variables for workflow URL construction", | |
| "GITHUB_SERVER_URL", githubServerURL, | |
| "GITHUB_REPOSITORY", githubRepository, | |
| "GITHUB_RUN_ID", githubRunID) | |
| return "#" | |
| } | |
| // Basic validation to prevent URL injection | |
| if !isValidURLComponent(githubServerURL) || !isValidURLComponent(githubRepository) || !isValidRunID(githubRunID) { | |
| slog.Warn("Invalid GitHub environment variables detected", | |
| "GITHUB_SERVER_URL", githubServerURL, | |
| "GITHUB_REPOSITORY", githubRepository, | |
| "GITHUB_RUN_ID", githubRunID) | |
| return "#" | |
| } | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL |
| func getWorkflowUrl() string { | ||
| if isGitHubActions() { | ||
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | ||
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | ||
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | ||
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | ||
| return workflowURL | ||
| } else { | ||
| return "#" | ||
| } |
There was a problem hiding this comment.
The getWorkflowUrl() function doesn't check if the required GitHub environment variables (GITHUB_SERVER_URL, GITHUB_REPOSITORY, GITHUB_RUN_ID) are present before constructing the workflow URL. If any of these variables are missing, it will create a malformed URL with empty segments like https:///actions/runs/123 or https://github.com//actions/runs/123, which could lead to invalid links in the application.
The fix adds validation to check if all required environment variables are present before constructing the URL, and returns the fallback value (#) if any of them are missing, while also logging a warning message to help with debugging.
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } | |
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| // Check if all required environment variables are present | |
| if githubServerURL == "" || githubRepository == "" || githubRunID == "" { | |
| slog.Warn("Missing required GitHub environment variables for workflow URL", | |
| "GITHUB_SERVER_URL", githubServerURL, | |
| "GITHUB_REPOSITORY", githubRepository, | |
| "GITHUB_RUN_ID", githubRunID) | |
| return "#" | |
| } | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } |
backend/controllers/projects.go
Outdated
| } else { | ||
| slog.Debug("Workflow URL not found for job", | ||
| "jobId", jobId) | ||
| return fmt.Errorf("workflow URL not found for job (workflowUrl returned: %v)") |
There was a problem hiding this comment.
The error message in updateWorkflowUrlForJob has a formatting placeholder %v but no value is provided to fill it. This will result in incomplete error messages where the placeholder is not replaced with the actual workflow URL value. The fix adds the workflowUrl variable as a parameter to the fmt.Errorf call to properly format the error message.
| return fmt.Errorf("workflow URL not found for job (workflowUrl returned: %v)") | |
| return fmt.Errorf("workflow URL not found for job (workflowUrl returned: %v)", workflowUrl) |
| func getWorkflowUrl() string { | ||
| if isGitHubActions() { | ||
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | ||
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | ||
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | ||
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | ||
| return workflowURL | ||
| } else { | ||
| return "#" | ||
| } | ||
| } |
There was a problem hiding this comment.
The getWorkflowUrl() function constructs a URL from environment variables without validating that the required variables are present or that the resulting URL is valid. This could lead to malformed URLs being stored in the database and potentially displayed to users. The fix adds validation to ensure all required environment variables are present and that the constructed URL is valid before returning it. If any validation fails, it returns a safe fallback value ("#") and logs a warning.
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } | |
| } | |
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| // Validate that we have all required components before constructing the URL | |
| if githubServerURL == "" || githubRepository == "" || githubRunID == "" { | |
| slog.Warn("Missing GitHub environment variables for workflow URL construction", | |
| "serverURL", githubServerURL, | |
| "repository", githubRepository, | |
| "runID", githubRunID) | |
| return "#" | |
| } | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| // Validate the constructed URL | |
| _, err := url.Parse(workflowURL) | |
| if err != nil { | |
| slog.Warn("Generated invalid workflow URL", "url", workflowURL, "error", err) | |
| return "#" | |
| } | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } | |
| } |
Summary of Bugs Found in CodebaseTotal bugs found: 3 Bug Summaries
The most critical bug is the unvalidated URL construction, as it could lead to malformed URLs that break functionality or potentially create security issues with improperly escaped URL components. |
| func getWorkflowUrl() string { | ||
| if isGitHubActions() { | ||
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | ||
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | ||
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | ||
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | ||
| return workflowURL | ||
| } else { | ||
| return "#" | ||
| } | ||
| } |
There was a problem hiding this comment.
The getWorkflowUrl() function in libs/backendapi/diggerapi.go returns a hardcoded "#" value for non-GitHub Actions environments. This can cause confusion or issues in other CI systems that might have their own workflow URLs.
Looking at the codebase, I found that:
- The CiBackend interface in
backend/ci_backends/ci_backends.godefines aGetWorkflowUrlmethod that each CI backend implementation should provide. - Other CI backends like GitLab, Bitbucket, and Buildkite have their own implementations of this method.
- When the workflow URL is sent to the backend API in
ReportProjectJobStatus, it uses the hardcoded "#" for non-GitHub Actions environments, which overrides any custom URL that might be provided by other CI systems.
By returning an empty string instead of "#", we allow other CI systems to provide their own workflow URLs without being overridden by this hardcoded value. This makes the system more flexible and avoids potential confusion in the UI where a "#" link might not make sense for non-GitHub Actions environments.
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| return "#" | |
| } | |
| } | |
| func getWorkflowUrl() string { | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| } else { | |
| // Return empty string for non-GitHub Actions environments | |
| // This allows other CI systems to provide their own workflow URLs | |
| return "" | |
| } | |
| } |
| if isGitHubActions() { | ||
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | ||
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | ||
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | ||
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | ||
| return workflowURL |
There was a problem hiding this comment.
The getWorkflowUrl() function constructs a URL using environment variables without any validation. This can lead to malformed URLs if any of the environment variables (GITHUB_SERVER_URL, GITHUB_REPOSITORY, or GITHUB_RUN_ID) contain unexpected characters or are missing.
The bug has several issues:
- No validation that the required environment variables exist
- No validation that
GITHUB_SERVER_URLis a valid URL - No escaping of path components which could contain special characters
The fix:
- Checks if any required environment variables are missing
- Validates that the server URL is properly formatted
- Uses
url.PathEscape()to properly escape path components - Adds appropriate error logging
- Returns a fallback URL ("#") when validation fails
This ensures the constructed workflow URL will always be valid, even if environment variables contain unexpected characters or are missing.
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", githubServerURL, githubRepository, githubRunID) | |
| return workflowURL | |
| if isGitHubActions() { | |
| githubServerURL := os.Getenv("GITHUB_SERVER_URL") // e.g., https://github.com | |
| githubRepository := os.Getenv("GITHUB_REPOSITORY") // e.g., diggerhq/demo-opentofu | |
| githubRunID := os.Getenv("GITHUB_RUN_ID") // numeric run ID | |
| // Validate environment variables to prevent malformed URLs | |
| if githubServerURL == "" || githubRepository == "" || githubRunID == "" { | |
| slog.Warn("Missing GitHub environment variables for workflow URL construction", | |
| "serverURL", githubServerURL, | |
| "repository", githubRepository, | |
| "runID", githubRunID) | |
| return "#" | |
| } | |
| // Ensure the URL is properly constructed | |
| serverURL, err := url.Parse(githubServerURL) | |
| if err != nil { | |
| slog.Error("Invalid GitHub server URL", "url", githubServerURL, "error", err) | |
| return "#" | |
| } | |
| workflowURL := fmt.Sprintf("%s/%s/actions/runs/%s", serverURL.String(), url.PathEscape(githubRepository), url.PathEscape(githubRunID)) | |
| return workflowURL |
| func isGitHubActions() bool { | ||
| return os.Getenv("GITHUB_ACTIONS") == "true" | ||
| } |
There was a problem hiding this comment.
The current implementation of isGitHubActions() only checks if the environment variable GITHUB_ACTIONS is set to "true". This is potentially problematic because:
-
There could be cases where the
GITHUB_ACTIONSenvironment variable is not set correctly or is missing, but the code is still running in a GitHub Actions environment. -
GitHub's documentation indicates that multiple environment variables are set in GitHub Actions environments, and relying on just one could lead to false negatives.
-
In some GitHub Actions configurations or custom runners, the environment variables might be set differently.
The fix adds additional checks for other GitHub Actions-specific environment variables (GITHUB_WORKFLOW, GITHUB_RUN_ID, and GITHUB_REPOSITORY) that are typically present in GitHub Actions environments. This creates a more robust detection mechanism that will correctly identify GitHub Actions environments even if the primary environment variable is missing or incorrectly set.
| func isGitHubActions() bool { | |
| return os.Getenv("GITHUB_ACTIONS") == "true" | |
| } | |
| func isGitHubActions() bool { | |
| // Check for GITHUB_ACTIONS environment variable | |
| if os.Getenv("GITHUB_ACTIONS") == "true" { | |
| return true | |
| } | |
| // Additional checks for GitHub Actions environment | |
| // Check for other GitHub Actions specific environment variables | |
| if os.Getenv("GITHUB_WORKFLOW") != "" && | |
| os.Getenv("GITHUB_RUN_ID") != "" && | |
| os.Getenv("GITHUB_REPOSITORY") != "" { | |
| return true | |
| } | |
| return false | |
| } |
No description provided.