feat: add megalinter to CI workflow#12
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 21 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces security hardening and operational monitoring across the infrastructure. Changes include enabling security scanning via MegaLinter in the CI pipeline, configuring Docker containers to run as non-root users with dropped Linux capabilities, adding health checks to Docker services, and disabling specific linters in the MegaLinter configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~13 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
| USER app | ||
| WORKDIR /app | ||
| COPY --from=publish /app/publish . | ||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 CMD curl -f http://localhost:8080/diagnostics/health || exit 1 |
There was a problem hiding this comment.
I'm not sure we shoul be tying our healthchecks to localhost - this would work in CI and locally, but how would this work in production?
| USER app | ||
| WORKDIR /app | ||
| COPY --from=publish /app/publish . | ||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 CMD curl -f http://localhost:8080/diagnostics/health || exit 1 |
There was a problem hiding this comment.
Same comment here about healthchecks and localhost
| cap_drop: | ||
| - ALL | ||
| healthcheck: | ||
| test: ["CMD", "curl", "-f", "http://localhost:8080/diagnostics/health"] |
There was a problem hiding this comment.
This compose file is only really intended for local dev, so it's probably fine that we use localhost in healthcheck here
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
.github/workflows/ci.yml (1)
9-13: Tighten workflow-level permissions to least privilegeLine 9 through Line 13 grant
issues: readandpull-requests: readglobally, but the shown jobs do not appear to need them. Keep only required scopes at workflow level and elevate per-job when necessary.Proposed fix
permissions: contents: read - issues: read - pull-requests: read🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 9 - 13, The workflow-level permissions currently grant unnecessary scopes (permissions: issues: read and pull-requests: read); remove those from the top-level permissions block so only the required narrow scope (e.g., contents: read) remains, and if any specific job needs issue or PR access add a per-job permissions block with issues: read or pull-requests: read inside that job; update references to the top-level permissions keys (permissions, contents, issues, pull-requests) and ensure no job relies on the removed global scopes without adding per-job elevation.ToyDb/Dockerfile (1)
29-29:USER appis duplicated in final stage.Line 29 is redundant because
FROM base AS finalalready inheritsUSER appfrom Line 5.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ToyDb/Dockerfile` at line 29, The final stage contains a redundant "USER app" because "FROM base AS final" already inherits the USER set in the base stage; remove the duplicate "USER app" statement in the final stage (the second occurrence) so only the original USER set in the base stage remains, leaving the rest of the final stage (FROM base AS final and its commands) unchanged.docker-compose.yml (1)
27-122: Consider YAML anchors to remove repeated service policy blocks.The same
security_opt,cap_drop, andhealthcheckblocks are duplicated across all ToyDb replicas. A shared anchor/alias would cut repetition and reduce drift risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 27 - 122, The compose file repeats identical security_opt, cap_drop and healthcheck blocks across the ToyDb services (toydb-p1-r1, toydb-p1-r2, toydb-p2-r1, toydb-p2-r2, toydb-p1-r3, toydb-p2-r3); refactor by defining a YAML anchor (e.g., &toydb_defaults) containing security_opt, cap_drop and healthcheck and then reference it in each service with the alias (*toydb_defaults) or merge key (<<: *toydb_defaults) to remove duplication while preserving the existing settings.ToyDbRouting/Dockerfile (1)
28-28: Remove redundantUSER appin final stage.Line 28 repeats
USER appalready set inbase(Line 5), so it can be dropped for clarity.Diff
FROM base AS final ARG HEALTHCHECK_URL=http://localhost:8080/diagnostics/health -USER app WORKDIR /app COPY --from=publish /app/publish . HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 CMD curl -f ${HEALTHCHECK_URL} || exit 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ToyDbRouting/Dockerfile` at line 28, Remove the redundant USER app declaration in the final Dockerfile stage: locate the final stage where "USER app" is repeated (the second occurrence) and delete that line because the base stage (USER app in the base stage) already sets the user; ensure no other instructions rely on redefining the user in the final stage and keep only the original USER app declaration in the base stage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Line 27: Replace the mutable tag in the workflow step that currently reads
"uses: oxsecurity/megalinter/flavors/security@v7.13.0" with the fixed commit SHA
to pin the action; update that string to use the commit SHA
b48455a119cc28045eee8f1e9d0a542a85e71f4f (i.e., "uses:
oxsecurity/megalinter/flavors/security@b48455a1...") so the CI references the
exact commit instead of the mutable v7.13.0 tag.
- Line 31: Remove the insecure environment variable by deleting the
DISABLE_HTTPS_CERTIFICATE_VALIDATION: "true" entry from the MegaLinter/CI step
configuration so TLS certificate validation is not disabled; locate the CI
workflow's MegaLinter/env block where DISABLE_HTTPS_CERTIFICATE_VALIDATION is
defined and remove that key entirely (or comment it out) and commit the change,
ensuring no other steps depend on this flag.
In @.mega-linter.yml:
- Around line 1-7: Remove the global disable entries in DISABLE_LINTERS and stop
overwriting exclusions via OVERWRITE_EXCLUSIONS; instead create per-linter
config files and add targeted suppressions: add a .checkov.yml with skip-check
entries for REPOSITORY_CHECKOV, add a .grype.yaml with ignore rules for
REPOSITORY_GRYPE, and add a .devskim.json with rule/glob suppressions for
REPOSITORY_DEVSKIM; then delete the DISABLE_LINTERS list (or remove those
specific entries) and remove the OVERWRITE_EXCLUSIONS: "" line from
.mega-linter.yml so MegaLinter runs but respects the new per-linter configs.
In `@docker-compose.yml`:
- Around line 21-26: The healthcheck in docker-compose for the toydb-routing
service calls /diagnostics/health which is only registered when
app.Environment.IsDevelopment() in ToyDbRouting/Program.cs; add the
ASPNETCORE_ENVIRONMENT=Development environment variable to the toydb-routing
service in docker-compose so the endpoint is available (i.e., add an environment
section with ASPNETCORE_ENVIRONMENT set to Development for the toydb-routing
service).
- Around line 37-42: The healthchecks in docker-compose point to a non-existent
HTTP endpoint (/diagnostics/health) while the app only registers a gRPC health
service via MapGrpcHealthChecksService in Program.cs; fix by either adding an
HTTP health probe endpoint in the service (e.g., register an HTTP route or
MapHealthChecks/MapGet that returns 200 for "/diagnostics/health" alongside
MapGrpcHealthChecksService) or by changing the docker-compose healthcheck tests
to use a gRPC health probe (use a grpc health probe binary or grpcurl against
the gRPC health service) and update the healthcheck test commands for all ToyDb
service entries accordingly.
In `@ToyDb/Dockerfile`:
- Around line 28-32: The Dockerfile HEALTHCHECK is probing an HTTP URL that the
app doesn't expose; update either the app or the Dockerfile: either add an HTTP
health endpoint in Program.cs (e.g., MapGet("/diagnostics/health", () =>
Results.Ok()) or map an HTTP health check that mirrors
MapGrpcHealthChecksService) so the existing HEALTHCHECK URL works, or replace
the Dockerfile HEALTHCHECK with a gRPC-aware probe (e.g., install and invoke
grpc_health_probe or use grpcurl against the gRPC endpoint) and update
HEALTHCHECK/CMD accordingly (referencing HEALTHCHECK, HEALTHCHECK_URL, and
MapGrpcHealthChecksService to locate the relevant code).
In `@ToyDbRouting/Dockerfile`:
- Around line 27-31: The Dockerfile HEALTHCHECK currently probes
/diagnostics/health which is only registered inside the development-only branch
in Program.cs (the app.Environment.IsDevelopment() block), causing health checks
to fail in production; fix by either moving the health endpoint registration out
of the development-only branch (register the diagnostics/health endpoint
unconditionally in Program.cs or add a production-safe endpoint like /health) or
change the Dockerfile HEALTHCHECK to probe an endpoint that exists in
production, and update the Dockerfile HEALTHCHECK (the HEALTHCHECK --interval...
CMD line) or the endpoint registration in Program.cs accordingly so the probed
path exists in non-development environments.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 9-13: The workflow-level permissions currently grant unnecessary
scopes (permissions: issues: read and pull-requests: read); remove those from
the top-level permissions block so only the required narrow scope (e.g.,
contents: read) remains, and if any specific job needs issue or PR access add a
per-job permissions block with issues: read or pull-requests: read inside that
job; update references to the top-level permissions keys (permissions, contents,
issues, pull-requests) and ensure no job relies on the removed global scopes
without adding per-job elevation.
In `@docker-compose.yml`:
- Around line 27-122: The compose file repeats identical security_opt, cap_drop
and healthcheck blocks across the ToyDb services (toydb-p1-r1, toydb-p1-r2,
toydb-p2-r1, toydb-p2-r2, toydb-p1-r3, toydb-p2-r3); refactor by defining a YAML
anchor (e.g., &toydb_defaults) containing security_opt, cap_drop and healthcheck
and then reference it in each service with the alias (*toydb_defaults) or merge
key (<<: *toydb_defaults) to remove duplication while preserving the existing
settings.
In `@ToyDb/Dockerfile`:
- Line 29: The final stage contains a redundant "USER app" because "FROM base AS
final" already inherits the USER set in the base stage; remove the duplicate
"USER app" statement in the final stage (the second occurrence) so only the
original USER set in the base stage remains, leaving the rest of the final stage
(FROM base AS final and its commands) unchanged.
In `@ToyDbRouting/Dockerfile`:
- Line 28: Remove the redundant USER app declaration in the final Dockerfile
stage: locate the final stage where "USER app" is repeated (the second
occurrence) and delete that line because the base stage (USER app in the base
stage) already sets the user; ensure no other instructions rely on redefining
the user in the final stage and keep only the original USER app declaration in
the base stage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b2c3c9b-04ff-42cb-9723-e2501fea3372
📒 Files selected for processing (5)
.github/workflows/ci.yml.mega-linter.ymlToyDb/DockerfileToyDbRouting/Dockerfiledocker-compose.yml
| DISABLE_LINTERS: | ||
| - REPOSITORY_DEVSKIM | ||
| - REPOSITORY_GRYPE | ||
| - REPOSITORY_CHECKOV | ||
| - REPOSITORY_KICS | ||
|
|
||
| OVERWRITE_EXCLUSIONS: "" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
For MegaLinter v7 security flavor, what is the recommended way to handle false positives for REPOSITORY_CHECKOV, REPOSITORY_GRYPE, and REPOSITORY_DEVSKIM: disable entire linters or configure targeted exclusions/rule suppressions?
💡 Result:
For MegaLinter v7 security flavor, the recommended way to handle false positives for REPOSITORY_CHECKOV, REPOSITORY_GRYPE, and REPOSITORY_DEVSKIM is to configure targeted exclusions and rule suppressions using each linter's native configuration files, rather than disabling the entire linters. These repository-level linters operate in 'project' CLI lint mode, scanning the entire repository root, so MegaLinter's general file filters (like FILTER_REGEX_EXCLUDE) do not apply—filtering and suppression must be done via the linters' own mechanisms. - REPOSITORY_CHECKOV: Use a .checkov.yml file at the repository root to skip specific checks, define exclusions, or configure rules (e.g., skip-check: [CKV_AWS_123]). MegaLinter looks for REPOSITORY_CHECKOV_CONFIG_FILE (.checkov.yml) in the workspace or uses a default. - REPOSITORY_GRYPE: Use a .grype.yaml (or .grype.yml) file to ignore specific vulnerabilities or paths (e.g., ignore: vulnerability: CVE-XXXX-XXXX). Configured via REPOSITORY_GRYPE_CONFIG_FILE. - REPOSITORY_DEVSKIM: Use a .devskim.json file for Glob exclusions (e.g., Glob: ["/node_modules/"]) or rule suppression. Supports --ignore-rules-ids via arguments, but config file is preferred. Configured via REPOSITORY_DEVSKIM_CONFIG_FILE. Disabling entire linters via DISABLE_LINTERS is possible but not recommended for false positives, as it eliminates all scanning. Official MegaLinter docs emphasize linter-specific config files for project-mode linters, with explicit notes that "filtering can not be done using MegaLinter configuration variables, it must be done using [linter] configuration."
Citations:
- 1: https://megalinter.io/7/descriptors/repository_checkov/
- 2: https://megalinter.io/7.4.0/descriptors/repository_checkov/
- 3: https://megalinter.io/v8/descriptors/repository_checkov/
- 4: https://megalinter.io/7.6.0/descriptors/repository_grype/
- 5: https://megalinter.io/7.4.0/descriptors/repository_grype/
- 6: https://megalinter.io/8/descriptors/repository_grype/
- 7: https://megalinter.io/v7/descriptors/repository_devskim/
- 8: https://megalinter.io/v7/flavors/security/
- 9: https://megalinter.io/7.8.0/configuration/
- 10: https://megalinter.io/8.1.0/configuration/
- 11: https://megalinter.io/v8/config-filtering/
Use linter-specific configuration files instead of global disables
Lines 1–7 disable multiple security scanners globally, which contradicts the PR objective of adding MegaLinter security checks. Per MegaLinter v7 documentation, the recommended approach for handling false positives is to configure targeted suppressions in each linter's native configuration file:
- REPOSITORY_CHECKOV: Use
.checkov.ymlwithskip-checkentries - REPOSITORY_GRYPE: Use
.grype.yamlwithignoreentries for specific vulnerabilities - REPOSITORY_DEVSKIM: Use
.devskim.jsonfor Glob exclusions and rule suppression
Remove global linter disables and create these configuration files at the repository root instead. Also remove OVERWRITE_EXCLUSIONS: "" unless explicitly required by your setup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.mega-linter.yml around lines 1 - 7, Remove the global disable entries in
DISABLE_LINTERS and stop overwriting exclusions via OVERWRITE_EXCLUSIONS;
instead create per-linter config files and add targeted suppressions: add a
.checkov.yml with skip-check entries for REPOSITORY_CHECKOV, add a .grype.yaml
with ignore rules for REPOSITORY_GRYPE, and add a .devskim.json with rule/glob
suppressions for REPOSITORY_DEVSKIM; then delete the DISABLE_LINTERS list (or
remove those specific entries) and remove the OVERWRITE_EXCLUSIONS: "" line from
.mega-linter.yml so MegaLinter runs but respects the new per-linter configs.
| healthcheck: | ||
| test: ["CMD", "curl", "-f", "http://localhost:8080/diagnostics/health"] | ||
| interval: 30s | ||
| timeout: 3s | ||
| retries: 3 | ||
| start_period: 10s |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm ToyDb does not expose /diagnostics/health and uses gRPC health checks
rg -n -C3 'MapGet\("/diagnostics/health"|MapGrpcHealthChecksService\(' ToyDb/Program.csRepository: ryanachten/ToyDb
Length of output: 210
🏁 Script executed:
#!/bin/bash
# Search for all health-related endpoint mappings in Program.cs
echo "=== All health-related mappings in ToyDb/Program.cs ==="
rg -n -i 'health|MapGet|MapPost|MapPut|Map' ToyDb/Program.cs | head -30
echo -e "\n=== Healthcheck lines in docker-compose.yml ==="
rg -n 'healthcheck:|test:' docker-compose.yml -A 1Repository: ryanachten/ToyDb
Length of output: 1331
ToyDb healthchecks target a non-existent HTTP route.
At lines 22, 38, 54, 70, 86, 102, 118, all ToyDb services probe /diagnostics/health via HTTP curl, but ToyDb/Program.cs exposes only a gRPC health service (MapGrpcHealthChecksService at line 20). The /diagnostics/health HTTP endpoint does not exist, causing all healthchecks to fail. Either add an HTTP health endpoint or reconfigure the docker-compose probes to use gRPC health checks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 37 - 42, The healthchecks in docker-compose
point to a non-existent HTTP endpoint (/diagnostics/health) while the app only
registers a gRPC health service via MapGrpcHealthChecksService in Program.cs;
fix by either adding an HTTP health probe endpoint in the service (e.g.,
register an HTTP route or MapHealthChecks/MapGet that returns 200 for
"/diagnostics/health" alongside MapGrpcHealthChecksService) or by changing the
docker-compose healthcheck tests to use a gRPC health probe (use a grpc health
probe binary or grpcurl against the gRPC health service) and update the
healthcheck test commands for all ToyDb service entries accordingly.
- Add megalinter security job to CI workflow - Pin action to commit SHA for security - Disable KICS, DEVSKIM, TRIVY, GRYPE, SEMGREP due to CI compatibility issues - Add security_opt, cap_drop, healthcheck to docker-compose - Add ASPNETCORE_ENVIRONMENT=Development to compose services - Remove HEALTHCHECK from Dockerfiles (configure at orchestration level)
8031616 to
0810e11
Compare
1 participant
Summary
Summary by CodeRabbit
New Features
Chores