Create ethicalcheck.yml - #200
Closed
marlon-costa-dc wants to merge 1 commit into
Closed
Conversation
|
Warning Review limit reached
Next review available in: 55 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
Contributor
PR Summary by QodoAdd EthicalCheck GitHub Action to run API security scans and upload SARIF
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Contributor
Code Review by Qodo
1. Hardcoded oas-url and email
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+50
to
+52
| permissions: | ||
| security-events: write # for github/codeql-action/upload-sarif to upload SARIF results | ||
| actions: read # only required for a private repository by github/codeql-action/upload-sarif to get the Action run status |
Contributor
There was a problem hiding this comment.
1. Overprivileged third-party action 🐞 Bug ⛨ Security
The job grants security-events: write at job scope, so apisec-inc/ethicalcheck-action runs with a token able to publish/alter code-scanning (SARIF) security events. If that third-party action is compromised, it can write arbitrary security alerts/results to the repo without further review.
Agent Prompt
### Issue description
`.github/workflows/ethicalcheck.yml` grants `security-events: write` at the **job** level, which unintentionally gives the same write-capable `GITHUB_TOKEN` to the third-party `apisec-inc/ethicalcheck-action` step.
### Issue Context
The permission is only needed for the later `github/codeql-action/upload-sarif` step, not for generating the SARIF.
### Fix Focus Areas
- .github/workflows/ethicalcheck.yml[45-68]
### Suggested fix
1. Split into two jobs:
- **scan job**: runs `apisec-inc/ethicalcheck-action` with minimal permissions (e.g., `contents: read` only), outputs `ethicalcheck-results.sarif`, and uploads it as an artifact.
- **upload job**: depends on scan job, downloads the artifact, and runs `github/codeql-action/upload-sarif` with `permissions: { security-events: write }`.
2. Keep the workflow-level permissions minimal (as the repo does elsewhere).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+59
to
+62
| # The OpenAPI Specification URL or Swagger Path or Public Postman collection URL. | ||
| oas-url: "http://netbanking.apisec.ai:8080/v2/api-docs" | ||
| # The email address to which the penetration test report will be sent. | ||
| email: "xxx@apisec.ai" |
Contributor
There was a problem hiding this comment.
2. Plaintext http scan url 🐞 Bug ⛨ Security
The workflow fetches the OpenAPI spec from a plaintext http:// URL, which allows interception or tampering of the spec in transit and can change what the scan tests. This weakens the integrity/confidentiality of the security scan inputs and results.
Agent Prompt
### Issue description
`oas-url` is configured with an `http://` endpoint, which is not transport-secure.
### Issue Context
This workflow runs on push/PR to `develop` and on a schedule, so the scan input is repeatedly fetched over plaintext.
### Fix Focus Areas
- .github/workflows/ethicalcheck.yml[59-63]
### Suggested fix
- Change `oas-url` to an `https://` endpoint.
- Make `oas-url` (and `email`) configurable via repo variables/secrets (e.g., `${{ vars.ETHICALCHECK_OAS_URL }}`, `${{ secrets.ETHICALCHECK_EMAIL }}`) so production targets/recipients aren’t hardcoded in the workflow file.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+65
to
+66
| - name: Upload sarif file to repository | ||
| uses: github/codeql-action/upload-sarif@v3 |
Contributor
There was a problem hiding this comment.
3. Unpinned upload-sarif action 🐞 Bug ⛨ Security
The workflow uses github/codeql-action/upload-sarif@v3 (a floating tag) instead of pinning to a commit SHA, reducing supply-chain integrity compared to the repo’s existing SHA-pinning practice. This can change behavior without any workflow diff.
Agent Prompt
### Issue description
`github/codeql-action/upload-sarif` is referenced via `@v3` instead of a commit SHA.
### Issue Context
Other workflows in this repo pin actions to commit SHAs for reproducibility and supply-chain hardening.
### Fix Focus Areas
- .github/workflows/ethicalcheck.yml[65-68]
- .github/workflows/ci.yml[50-61]
### Suggested fix
- Replace `github/codeql-action/upload-sarif@v3` with a full commit SHA (optionally keep a comment indicating the version), consistent with the rest of the repository workflows.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+60
to
+62
| oas-url: "http://netbanking.apisec.ai:8080/v2/api-docs" | ||
| # The email address to which the penetration test report will be sent. | ||
| email: "xxx@apisec.ai" |
Contributor
There was a problem hiding this comment.
1. Hardcoded oas-url and email 📜 Skill insight ≡ Correctness
The workflow hardcodes oas-url and email values (including template/sample endpoints and a placeholder external recipient) instead of sourcing them from configurable GitHub Actions vars/secrets. This makes environment changes/rotation harder and can cause normal scheduled/push/PR runs to scan the wrong target and potentially disclose penetration-test reports outside the project.
Agent Prompt
## Issue description
`.github/workflows/ethicalcheck.yml` hardcodes configurable values (`oas-url` and `email`), including template/sample values, rather than sourcing them from GitHub Actions configuration. This violates compliance requirements and risks running scans against the wrong endpoint and sending penetration-test reports to an unintended/external recipient during normal scheduled and push/PR runs.
## Issue Context
- PR Compliance ID 2634353 forbids hardcoding configurable values.
- The EthicalCheck step uses `email` as the address where the penetration-test report will be sent.
- The workflow triggers automatically on push/PR to `develop` and on a weekly cron, so once merged these hard-coded values will be exercised routinely.
- These values are environment/deployment-specific and should be changeable without code edits via GitHub Actions `vars`/`secrets` (and optionally `workflow_dispatch` inputs), and it’s advisable to fail fast if values are unset/placeholder to prevent accidental external scans/reporting.
## Fix Focus Areas
- .github/workflows/ethicalcheck.yml[32-44]
- .github/workflows/ethicalcheck.yml[56-63]
- .github/workflows/ethicalcheck.yml[58-63]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+55
to
+57
| steps: | ||
| - name: EthicalCheck Free & Automated API Security Testing Service | ||
| uses: apisec-inc/ethicalcheck-action@005fac321dd843682b1af6b72f30caaf9952c641 |
Contributor
There was a problem hiding this comment.
2. Yamllint rule violations 🐞 Bug ⚙ Maintainability
The new workflow likely violates the repository’s yamllint rules (missing YAML document start marker and non-2-space sequence indentation), which can trigger lint failures where yamllint is enforced. This also makes the workflow inconsistent with other repo workflows that follow the established style.
Agent Prompt
## Issue description
The repository includes a `.yamllint.yml` that requires a YAML document start marker and 2-space indentation. The new workflow does not include `---` before the first non-comment line and uses inconsistent indentation for the `steps` sequence.
## Issue Context
Other workflows in this repo commonly include `---` and/or yamllint suppression for `on:` and use consistent 2-space indentation.
## Fix Focus Areas
- .github/workflows/ethicalcheck.yml[1-4]
- .github/workflows/ethicalcheck.yml[29-40]
- .github/workflows/ethicalcheck.yml[55-68]
- .yamllint.yml[19-23]
## Suggested fix
- Add `---` at the top (before the first non-comment key).
- Normalize indentation to 2 spaces throughout (especially list items under `steps:`).
- Consider matching existing workflow conventions for `on:` (e.g., quoting `"on":` or adding `# yamllint disable-line rule:truthy`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
|
Code review by qodo was updated up to the latest commit e820c15 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 1 file
Confidence score: 2/5
- In
.github/workflows/ethicalcheck.yml, the EthicalCheck scan is pointed at a hardcoded third-party demooas-url(netbanking.apisec.ai) that does not match this repository, so the job can produce misleading “pass/fail” signals and no real coverage of your codebase — pointoas-urlto this repo’s actual API spec/source of truth or gate the job until one exists. - In
.github/workflows/ethicalcheck.yml, the report destination is still the template placeholder (xxx@apisec.ai), which means security findings may be sent to the wrong party or never reach maintainers, reducing incident response value — replace it with the correct monitored security mailbox/team channel. - In
.github/workflows/ethicalcheck.yml,security-events: writeis granted for the whole job, including the third-partyapisec-inc/ethicalcheck-actionstep, increasing blast radius if that step is compromised — scopesecurity-events: writeto only the SARIF upload step (or split steps into separate jobs with least privilege). - In
.github/workflows/ethicalcheck.yml,github/codeql-action/upload-sarif@v3is pinned to a mutable major tag, which can change behavior unexpectedly and weaken supply-chain reproducibility — pin to a full commit SHA like the EthicalCheck action.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ethicalcheck.yml">
<violation number="1" location=".github/workflows/ethicalcheck.yml:51">
P2: The security-events: write permission is granted at the job level, so it also applies to the third-party apisec-inc/ethicalcheck-action step, not just the later upload-sarif step that actually needs it. If that action is compromised, it could write arbitrary code-scanning alerts to the repo. Consider splitting into two jobs (scan + upload) so the write permission is scoped only to the trusted upload-sarif step.</violation>
<violation number="2" location=".github/workflows/ethicalcheck.yml:60">
P1: The `oas-url` points to a hardcoded third-party demo endpoint (`netbanking.apisec.ai`), not to this repository's own API. This repo is the MCB Rust/MCP server and contains no OpenAPI/Swagger spec, so this workflow will scan an unrelated external service rather than any API in this project. That makes every scheduled/push run produce meaningless results and send security-test traffic against an outside host you don't control. This looks like leftover starter-template content. Point the value at this repo's real OpenAPI spec (or a public Postman collection) once such an API exists, and prefer an `https://` URL.</violation>
<violation number="3" location=".github/workflows/ethicalcheck.yml:62">
P2: The penetration-test report is delivered to the placeholder address `xxx@apisec.ai`, which is the generic example value from the starter template. The security report generated by this job will be sent to / displayed for an invalid mailbox instead of the responsible engineer, so no one receives the findings. Wire this up to a real recipient, ideally via a GitHub secret (e.g. `secrets.ETHICALCHECK_REPORT_EMAIL`) rather than baking the address into the workflow.</violation>
<violation number="4" location=".github/workflows/ethicalcheck.yml:66">
P2: The upload-sarif action is referenced by the floating major-version tag `@v3`, unlike the EthicalCheck action above which is pinned to a full commit SHA (`005fac321dd843682b1af6b72f30caaf9952c641`). A mutable tag can change behavior (or be compromised) across runs, breaking reproducibility and opening a supply-chain risk for a security action that processes untrusted SARIF. Pin it to a SHA for consistency with the pinning approach already used in this file.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| uses: apisec-inc/ethicalcheck-action@005fac321dd843682b1af6b72f30caaf9952c641 | ||
| with: | ||
| # The OpenAPI Specification URL or Swagger Path or Public Postman collection URL. | ||
| oas-url: "http://netbanking.apisec.ai:8080/v2/api-docs" |
There was a problem hiding this comment.
P1: The oas-url points to a hardcoded third-party demo endpoint (netbanking.apisec.ai), not to this repository's own API. This repo is the MCB Rust/MCP server and contains no OpenAPI/Swagger spec, so this workflow will scan an unrelated external service rather than any API in this project. That makes every scheduled/push run produce meaningless results and send security-test traffic against an outside host you don't control. This looks like leftover starter-template content. Point the value at this repo's real OpenAPI spec (or a public Postman collection) once such an API exists, and prefer an https:// URL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ethicalcheck.yml, line 60:
<comment>The `oas-url` points to a hardcoded third-party demo endpoint (`netbanking.apisec.ai`), not to this repository's own API. This repo is the MCB Rust/MCP server and contains no OpenAPI/Swagger spec, so this workflow will scan an unrelated external service rather than any API in this project. That makes every scheduled/push run produce meaningless results and send security-test traffic against an outside host you don't control. This looks like leftover starter-template content. Point the value at this repo's real OpenAPI spec (or a public Postman collection) once such an API exists, and prefer an `https://` URL.</comment>
<file context>
@@ -0,0 +1,69 @@
+ uses: apisec-inc/ethicalcheck-action@005fac321dd843682b1af6b72f30caaf9952c641
+ with:
+ # The OpenAPI Specification URL or Swagger Path or Public Postman collection URL.
+ oas-url: "http://netbanking.apisec.ai:8080/v2/api-docs"
+ # The email address to which the penetration test report will be sent.
+ email: "xxx@apisec.ai"
</file context>
| sarif-result-file: "ethicalcheck-results.sarif" | ||
|
|
||
| - name: Upload sarif file to repository | ||
| uses: github/codeql-action/upload-sarif@v3 |
There was a problem hiding this comment.
P2: The upload-sarif action is referenced by the floating major-version tag @v3, unlike the EthicalCheck action above which is pinned to a full commit SHA (005fac321dd843682b1af6b72f30caaf9952c641). A mutable tag can change behavior (or be compromised) across runs, breaking reproducibility and opening a supply-chain risk for a security action that processes untrusted SARIF. Pin it to a SHA for consistency with the pinning approach already used in this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ethicalcheck.yml, line 66:
<comment>The upload-sarif action is referenced by the floating major-version tag `@v3`, unlike the EthicalCheck action above which is pinned to a full commit SHA (`005fac321dd843682b1af6b72f30caaf9952c641`). A mutable tag can change behavior (or be compromised) across runs, breaking reproducibility and opening a supply-chain risk for a security action that processes untrusted SARIF. Pin it to a SHA for consistency with the pinning approach already used in this file.</comment>
<file context>
@@ -0,0 +1,69 @@
+ sarif-result-file: "ethicalcheck-results.sarif"
+
+ - name: Upload sarif file to repository
+ uses: github/codeql-action/upload-sarif@v3
+ with:
+ sarif_file: ./ethicalcheck-results.sarif
</file context>
| # The OpenAPI Specification URL or Swagger Path or Public Postman collection URL. | ||
| oas-url: "http://netbanking.apisec.ai:8080/v2/api-docs" | ||
| # The email address to which the penetration test report will be sent. | ||
| email: "xxx@apisec.ai" |
There was a problem hiding this comment.
P2: The penetration-test report is delivered to the placeholder address xxx@apisec.ai, which is the generic example value from the starter template. The security report generated by this job will be sent to / displayed for an invalid mailbox instead of the responsible engineer, so no one receives the findings. Wire this up to a real recipient, ideally via a GitHub secret (e.g. secrets.ETHICALCHECK_REPORT_EMAIL) rather than baking the address into the workflow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ethicalcheck.yml, line 62:
<comment>The penetration-test report is delivered to the placeholder address `xxx@apisec.ai`, which is the generic example value from the starter template. The security report generated by this job will be sent to / displayed for an invalid mailbox instead of the responsible engineer, so no one receives the findings. Wire this up to a real recipient, ideally via a GitHub secret (e.g. `secrets.ETHICALCHECK_REPORT_EMAIL`) rather than baking the address into the workflow.</comment>
<file context>
@@ -0,0 +1,69 @@
+ # The OpenAPI Specification URL or Swagger Path or Public Postman collection URL.
+ oas-url: "http://netbanking.apisec.ai:8080/v2/api-docs"
+ # The email address to which the penetration test report will be sent.
+ email: "xxx@apisec.ai"
+ sarif-result-file: "ethicalcheck-results.sarif"
+
</file context>
Suggested change
| email: "xxx@apisec.ai" | |
| email: ${{ secrets.ETHICALCHECK_REPORT_EMAIL }} |
| jobs: | ||
| Trigger_EthicalCheck: | ||
| permissions: | ||
| security-events: write # for github/codeql-action/upload-sarif to upload SARIF results |
There was a problem hiding this comment.
P2: The security-events: write permission is granted at the job level, so it also applies to the third-party apisec-inc/ethicalcheck-action step, not just the later upload-sarif step that actually needs it. If that action is compromised, it could write arbitrary code-scanning alerts to the repo. Consider splitting into two jobs (scan + upload) so the write permission is scoped only to the trusted upload-sarif step.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ethicalcheck.yml, line 51:
<comment>The security-events: write permission is granted at the job level, so it also applies to the third-party apisec-inc/ethicalcheck-action step, not just the later upload-sarif step that actually needs it. If that action is compromised, it could write arbitrary code-scanning alerts to the repo. Consider splitting into two jobs (scan + upload) so the write permission is scoped only to the trusted upload-sarif step.</comment>
<file context>
@@ -0,0 +1,69 @@
+jobs:
+ Trigger_EthicalCheck:
+ permissions:
+ security-events: write # for github/codeql-action/upload-sarif to upload SARIF results
+ actions: read # only required for a private repository by github/codeql-action/upload-sarif to get the Action run status
+ runs-on: ubuntu-latest
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Adds a GitHub Actions workflow to run EthicalCheck API security tests for our API. Helps catch OWASP-style issues early with automated checks and reports.
New Features
apisec-inc/ethicalcheck-actionagainst the provided OpenAPI spec.develop, weekly cron, and manual dispatch.ethicalcheck-results.sarifviagithub/codeql-action/upload-sarifto Code Scanning.Migration
oas-urlto your API’s OpenAPI/Swagger or public Postman collection URL.emailto a valid recipient for the test report.Written for commit e820c15. Summary will update on new commits.