Conversation
📝 WalkthroughWalkthroughIntroduces automated Polaris findings extraction via a Python script that fetches issues from Polaris portfolios/projects and generates JSON and SARIF-formatted outputs. Pairs with a new GitHub Actions workflow that executes the script on a weekly schedule and uploads results to GitHub's security tab. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
.github/tools/extract_findings.py (6)
8-10: Useis Noneinstead of== Nonefor None comparisons.Python best practice (PEP 8) recommends using
is Nonefor identity checks rather than== None.🔎 Proposed fix
- if params == None: + if params is None: params = {} - if headers == None: + if headers is None: headers = {}And at line 42:
- if params == None: + if params is None: params = {}Also applies to: 42-42
16-19: Avoid bareexceptclauses.Bare
exceptcatches all exceptions includingSystemExitandKeyboardInterrupt, which can make debugging difficult. Catch specific exceptions instead.🔎 Proposed fix
For lines 16-19:
- try: + try: print(response.json()) - except: + except Exception: print("No json data returned")For lines 70-74:
try: return(resp) - except: - print(f"ERROR: No issues found") + except Exception as e: + print(f"ERROR: No issues found: {e}") sys.exit(1)Also applies to: 70-74
23-31: Use a more descriptive variable name.The variable name
lis ambiguous and can be confused with1orI. Use a more descriptive name likelink.🔎 Proposed fix
def getNextAndFirst(links): nextlink = None firstlink = None - for l in links: - if l['rel'] == 'next': - nextlink = l['href'] - if l['rel'] == 'first': - firstlink = l['href'] + for link in links: + if link['rel'] == 'next': + nextlink = link['href'] + if link['rel'] == 'first': + firstlink = link['href'] return nextlink, firstlink
73-73: Remove unnecessaryfprefix from string.The string has no placeholders, so the
fprefix is unnecessary.🔎 Proposed fix
- print(f"ERROR: No issues found") + print("ERROR: No issues found")
76-80: Remove unusedurlparameter.The
urlparameter is not used in the function body and can be removed.🔎 Proposed fix
-def createSession(url, token): +def createSession(token): headers = {'API-TOKEN': token} s = requests.Session() s.headers.update(headers) return sDon't forget to update the call site at line 104:
- session = createSession(url, token) + session = createSession(token)
130-130: Move import to module level.The
osmodule import should be at the top of the file with other imports, following Python conventions (PEP 8).🔎 Proposed fix
At the top of the file (after line 4):
import re +import osThen remove the import from line 130:
# Remove old output files if they exist - import os json_path = "issues_output.json".github/workflows/extract_polaris_findings.yml (1)
37-52: Consider uploadingissues_output.jsonas a workflow artifact.The script generates
issues_output.jsonin addition to the SARIF file, but it's not uploaded as an artifact. While the SARIF file is the primary output for GitHub Security, the JSON file could be useful for debugging or further analysis.🔎 Proposed addition
Add this step after the script execution and before SARIF upload:
- name: Upload issues JSON as artifact uses: actions/upload-artifact@v4 with: name: polaris-issues-json-${{ matrix.name }} path: .github/tools/issues_output.json retention-days: 30
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/tools/extract_findings.py(1 hunks).github/workflows/extract_polaris_findings.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CelineTrammi
Repo: Altinn/altinn-correspondence PR: 421
File: src/Altinn.Correspondence.Application/DownloadCorrespondenceAttachment/LegacyDownloadCorrespondenceAttachmentHandler.cs:52-52
Timestamp: 2024-11-01T13:32:16.888Z
Learning: When reviewing pull requests in the `Altinn/altinn-correspondence` repository, suggestions for adding error handling may be considered outside the scope if they are not directly related to the PR's objectives.
🪛 Ruff (0.14.8)
.github/tools/extract_findings.py
8-8: Comparison to None should be cond is None
Replace with cond is None
(E711)
10-10: Comparison to None should be cond is None
Replace with cond is None
(E711)
18-18: Do not use bare except
(E722)
26-26: Ambiguous variable name: l
(E741)
42-42: Comparison to None should be cond is None
Replace with cond is None
(E711)
72-72: Do not use bare except
(E722)
73-73: f-string without any placeholders
Remove extraneous f prefix
(F541)
76-76: Unused function argument: url
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (csharp)
- GitHub Check: Test application
🔇 Additional comments (5)
.github/workflows/extract_polaris_findings.yml (5)
4-6: LGTM! Appropriate minimal permissions.The workflow uses minimal permissions following security best practices:
contents: readfor checkout andsecurity-events: writefor SARIF upload.
8-11: LGTM! Appropriate triggers configured.Weekly schedule on Mondays and manual dispatch capability are well-suited for periodic security scanning.
16-23: LGTM! Matrix strategy with fail-fast disabled.The matrix approach with
fail-fast: falseensures both projects are scanned independently, even if one fails.
48-52: LGTM! SARIF upload properly configured.The SARIF upload uses the correct action with per-project categories for proper organization in GitHub Security.
37-46: Verify that all required secrets are configured.The workflow depends on four secrets:
POLARIS_URL,POLARIS_TOKEN,POLARIS_PORTFOLIO_ID,POLARIS_PROJECT_ID_ALTINN_BROKER, andPOLARIS_PROJECT_ID_CORRESPONDENCE. Ensure these are properly configured in the repository settings.Run the following command to check if the secrets are configured (note: this will only confirm they exist, not their values):
#!/bin/bash # Description: Check if required secrets are configured in the repository gh api repos/Altinn/altinn-correspondence/actions/secrets --jq '.secrets[].name' | grep -E 'POLARIS_(URL|TOKEN|PORTFOLIO_ID|PROJECT_ID_ALTINN_BROKER|PROJECT_ID_CORRESPONDENCE)'
| "tool": { | ||
| "driver": { | ||
| "name": "DAST-Scanner", | ||
| "rules": [] | ||
| } |
There was a problem hiding this comment.
Incorrect tool name: Polaris is a SAST tool, not DAST.
The tool name is set to "DAST-Scanner" (Dynamic Application Security Testing), but Polaris is a SAST (Static Application Security Testing) tool. This misidentification could mislead users viewing the security findings.
🔎 Proposed fix
"tool": {
"driver": {
- "name": "DAST-Scanner",
+ "name": "Polaris",
"rules": []
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "tool": { | |
| "driver": { | |
| "name": "DAST-Scanner", | |
| "rules": [] | |
| } | |
| "tool": { | |
| "driver": { | |
| "name": "Polaris", | |
| "rules": [] | |
| } |
🤖 Prompt for AI Agents
In .github/tools/extract_findings.py around lines 148 to 152, the tool driver
name is incorrectly set to "DAST-Scanner" although Polaris is a SAST tool;
update the "name" value to reflect Polaris (for example "Polaris" or
"Polaris-SAST") and, if there is a separate field for testing type, ensure it
indicates SAST rather than DAST so the generated findings correctly identify the
scanner as a static analysis tool.
| artifacts.append({ | ||
| "location": { | ||
| "uri": file_path, | ||
| "uriBaseId": "SRCROOT" | ||
| }, | ||
| "sourceLanguage": "python" | ||
| }) |
There was a problem hiding this comment.
Incorrect hardcoded sourceLanguage value.
The sourceLanguage is hardcoded to "python", but Polaris scans multiple languages (Java, C#, JavaScript, Python, etc.). This incorrectly labels all findings as Python.
🔎 Proposed fix
Either infer the language from file extension or omit the field (it's optional in SARIF):
Option 1: Infer from file extension
+ # Infer language from file extension
+ extension = file_path.split('.')[-1].lower() if '.' in file_path else ''
+ language_map = {
+ 'py': 'python', 'java': 'java', 'cs': 'csharp',
+ 'js': 'javascript', 'ts': 'typescript', 'cpp': 'cpp',
+ 'c': 'c', 'go': 'go', 'rb': 'ruby'
+ }
+ source_language = language_map.get(extension, 'unknown')
+
artifacts.append({
"location": {
"uri": file_path,
"uriBaseId": "SRCROOT"
},
- "sourceLanguage": "python"
+ "sourceLanguage": source_language
})Option 2: Omit the field (simpler)
artifacts.append({
"location": {
"uri": file_path,
"uriBaseId": "SRCROOT"
- },
- "sourceLanguage": "python"
+ }
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| artifacts.append({ | |
| "location": { | |
| "uri": file_path, | |
| "uriBaseId": "SRCROOT" | |
| }, | |
| "sourceLanguage": "python" | |
| }) | |
| # Infer language from file extension | |
| extension = file_path.split('.')[-1].lower() if '.' in file_path else '' | |
| language_map = { | |
| 'py': 'python', 'java': 'java', 'cs': 'csharp', | |
| 'js': 'javascript', 'ts': 'typescript', 'cpp': 'cpp', | |
| 'c': 'c', 'go': 'go', 'rb': 'ruby' | |
| } | |
| source_language = language_map.get(extension, 'unknown') | |
| artifacts.append({ | |
| "location": { | |
| "uri": file_path, | |
| "uriBaseId": "SRCROOT" | |
| }, | |
| "sourceLanguage": source_language | |
| }) |
| artifacts.append({ | |
| "location": { | |
| "uri": file_path, | |
| "uriBaseId": "SRCROOT" | |
| }, | |
| "sourceLanguage": "python" | |
| }) | |
| artifacts.append({ | |
| "location": { | |
| "uri": file_path, | |
| "uriBaseId": "SRCROOT" | |
| } | |
| }) |
🤖 Prompt for AI Agents
In .github/tools/extract_findings.py around lines 223-229 the artifacts dict
unconditionally sets "sourceLanguage": "python", mislabeling findings for other
file types; change this by either removing the sourceLanguage key entirely
(simpler) or replacing it with logic that maps the file extension to the correct
SARIF language identifier (e.g., .py -> "python", .js -> "javascript", .ts ->
"typescript", .java -> "java", .cs -> "csharp") and only include the field when
a known mapping exists (omit for unknown extensions).
| # Build direct link to the specific issue in Polaris | ||
| issue_url = f"https://eu.polaris.blackduck.com/portfolio/portfolios/{portfolio_id}/portfolio-items/{application_id}/projects/{project_id}/issues/{rule_id}?filter=triage%3Astatus%3Dnot-dismissed%2Cto-be-fixed" |
There was a problem hiding this comment.
Hardcoded Polaris URL may not match the actual instance.
The issue URL uses a hardcoded domain eu.polaris.blackduck.com, but the script accepts POLARIS_URL as a CLI argument which may point to a different instance. This will result in broken links in the SARIF output.
🔎 Proposed fix
# Build direct link to the specific issue in Polaris
- issue_url = f"https://eu.polaris.blackduck.com/portfolio/portfolios/{portfolio_id}/portfolio-items/{application_id}/projects/{project_id}/issues/{rule_id}?filter=triage%3Astatus%3Dnot-dismissed%2Cto-be-fixed"
+ issue_url = f"{url}/portfolio/portfolios/{portfolio_id}/portfolio-items/{application_id}/projects/{project_id}/issues/{rule_id}?filter=triage%3Astatus%3Dnot-dismissed%2Cto-be-fixed"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Build direct link to the specific issue in Polaris | |
| issue_url = f"https://eu.polaris.blackduck.com/portfolio/portfolios/{portfolio_id}/portfolio-items/{application_id}/projects/{project_id}/issues/{rule_id}?filter=triage%3Astatus%3Dnot-dismissed%2Cto-be-fixed" | |
| # Build direct link to the specific issue in Polaris | |
| issue_url = f"{url}/portfolio/portfolios/{portfolio_id}/portfolio-items/{application_id}/projects/{project_id}/issues/{rule_id}?filter=triage%3Astatus%3Dnot-dismissed%2Cto-be-fixed" |
🤖 Prompt for AI Agents
In .github/tools/extract_findings.py around lines 238 to 239, the code builds a
Polaris issue URL using a hardcoded domain (eu.polaris.blackduck.com) which
breaks when POLARIS_URL CLI arg points to a different instance; replace the
hardcoded domain with the actual POLARIS_URL value passed into the script,
constructing the path against POLARIS_URL (handle trailing/leading slashes
correctly, e.g., strip trailing slash from POLARIS_URL or use a proper URL join)
and ensure any path segments or query parameters are URL-encoded as needed so
generated issue links work for all Polaris instances.
2 participants
Description
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.