Implement command to detect non-compliant project levels and update score calculation#3340
Implement command to detect non-compliant project levels and update score calculation#3340OM-JADHAV25 wants to merge 11 commits intoOWASP:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughAdds a boolean flag to ProjectHealthMetrics, a utility and command to fetch official OWASP project levels and mark non‑compliant projects, applies a 10.0 penalty to health scores for flagged projects, and invokes compliance + scoring commands after project aggregation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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: 2
🤖 Fix all issues with AI agents
In
@backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py:
- Around line 80-86: The warning string built in the
self.stdout.write(self.style.WARNING(...)) block concatenates adjacent f-strings
without spaces; update the f-strings in that block (the f"Level mismatch:
{project.name}", f"local={project.level}", f"official={expected_level}" pieces)
so they include separators or merge into one f-string (e.g., include spaces or
commas between parts: "Level mismatch: {project.name} local={project.level}
official={expected_level}") to ensure readable output.
- Around line 90-94: The call to ProjectHealthMetrics.bulk_save uses the wrong
keyword; replace the unexpected keyword update_fields with the correct parameter
name fields so the call becomes ProjectHealthMetrics.bulk_save(updated_metrics,
fields=["level_non_compliant"]); keep the same updated_metrics variable and
ensure only the "level_non_compliant" field is passed to fields.
🧹 Nitpick comments (7)
backend/apps/owasp/utils/project_level.py (1)
8-15: LGTM! Consider consistent Decimal construction style.The mapping logic is correct. Minor style note: you're mixing
Decimal(4)(int) withDecimal("3.5")(string). While both work, using string literals consistently (e.g.,Decimal("4")) is the idiomatic approach for Decimal to avoid potential floating-point precision issues in other contexts.backend/apps/owasp/models/project_health_metrics.py (1)
46-46: Consider naming consistency.The new field
level_non_compliantuses negative phrasing, while similar fields (is_funding_requirements_compliant,is_leader_requirements_compliant) use positive phrasing. Consider renaming tois_level_compliantwith inverted logic for consistency, or alternativelyis_level_non_compliantto match theis_prefix convention.That said, the current name clearly conveys the field's purpose and default=False is correct.
backend/tests/apps/owasp/management/commands/owasp_update_project_health_scores_test.py (1)
87-131: Test covers the penalty correctly. Consider adding a clamping edge case.The test validates that a 10-point penalty is applied when
level_non_compliant = True. The logic and assertions are correct.Consider adding a test case where the base score is less than 10 (e.g., 5.0) to verify the score clamps to 0.0 rather than going negative, if the implementation includes such clamping logic.
backend/apps/owasp/management/commands/owasp_aggregate_projects.py (1)
118-124: Post-processing logic is correct. Consider error handling.The conditional execution when
offset == 0correctly ensures post-processing only runs for full syncs. The command order is correct: level compliance must run before health scores since the penalty calculation depends on thelevel_non_compliantflag.Using
self.stdout.writewithself.style.NOTICEfollows Django conventions per learnings.Consider whether a failure in
owasp_update_project_level_complianceshould preventowasp_update_project_health_scoresfrom running. If independent execution is preferred, wrap eachcall_commandin a try/except with appropriate logging.backend/tests/apps/owasp/utils/project_level_test.py (1)
9-27: Consider adding a test for unmapped levels.The tests cover known mappings and negative values, but there's no test verifying that an unmapped positive value (e.g.,
Decimal("2.5")orDecimal(5)) returnsNone. This edge case would ensuremap_levelbehaves correctly for unexpected official level values.💡 Suggested test case
def test_unmapped_level_returns_none(self): assert map_level(Decimal("2.5")) is None assert map_level(Decimal(5)) is Nonebackend/tests/apps/owasp/management/commands/owasp_update_project_level_compliance_test.py (1)
19-35: Consider adding error handling tests.The command uses
response.raise_for_status()which can raiserequests.HTTPErroron non-2xx responses, but there's no test verifying error handling (e.g., network failure, HTTP 500, invalid JSON). Consider adding tests for these failure modes to ensure the command fails gracefully.💡 Example test for HTTP error
@patch("apps.owasp.management.commands.owasp_update_project_level_compliance.requests.get") def test_handles_http_error(self, mock_get): mock_get.return_value.raise_for_status.side_effect = requests.HTTPError("500 Server Error") with self.assertRaises(requests.HTTPError): call_command("owasp_update_project_level_compliance")backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py (1)
32-33: Consider adding retry logic for transient network failures.The HTTP request has a timeout (good), but transient failures could cause the entire compliance check to fail. For a scheduled job, consider adding retry logic or catching and logging the exception to allow graceful degradation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/apps/owasp/management/commands/owasp_aggregate_projects.pybackend/apps/owasp/management/commands/owasp_update_project_health_scores.pybackend/apps/owasp/management/commands/owasp_update_project_level_compliance.pybackend/apps/owasp/migrations/0070_projecthealthmetrics_level_non_compliant.pybackend/apps/owasp/models/project_health_metrics.pybackend/apps/owasp/utils/project_level.pybackend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.pybackend/tests/apps/owasp/management/commands/owasp_update_project_health_scores_test.pybackend/tests/apps/owasp/management/commands/owasp_update_project_level_compliance_test.pybackend/tests/apps/owasp/utils/project_level_test.py
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_update_project_health_scores.pybackend/apps/owasp/management/commands/owasp_aggregate_projects.pybackend/tests/apps/owasp/management/commands/owasp_update_project_health_scores_test.pybackend/apps/owasp/management/commands/owasp_update_project_level_compliance.pybackend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.pybackend/tests/apps/owasp/management/commands/owasp_update_project_level_compliance_test.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/owasp/management/commands/owasp_update_project_health_scores.pybackend/apps/owasp/management/commands/owasp_aggregate_projects.pybackend/tests/apps/owasp/management/commands/owasp_update_project_health_scores_test.pybackend/apps/owasp/management/commands/owasp_update_project_level_compliance.pybackend/apps/owasp/utils/project_level.pybackend/apps/owasp/migrations/0070_projecthealthmetrics_level_non_compliant.pybackend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.pybackend/apps/owasp/models/project_health_metrics.pybackend/tests/apps/owasp/management/commands/owasp_update_project_level_compliance_test.pybackend/tests/apps/owasp/utils/project_level_test.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/owasp/management/commands/owasp_update_project_health_scores.pybackend/apps/owasp/management/commands/owasp_aggregate_projects.pybackend/tests/apps/owasp/management/commands/owasp_update_project_health_scores_test.pybackend/apps/owasp/management/commands/owasp_update_project_level_compliance.pybackend/apps/owasp/utils/project_level.pybackend/apps/owasp/migrations/0070_projecthealthmetrics_level_non_compliant.pybackend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.pybackend/apps/owasp/models/project_health_metrics.pybackend/tests/apps/owasp/management/commands/owasp_update_project_level_compliance_test.pybackend/tests/apps/owasp/utils/project_level_test.py
📚 Learning: 2026-01-01T18:57:05.007Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/video.py:189-215
Timestamp: 2026-01-01T18:57:05.007Z
Learning: In the OWASP backend area, maintain the established pattern: when dealing with sponsors, include all entries from Sponsor.objects.all() (including NOT_SPONSOR) and perform in-memory sorting using the same criteria/pattern used by the GraphQL sponsor query implemented in backend/apps/owasp/api/internal/queries/sponsor.py. Apply this behavior consistently to files in backend/apps/owasp (not just video.py), and ensure code paths that render sponsor lists follow this in-code sorting approach rather than pre-filtering NOT_SPONSOR entries before sorting.
Applied to files:
backend/apps/owasp/management/commands/owasp_update_project_health_scores.pybackend/apps/owasp/management/commands/owasp_aggregate_projects.pybackend/apps/owasp/management/commands/owasp_update_project_level_compliance.pybackend/apps/owasp/utils/project_level.pybackend/apps/owasp/migrations/0070_projecthealthmetrics_level_non_compliant.pybackend/apps/owasp/models/project_health_metrics.py
🧬 Code graph analysis (6)
backend/tests/apps/owasp/management/commands/owasp_update_project_health_scores_test.py (2)
backend/apps/owasp/models/project_health_metrics.py (1)
ProjectHealthMetrics(16-236)backend/apps/owasp/models/project_health_requirements.py (1)
ProjectHealthRequirements(9-63)
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py (3)
backend/apps/owasp/models/project_health_metrics.py (2)
ProjectHealthMetrics(16-236)bulk_save(150-158)backend/apps/owasp/utils/project_level.py (1)
map_level(18-28)backend/apps/owasp/management/commands/owasp_aggregate_projects.py (2)
Command(9-124)handle(21-124)
backend/apps/owasp/utils/project_level.py (1)
backend/apps/owasp/models/enums/project.py (1)
ProjectLevel(37-44)
backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py (1)
backend/apps/owasp/management/commands/owasp_aggregate_projects.py (1)
handle(21-124)
backend/tests/apps/owasp/management/commands/owasp_update_project_level_compliance_test.py (2)
backend/apps/owasp/models/enums/project.py (1)
ProjectLevel(37-44)backend/tests/apps/owasp/management/commands/owasp_aggregate_contributions_test.py (1)
select_related(36-38)
backend/tests/apps/owasp/utils/project_level_test.py (2)
backend/apps/owasp/models/enums/project.py (1)
ProjectLevel(37-44)backend/apps/owasp/utils/project_level.py (1)
map_level(18-28)
🔇 Additional comments (11)
backend/apps/owasp/utils/project_level.py (1)
18-28: LGTM!The function handles edge cases appropriately: invalid inputs return
None, negative values are rejected, and unmapped positive values also returnNone. The defensive normalization viaDecimal(str(level))ensures robustness against various numeric input types.backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py (1)
79-89: LGTM!The test correctly verifies the conditional invocation of post-processing commands:
- Uses
assert_any_callto verify both commands are called whenoffset == 0.- Uses
assert_not_calledto verify commands are skipped for non-zero offsets.The parameterized test cases include both scenarios, providing good coverage.
backend/tests/apps/owasp/management/commands/owasp_update_project_health_scores_test.py (1)
66-66: LGTM!Correctly setting
level_non_compliant = Falseensures this test exercises the compliant path.backend/apps/owasp/management/commands/owasp_aggregate_projects.py (1)
3-3: LGTM!Import for
call_commandis correctly added to support the post-processing workflow.backend/apps/owasp/management/commands/owasp_update_project_health_scores.py (2)
8-9: LGTM!The penalty constant is well-defined at the module level, making it easy to adjust if needed. The 10.0 penalty aligns with the PR objectives for scoring adjustment.
62-66: LGTM!The penalty logic correctly subtracts from the score when
level_non_compliantisTrueand clamps to0.0to prevent negative scores.backend/apps/owasp/migrations/0070_projecthealthmetrics_level_non_compliant.py (1)
1-17: LGTM!Standard migration adding the
level_non_compliantboolean field with an appropriatedefault=False. Existing records will correctly start as compliant.backend/tests/apps/owasp/management/commands/owasp_update_project_level_compliance_test.py (2)
46-62: LGTM!The test correctly verifies that when the official level (4 → FLAGSHIP) differs from local level (PRODUCTION), the metric is marked as non-compliant.
73-89: LGTM!The test correctly verifies that when the official level (3 → PRODUCTION) matches the local level (PRODUCTION), the metric remains compliant.
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py (2)
20-23: LGTM!The
clean_namefunction effectively normalizes project names by lowercasing, removing "owasp" prefix/occurrences, and stripping non-alphanumeric characters. This should handle most naming variations.
56-72: Consider handling projects not found in official data.When a project isn't found in the official data (lines 71-72
continue), the metric'slevel_non_compliantflag is not updated. If a previously non-compliant project is removed from the official list, it will remain flagged as non-compliant. Consider whether you want to reset the flag or log these cases.
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py
Show resolved
Hide resolved
OM-JADHAV25
changed the title
OM-JADHAV25
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py:
- Around line 90-94: The call to ProjectHealthMetrics.bulk_save uses the wrong
keyword: replace the unexpected keyword argument update_fields with the correct
fields parameter so the call becomes
ProjectHealthMetrics.bulk_save(updated_metrics, fields=["level_non_compliant"]);
update the invocation inside the owasp_update_project_level_compliance command
where bulk_save is called to use fields= instead of update_fields=.
🧹 Nitpick comments (3)
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py (3)
32-35: Add error handling for network and JSON parsing failures.The HTTP request and JSON parsing can fail with
requests.RequestExceptionorjson.JSONDecodeError. Whileraise_for_status()handles HTTP errors, network failures and malformed JSON would cause the command to crash with an unhelpful stack trace.♻️ Suggested improvement
+from requests.exceptions import RequestException + class Command(BaseCommand): help = "Update project level compliance." def handle(self, *args, **options): self.stdout.write("Updating project level compliance...") - response = requests.get(LEVELS_URL, timeout=15) - response.raise_for_status() - - official = response.json() + try: + response = requests.get(LEVELS_URL, timeout=15) + response.raise_for_status() + official = response.json() + except RequestException as exc: + self.stderr.write(self.style.ERROR(f"Failed to fetch project levels: {exc}")) + return + except ValueError as exc: + self.stderr.write(self.style.ERROR(f"Invalid JSON in project levels: {exc}")) + return
78-88: Optimize to only update metrics where compliance status changed.Currently, all metrics with an official level match are added to
updated_metricsand saved, even whenlevel_non_complianthasn't changed. This causes unnecessary database writes.♻️ Suggested improvement
expected_level = map_level(official_level) if expected_level is None: continue - metric.level_non_compliant = project.level != expected_level - if metric.level_non_compliant: + is_non_compliant = project.level != expected_level + if metric.level_non_compliant != is_non_compliant: + metric.level_non_compliant = is_non_compliant + updated_metrics.append(metric) + + if is_non_compliant: self.stdout.write( self.style.WARNING( f"Level mismatch: {project.name} " f"local={project.level} " f"official={expected_level}" ) ) - - updated_metrics.append(metric)
64-66: Consider edge case for URLs with query parameters.The slug extraction assumes clean URLs. If
repo_urlcontains query parameters (e.g.,?ref=main), the slug would include them. This is likely not an issue with OWASP data, but for robustness you could strip query strings.♻️ Optional improvement
+from urllib.parse import urlparse + if project.repo_url: - slug = project.repo_url.rstrip("/").split("/")[-1].lower() + path = urlparse(project.repo_url).path.rstrip("/") + slug = path.split("/")[-1].lower() official_level = by_repo.get(slug)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py
📚 Learning: 2026-01-01T18:57:05.007Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/video.py:189-215
Timestamp: 2026-01-01T18:57:05.007Z
Learning: In the OWASP backend area, maintain the established pattern: when dealing with sponsors, include all entries from Sponsor.objects.all() (including NOT_SPONSOR) and perform in-memory sorting using the same criteria/pattern used by the GraphQL sponsor query implemented in backend/apps/owasp/api/internal/queries/sponsor.py. Apply this behavior consistently to files in backend/apps/owasp (not just video.py), and ensure code paths that render sponsor lists follow this in-code sorting approach rather than pre-filtering NOT_SPONSOR entries before sorting.
Applied to files:
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py
🧬 Code graph analysis (1)
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py (1)
backend/apps/owasp/utils/project_level.py (1)
map_level(18-28)
🔇 Additional comments (3)
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py (3)
1-17: LGTM on imports and constant definition.The imports are appropriate, and pointing to the official OWASP GitHub raw content is a reasonable approach for the source of truth.
20-23: LGTM on name normalization.The function correctly normalizes project names for fuzzy matching. The approach of stripping "owasp" and non-alphanumeric characters is appropriate for matching against official data.
96-96: Good use ofself.stdout.writefor output.This follows Django management command conventions for user-facing output. Based on learnings, this is the preferred approach over
print().
backend/apps/owasp/management/commands/owasp_update_project_level_compliance.py
Show resolved
Hide resolved
|
1 participant
Proposed change
Resolves #2039
This PR implements project level non-compliance detection based on the official OWASP source of truth project_levels.json.
A new management command fetches and parses the latest official project levels, compares them against locally stored project levels, and flags mismatches by setting level_non_compliant on ProjectHealthMetrics.The existing health score calculation is updated to apply a penalty when a project is flagged as non-compliant, ensuring project health scores reflect official level alignment.
The compliance check is executed automatically after the OWASP project sync/aggregate job to ensure the latest synced projects are validated.
Checklist
make check-testlocally: all warnings addressed, tests passed