Skip to content

Conversation

@gusfcarvalho
Copy link
Contributor

No description provided.

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
@gusfcarvalho gusfcarvalho marked this pull request as ready for review January 7, 2026 10:01
@gusfcarvalho gusfcarvalho requested a review from Copilot January 7, 2026 10:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements the first version of a Jira compliance plugin for the Continuous Compliance Framework. The plugin collects data from Jira Platform, Service Management, and Software APIs to evaluate change request compliance and generate evidence.

Key changes:

  • Implements OAuth2 and token-based authentication for Jira Cloud APIs
  • Collects comprehensive Jira data including projects, workflows, issues, approvals, SLAs, and deployment information
  • Integrates with the compliance-framework agent using the plugin architecture
  • Sets up CI/CD workflows for testing and releasing

Reviewed changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
main.go Core plugin implementation with configuration handling, data collection orchestration, and policy evaluation logic
internal/jira/types.go Type definitions for Jira API responses including custom time handling for audit records
internal/jira/client.go HTTP client implementation with OAuth2 and token authentication, API endpoint methods for data fetching
go.mod Go module definition with dependencies (contains invalid Go version)
go.sum Dependency checksums for reproducible builds
PLAN.md Development roadmap and architecture documentation
Makefile Build automation with clean and build targets
.goreleaser.yaml Release configuration for multi-platform builds
.gitignore Standard ignore patterns for Go projects
.github/workflows/*.yml CI/CD workflows for testing, building, and releasing the plugin

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

func (c *Client) FetchProjectRemoteLinks(ctx context.Context, projectKey string) ([]JiraRemoteLink, error) {
// Placeholder as requested in original code
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FetchProjectRemoteLinks method is a placeholder that always returns nil without any implementation. If this functionality is not yet needed, it should be documented with a TODO comment explaining why it's not implemented or when it will be implemented.

Suggested change
// Placeholder as requested in original code
// TODO: Implement fetching project remote links for the given projectKey via the Jira API
// when this functionality is required by the application.

Copilot uses AI. Check for mistakes.
main.go Outdated
Comment on lines 166 to 167
indentedJSON, _ := json.MarshalIndent(jiraData, "", " ")
os.WriteFile("/data/jira_data.json", indentedJSON, 0o644)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing collected data to a hard-coded file path '/data/jira_data.json' is problematic for several reasons: the path may not exist, it lacks proper error handling, and it's not configurable. Consider making this path configurable or removing this debug code entirely before production use.

Copilot uses AI. Check for mistakes.
main.go Outdated
Comment on lines 166 to 167
indentedJSON, _ := json.MarshalIndent(jiraData, "", " ")
os.WriteFile("/data/jira_data.json", indentedJSON, 0o644)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error from writing the file is ignored (line 167). While the data is also written successfully before this (line 166), the file write error should be checked and logged at minimum to avoid silent failures during debugging.

Copilot uses AI. Check for mistakes.

func fetchCloudID(baseURL string) (string, error) {
// Make request to get tenant info
resp, err := http.Get(baseURL + "/_edge/tenant_info")
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fetchCloudID function uses http.Get without any timeout configured, which could cause the request to hang indefinitely. Consider using a context with timeout or creating an http.Client with appropriate timeout settings.

Copilot uses AI. Check for mistakes.
// Clone the request and set the bearer token
newReq := req.Clone(req.Context())
newReq.Header.Set("Authorization", "Bearer "+tokenResp.AccessToken)
t.logger.Debug("Making request with Bearer token", "url", newReq.URL.String(), "auth", "Bearer "+tokenResp.AccessToken[:10]+"...")
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sensitive information (the first 10 characters of the access token) is being logged. Even partial token exposure in logs can be a security risk and should be avoided. Consider logging only that authentication succeeded without including token data.

Suggested change
t.logger.Debug("Making request with Bearer token", "url", newReq.URL.String(), "auth", "Bearer "+tokenResp.AccessToken[:10]+"...")
t.logger.Debug("Making request with Bearer token", "url", newReq.URL.String(), "auth", "Bearer <redacted>")

Copilot uses AI. Check for mistakes.
main.go Outdated
return nil
}

// TrackedFileInfo holds information about a tracked file and its attestation
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says "TrackedFileInfo holds information about a tracked file and its attestation" but the struct is actually JiraPlugin, not TrackedFileInfo. This appears to be copy-pasted documentation that was not updated.

Suggested change
// TrackedFileInfo holds information about a tracked file and its attestation
// JiraPlugin implements the Jira integration plugin, managing configuration and the Jira HTTP client.

Copilot uses AI. Check for mistakes.
Comment on lines 456 to 457
url := fmt.Sprintf("/rest/api/3/workflowscheme/project?%s", params.Encode())
resp, err := c.do(ctx, "GET", url, nil)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name 'url' shadows the imported 'url' package (imported on line 9), which can lead to confusion and potential bugs when trying to use url package functions later in this function.

Suggested change
url := fmt.Sprintf("/rest/api/3/workflowscheme/project?%s", params.Encode())
resp, err := c.do(ctx, "GET", url, nil)
endpoint := fmt.Sprintf("/rest/api/3/workflowscheme/project?%s", params.Encode())
resp, err := c.do(ctx, "GET", endpoint, nil)

Copilot uses AI. Check for mistakes.
Comment on lines 281 to 293
// Read the raw response for debugging
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
c.Logger.Info("Raw audit records response", "body", string(body))

var result struct {
Records []JiraAuditRecord `json:"records"`
}
if err := json.Unmarshal(body, &result); err != nil {
return nil, err
}
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response body is read twice: once for debugging output (line 282) and then again for unmarshaling (line 291). After reading the body the first time, it's consumed and cannot be read again. This will cause json.Unmarshal to fail. The body should only be read once, or the bytes should be reused.

Copilot uses AI. Check for mistakes.
Comment on lines 76 to 118
func (t *oauth2Transport) RoundTrip(req *http.Request) (*http.Response, error) {
// Get OAuth2 token with required scopes
tokenReq, err := http.NewRequest("POST", t.tokenURL, nil)
if err != nil {
return nil, err
}
tokenReq.SetBasicAuth(t.clientID, t.secret)
tokenReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")

// Form encode the parameters
data := url.Values{}
data.Set("grant_type", "client_credentials")
data.Set("scope", "read:jira-user read:jira-work read:workflow:jira read:permission:jira read:workflow-scheme:jira read:audit-log:jira read:avatar:jira read:group:jira read:issue-type:jira read:project-category:jira read:project:jira read:user:jira read:application-role:jira")
tokenReq.Body = io.NopCloser(strings.NewReader(data.Encode()))

resp, err := t.base.RoundTrip(tokenReq)
if err != nil {
return nil, err
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("failed to get token: %d, body: %s", resp.StatusCode, string(body))
}

var tokenResp struct {
AccessToken string `json:"access_token"`
Scope string `json:"scope"`
ExpiresIn int `json:"expires_in"`
TokenType string `json:"token_type"`
}
body, _ := io.ReadAll(resp.Body)
if err := json.Unmarshal(body, &tokenResp); err != nil {
return nil, fmt.Errorf("failed to decode token response: %w, body: %s", err, string(body))
}
t.logger.Debug("Got OAuth2 token", "scope", tokenResp.Scope, "expiresIn", tokenResp.ExpiresIn)

// Clone the request and set the bearer token
newReq := req.Clone(req.Context())
newReq.Header.Set("Authorization", "Bearer "+tokenResp.AccessToken)
t.logger.Debug("Making request with Bearer token", "url", newReq.URL.String(), "auth", "Bearer "+tokenResp.AccessToken[:10]+"...")
return t.base.RoundTrip(newReq)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OAuth2 token is fetched on every request in the RoundTrip method, which is inefficient. The token should be cached and only refreshed when it expires. This will cause significant performance issues with multiple API calls.

Copilot uses AI. Check for mistakes.
Comment on lines 128 to 129
req.SetBasicAuth(t.email, t.token)
return t.base.RoundTrip(req)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tokenAuthTransport.RoundTrip method modifies the original request object directly with SetBasicAuth, which could cause issues if the request is reused. The request should be cloned before modification, similar to how it's done in oauth2Transport.

Suggested change
req.SetBasicAuth(t.email, t.token)
return t.base.RoundTrip(req)
newReq := req.Clone(req.Context())
newReq.SetBasicAuth(t.email, t.token)
return t.base.RoundTrip(newReq)

Copilot uses AI. Check for mistakes.
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 14 changed files in this pull request and generated 14 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

main.go Outdated
Comment on lines 181 to 183
jiraJSON, _ := json.MarshalIndent(jiraData, "", " ")
converted := jiraData.ToProjectCentric()
indentedJSON, _ := json.MarshalIndent(converted, "", " ")
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error return values from json.MarshalIndent are silently ignored. These marshaling operations could fail, and the errors should be handled appropriately or at least logged.

Copilot uses AI. Check for mistakes.
Comment on lines 76 to 119
func (t *oauth2Transport) RoundTrip(req *http.Request) (*http.Response, error) {
// Get OAuth2 token with required scopes
tokenReq, err := http.NewRequest("POST", t.tokenURL, nil)
if err != nil {
return nil, err
}
tokenReq.SetBasicAuth(t.clientID, t.secret)
tokenReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")

// Form encode the parameters
data := url.Values{}
data.Set("grant_type", "client_credentials")
data.Set("scope", "read:jira-user read:jira-work read:workflow:jira read:permission:jira read:workflow-scheme:jira read:audit-log:jira read:avatar:jira read:group:jira read:issue-type:jira read:project-category:jira read:project:jira read:user:jira read:application-role:jira read:servicedesk-request")
tokenReq.Body = io.NopCloser(strings.NewReader(data.Encode()))

resp, err := t.base.RoundTrip(tokenReq)
if err != nil {
return nil, err
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("failed to get token: %d, body: %s", resp.StatusCode, string(body))
}

var tokenResp struct {
AccessToken string `json:"access_token"`
Scope string `json:"scope"`
ExpiresIn int `json:"expires_in"`
TokenType string `json:"token_type"`
}
body, _ := io.ReadAll(resp.Body)
if err := json.Unmarshal(body, &tokenResp); err != nil {
return nil, fmt.Errorf("failed to decode token response: %w, body: %s", err, string(body))
}
t.logger.Debug("Got OAuth2 token", "scope", tokenResp.Scope, "expiresIn", tokenResp.ExpiresIn)

// Clone the request and set the bearer token
newReq := req.Clone(req.Context())
newReq.Header.Set("Authorization", "Bearer "+tokenResp.AccessToken)
t.logger.Debug("Making request with Bearer token", "url", newReq.URL.String(), "auth", "Bearer "+tokenResp.AccessToken[:10]+"...")
return t.base.RoundTrip(newReq)
}
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OAuth2 token is fetched on every request in the RoundTrip method. This is inefficient and could cause rate limiting issues. Implement token caching with expiration tracking to reuse tokens until they expire.

Copilot uses AI. Check for mistakes.
c.Logger.Error("Error fetching SLAs", "issue", issueKey, "error", err)
return nil, err
}
defer resp.Body.Close()
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple deferred resp.Body.Close() calls in a loop can lead to resource leaks because deferred functions in loops don't execute until the function returns. The response bodies should be closed immediately after processing each response, not deferred.

Copilot uses AI. Check for mistakes.
if err != nil {
return nil, err
}
defer resp.Body.Close()
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple deferred resp.Body.Close() calls in a loop can lead to resource leaks because deferred functions in loops don't execute until the function returns. The response bodies should be closed immediately after processing each response, not deferred.

Copilot uses AI. Check for mistakes.
Comment on lines 196 to 206
defer resp.Body.Close()
c.logSuccessOrWarn("FetchProjects", resp, err)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}
body := resp.Body
var searchResp JiraProjectSearchResponse
if err := json.NewDecoder(body).Decode(&searchResp); err != nil {
return nil, err
}
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple deferred resp.Body.Close() calls in a loop can lead to resource leaks because deferred functions in loops don't execute until the function returns. The response bodies should be closed immediately after processing each response, not deferred.

Suggested change
defer resp.Body.Close()
c.logSuccessOrWarn("FetchProjects", resp, err)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}
body := resp.Body
var searchResp JiraProjectSearchResponse
if err := json.NewDecoder(body).Decode(&searchResp); err != nil {
return nil, err
}
c.logSuccessOrWarn("FetchProjects", resp, err)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
resp.Body.Close()
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}
var searchResp JiraProjectSearchResponse
if err := json.NewDecoder(resp.Body).Decode(&searchResp); err != nil {
resp.Body.Close()
return nil, err
}
resp.Body.Close()

Copilot uses AI. Check for mistakes.
Comment on lines 260 to 270
defer resp.Body.Close()
c.logSuccessOrWarn("FetchWorkflows", resp, err)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}

var searchResp JiraWorkflowSearchResponse
if err := json.NewDecoder(resp.Body).Decode(&searchResp); err != nil {
return nil, err
}
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple deferred resp.Body.Close() calls in a loop can lead to resource leaks because deferred functions in loops don't execute until the function returns. The response bodies should be closed immediately after processing each response, not deferred.

Suggested change
defer resp.Body.Close()
c.logSuccessOrWarn("FetchWorkflows", resp, err)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}
var searchResp JiraWorkflowSearchResponse
if err := json.NewDecoder(resp.Body).Decode(&searchResp); err != nil {
return nil, err
}
c.logSuccessOrWarn("FetchWorkflows", resp, err)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
resp.Body.Close()
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}
var searchResp JiraWorkflowSearchResponse
if err := json.NewDecoder(resp.Body).Decode(&searchResp); err != nil {
resp.Body.Close()
return nil, err
}
resp.Body.Close()

Copilot uses AI. Check for mistakes.
if err != nil {
return nil, err
}
defer resp.Body.Close()
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple deferred resp.Body.Close() calls in a loop can lead to resource leaks because deferred functions in loops don't execute until the function returns. The response bodies should be closed immediately after processing each response, not deferred.

Copilot uses AI. Check for mistakes.
if err != nil {
return nil, err
}
defer resp.Body.Close()
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple deferred resp.Body.Close() calls in a loop can lead to resource leaks because deferred functions in loops don't execute until the function returns. The response bodies should be closed immediately after processing each response, not deferred.

Copilot uses AI. Check for mistakes.
Comment on lines 664 to 675
defer resp.Body.Close()
c.logSuccessOrWarn("GetAllStatuses", resp, err)

if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}

var searchResp JiraStatusSearchResponse
if err := json.NewDecoder(resp.Body).Decode(&searchResp); err != nil {
return nil, err
}
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple deferred resp.Body.Close() calls in a loop can lead to resource leaks because deferred functions in loops don't execute until the function returns. The response bodies should be closed immediately after processing each response, not deferred.

Suggested change
defer resp.Body.Close()
c.logSuccessOrWarn("GetAllStatuses", resp, err)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}
var searchResp JiraStatusSearchResponse
if err := json.NewDecoder(resp.Body).Decode(&searchResp); err != nil {
return nil, err
}
c.logSuccessOrWarn("GetAllStatuses", resp, err)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
resp.Body.Close()
return nil, fmt.Errorf("unexpected status code: %d, body: %s", resp.StatusCode, string(body))
}
var searchResp JiraStatusSearchResponse
if err := json.NewDecoder(resp.Body).Decode(&searchResp); err != nil {
resp.Body.Close()
return nil, err
}
resp.Body.Close()

Copilot uses AI. Check for mistakes.
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
@gusfcarvalho gusfcarvalho merged commit d9896ab into main Jan 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants