feat(exp): convert qualitative risk assessments to quantitative ones #2814
Conversation
WalkthroughAdds a backend REST action to convert qualitative RiskAssessments into QuantitativeRiskStudies, new frontend UI/route/form to collect anchoring and submit conversion, updates list table columns, and expands English/French i18n for the quantitative conversion workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FE as Frontend UI
participant SR as SvelteKit Server Action
participant API as Backend API
participant DB as Database
User->>FE: Click "Convert to Quantitative"
FE->>FE: Render anchor form (probability & impact, loss threshold)
User->>FE: Submit form
FE->>SR: POST form (JSON payload)
activate SR
SR->>API: POST /risk-assessments/{id}/convert_to_quantitative/
activate API
API->>DB: Create QuantitativeRiskStudy
loop per scenario
API->>DB: Create QuantitativeRiskScenario & hypotheses
API->>API: Run simulations (current/residual) if valid
end
API-->>SR: {quantitative_risk_study_id, scenarios_converted, scenarios_skipped}
deactivate API
SR-->>FE: JSON response (success or error)
deactivate SR
FE->>FE: Show toasts/progress, redirect to created quantitative study on success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 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: 7
🧹 Nitpick comments (3)
backend/core/views.py (3)
1982-1992: Avoid orphan studies when nothing is convertible.If no scenario has valid current/residual values, an empty QuantitativeRiskStudy is still created. Pre-scan scenarios and bail out with 400 when none qualify.
Minimal pattern:
eligible = [] for sc in risk_assessment.risk_scenarios.all(): if ((sc.current_proba in probability_map and sc.current_impact in impact_map) or (sc.residual_proba in probability_map and sc.residual_impact in impact_map)) and \ (sc.threats.exists() and sc.assets.exists()): eligible.append(sc) if not eligible: return Response({"detail": "No scenarios eligible for conversion"}, status=400) # then create the study and iterate eligible onlyAlso applies to: 2001-2046
2001-2046: Make scenario conversion atomic to prevent partial writes per scenario.Wrap each scenario conversion in a transaction to keep objects consistent if any step fails (relations, simulation, etc.).
Apply this diff:
- for scenario in risk_assessment.risk_scenarios.all(): + for scenario in risk_assessment.risk_scenarios.all(): + with transaction.atomic(): # existing body unchanged…Also applies to: 2060-2139
2064-2070: Prefer default hypothesis ref_ids to avoid collisions.Using scenario.ref_id for hypothesis ref_id can collide; let the model generate default via get_default_ref_id or omit ref_id.
Example:
- ref_id=scenario.ref_id or "", + # let default ref_id be generatedAlso applies to: 2103-2111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/core/views.py(1 hunks)frontend/messages/en.json(1 hunks)frontend/messages/fr.json(1 hunks)frontend/src/lib/utils/table.ts(1 hunks)frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/+page.svelte(1 hunks)frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.server.ts(1 hunks)frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.svelte(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.server.ts (1)
frontend/src/lib/utils/constants.ts (1)
BASE_API_URL(4-8)
backend/core/views.py (3)
backend/crq/models.py (4)
QuantitativeRiskStudy(23-360)QuantitativeRiskScenario(363-524)QuantitativeRiskHypothesis(527-940)run_simulation(595-698)backend/core/models.py (19)
RiskAssessment(4279-4683)create(4123-4125)risk_scenarios(3844-3845)assets(6701-6702)save(1636-1644)save(1760-1763)save(2733-2735)save(3294-3296)save(3369-3376)save(3551-3555)save(3765-3789)save(4136-4138)save(4273-4276)save(4344-4346)save(5007-5033)save(5156-5162)save(6100-6120)save(6649-6662)applied_controls(6705-6706)backend/iam/models.py (3)
RoleAssignment(807-1094)is_access_allowed(837-857)save(349-356)
🔇 Additional comments (3)
frontend/src/lib/utils/table.ts (1)
1919-1920: LGTM: Timestamp column additionThe addition of
updatedAt/updated_atcolumns to the quantitative-risk-studies table configuration is appropriate and follows the established naming conventions (camelCase for head, snake_case for body).frontend/messages/en.json (1)
2577-2605: No issues found. Localization is correct and matches backend validation.The backend validation in
backend/crq/utils.pyconfirms that probabilities must satisfy0 < p < 1(exclusive), which aligns perfectly with the localization message at line 2598: "Probability for '{level}' must be between 0 and 1 (exclusive). Please use a value like 0.05 or 0.9." The examples provided (0.05, 0.9) are valid under this constraint.backend/core/views.py (1)
2043-2045: The folder field is confirmed to exist on both models.Both QuantitativeRiskScenario and QuantitativeRiskHypothesis inherit from FolderMixin, which explicitly defines a folder field as a ForeignKey to Folder. The code at all three affected locations (lines 2043–2045, 2069–2070, and 2109–2110) is correct and will not error.
Likely an incorrect or invalid review comment.
| if not scenario.threats.exists() and not scenario.assets.exists(): | ||
| logger.info( | ||
| f"Skipping scenario '{scenario.name}' - no threats or assets associated" | ||
| ) | ||
| scenarios_skipped += 1 | ||
| continue |
There was a problem hiding this comment.
Bug: skip condition mismatches comment/intent.
Comment says “without threats or assets”; using “and” only skips when both are missing. Use “or” to skip when either side is missing.
Apply this diff:
- # Skip scenarios without threats or assets
- if not scenario.threats.exists() and not scenario.assets.exists():
+ # Skip scenarios without threats or assets
+ if not scenario.threats.exists() or not scenario.assets.exists():📝 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.
| if not scenario.threats.exists() and not scenario.assets.exists(): | |
| logger.info( | |
| f"Skipping scenario '{scenario.name}' - no threats or assets associated" | |
| ) | |
| scenarios_skipped += 1 | |
| continue | |
| if not scenario.threats.exists() or not scenario.assets.exists(): | |
| logger.info( | |
| f"Skipping scenario '{scenario.name}' - no threats or assets associated" | |
| ) | |
| scenarios_skipped += 1 | |
| continue |
🤖 Prompt for AI Agents
In backend/core/views.py around lines 2003 to 2008 the skip condition currently
uses "and" so it only skips when both threats and assets are missing, which
contradicts the comment/intent; change the condition to use "or" so scenarios
missing either threats or assets are skipped, and update the log message if
needed to reflect "no threats or no assets" (ensure scenarios_skipped increment
and continue remain unchanged).
frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/+page.svelte
Show resolved
Hide resolved
...c/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.server.ts
Show resolved
Hide resolved
.../src/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.svelte (1)
339-379: Attach payload inside enhance so all submission paths work (Enter/AT included).Currently the hidden input is only populated in the button’s onclick handler. Submitting via Enter or assistive tech skips it and posts an empty payload.
Move serialization into enhance, set formData there, and drop the onclick.
- <form - method="POST" - use:enhance={() => { - const payload = validateAndPrepareData(); - if (!payload) { - return async ({ update }) => { - await update({ reset: false }); - }; - } - - isSubmitting = true; - - return async ({ result, update }) => { - await update(); - if (result.type !== 'success') { - isSubmitting = false; - } - }; - }} - > - <input type="hidden" name="data" value="" /> + <form + method="POST" + use:enhance={(form) => { + return async ({ formData, cancel, update }) => { + const payload = validateAndPrepareData(); + if (!payload) { + cancel(); + await update({ reset: false }); + return; + } + formData.set('data', JSON.stringify(payload)); + isSubmitting = true; + const result = await update(); + if (!result || result.type !== 'success') { + isSubmitting = false; + } + }; + }} + > + <input type="hidden" name="data" /> @@ - <button + <button type="submit" class="btn preset-filled-primary-500" disabled={isSubmitting} - onclick={(e) => { - const payload = validateAndPrepareData(); - if (!payload) { - e.preventDefault(); - return; - } - const form = e.currentTarget.closest('form'); - if (form) { - const input = form.querySelector('input[name="data"]'); - if (input) { - input.value = JSON.stringify(payload); - } - } - }} >
🧹 Nitpick comments (1)
frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.svelte (1)
229-233: Avoid redefining the global “requirements” i18n key; add the colon in markup instead.Using m.requirements() is fine; append a colon in the template to avoid colliding with the existing “requirements” key used elsewhere.
- <div> - <strong>{m.requirements()}</strong> - {m.probabilityRequirements()} - </div> + <div> + <strong>{m.requirements()}</strong>: {m.probabilityRequirements()} + </div> @@ - <div> - <strong>{m.requirements()}</strong> - {m.impactRequirements()} - </div> + <div> + <strong>{m.requirements()}</strong>: {m.impactRequirements()} + </div>Also applies to: 281-284
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/messages/en.json(3 hunks)frontend/messages/fr.json(2 hunks)frontend/src/lib/components/Forms/ModelForm/QuantitativeRiskHypothesisForm.svelte(2 hunks)frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-10T17:32:54.445Z
Learnt from: nas-tabchiche
Repo: intuitem/ciso-assistant-community PR: 1332
File: frontend/src/routes/(app)/(internal)/stored-libraries/[id=uuid]/+page.svelte:0-0
Timestamp: 2025-02-10T17:32:54.445Z
Learning: In SvelteKit applications using form actions with `enhance` and `applyAction`, error handling is managed at the form action level, making additional try/catch blocks in the frontend fetch redundant.
Applied to files:
frontend/src/routes/(app)/(internal)/risk-assessments/[id=uuid]/convert-to-quantitative/+page.svelte
🔇 Additional comments (1)
frontend/messages/fr.json (1)
2538-2566: Localization additions look consistent; consider mirroring the EN cleanup.FR didn’t introduce a second “requirements” key (good). Once EN removes the duplicate, both locales will stay aligned. No blocking issues here.
If you drop the duplicate in EN, re-run the duplicate‑key check script shared in the EN comment to confirm FR stays clean.
frontend/src/lib/components/Forms/ModelForm/QuantitativeRiskHypothesisForm.svelte
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
backend/core/views.py (4)
1948-1974: Make loss_threshold optional and accept 0; fix required-fields check.Doc/design calls loss_threshold optional; current check requires it and rejects 0. Coerce when provided; allow None and 0.
Apply this diff:
- # Validate payload + # Validate payload probability_anchors = request.data.get("probability_anchors", []) impact_anchors = request.data.get("impact_anchors", []) loss_threshold = request.data.get("loss_threshold") - if not probability_anchors or not impact_anchors or loss_threshold is None: + if not probability_anchors or not impact_anchors: return Response( { - "detail": "Missing required fields: probability_anchors, impact_anchors, and loss_threshold" + "detail": "Missing required fields: probability_anchors and impact_anchors" }, status=status.HTTP_400_BAD_REQUEST, ) - # Validate loss_threshold is a positive number - try: - loss_threshold = float(loss_threshold) - if loss_threshold <= 0: - return Response( - {"detail": "loss_threshold must be greater than 0"}, - status=status.HTTP_400_BAD_REQUEST, - ) - except (TypeError, ValueError): - return Response( - {"detail": "loss_threshold must be a valid number"}, - status=status.HTTP_400_BAD_REQUEST, - ) + # Coerce optional loss_threshold if provided (0 is valid) + if loss_threshold is not None: + try: + loss_threshold = float(loss_threshold) + except (TypeError, ValueError): + return Response( + {"detail": "loss_threshold must be a number"}, + status=status.HTTP_400_BAD_REQUEST, + )
1975-1994: Cast anchor indices to int; prevent mismatches with scenario.*_proba.scenario.current_proba/residual_proba are ints; keeping string indices causes lookups to fail.
Apply this diff:
- probability_map = {} + probability_map: dict[int, float] = {} try: for item in probability_anchors: - idx = item["index"] + idx = int(item["index"]) value = float(item["value"]) if not (0 <= value <= 1): return Response( { "detail": f"Probability value must be between 0 and 1, got {value} for index {idx}" }, status=status.HTTP_400_BAD_REQUEST, ) probability_map[idx] = value except (KeyError, TypeError, ValueError) as e: return Response( {"detail": f"Invalid probability anchor format: {str(e)}"}, status=status.HTTP_400_BAD_REQUEST, )
2001-2019: Normalize impact indices to int; add explicit bounds sanity check.Avoid string/int key mismatch; ensure 0 < lb < ub explicitly.
Apply this diff:
- impact_map = {} + impact_map: dict[int, dict[str, float]] = {} try: for item in impact_anchors: - idx = item["index"] + idx = int(item["index"]) central_value = float(item["central_value"]) if central_value <= 0: return Response( { "detail": f"Impact central value must be positive, got {central_value} for index {idx}" }, status=status.HTTP_400_BAD_REQUEST, ) - impact_map[idx] = { - "lb": central_value * 0.7, - "ub": central_value * 1.43, - } + lb = central_value * 0.7 + ub = central_value * 1.43 + if not (lb > 0 and ub > lb): + return Response( + {"detail": f"Invalid derived bounds for index {idx}"}, + status=status.HTTP_400_BAD_REQUEST, + ) + impact_map[idx] = {"lb": lb, "ub": ub} except (KeyError, TypeError, ValueError) as e: return Response( {"detail": f"Invalid impact anchor format: {str(e)}"}, status=status.HTTP_400_BAD_REQUEST, )
2047-2053: Bug: skip condition should use “or”, not “and”.Comment says “without threats or assets”; current logic only skips when both missing.
- # Skip scenarios without threats or assets - if not scenario.threats.exists() and not scenario.assets.exists(): + # Skip scenarios without threats or assets + if not scenario.threats.exists() or not scenario.assets.exists(): logger.info( - f"Skipping scenario '{scenario.name}' - no threats or assets associated" + f"Skipping scenario '{scenario.name}' - no threats or no assets associated" ) scenarios_skipped += 1 continue
🧹 Nitpick comments (2)
backend/core/views.py (2)
2045-2047: Reduce N+1 queries when copying relations.Prefetch M2M before looping.
- # Convert each risk scenario to a quantitative risk scenario - for scenario in risk_assessment.risk_scenarios.all(): + # Convert each risk scenario to a quantitative risk scenario + scenarios = risk_assessment.risk_scenarios.all().prefetch_related( + "threats", "assets", "vulnerabilities", "owner", + "existing_applied_controls", "applied_controls" + ) + for scenario in scenarios:
2132-2142: Avoid long compute in request; consider async or post-commit.run_simulation can be heavy; move to background job or transaction.on_commit callbacks to keep API snappy and resilient.
Also applies to: 2176-2186
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/core/views.py(1 hunks)frontend/messages/en.json(3 hunks)frontend/src/lib/components/Forms/ModelForm/QuantitativeRiskHypothesisForm.svelte(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/lib/components/Forms/ModelForm/QuantitativeRiskHypothesisForm.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-05T09:18:16.194Z
Learnt from: Mohamed-Hacene
Repo: intuitem/ciso-assistant-community PR: 2472
File: backend/iam/models.py:906-907
Timestamp: 2025-09-05T09:18:16.194Z
Learning: In the get_accessible_object_ids method in backend/iam/models.py, for objects with parent_folder attribute (like Folder objects), the folder association logic treats each folder as being associated with itself, so the mapping is folder_id → folder_id. The code `[(fid, fid) for fid in folder_permissions.keys()]` correctly implements this self-referential mapping rather than querying for child folders.
Applied to files:
backend/core/views.py
📚 Learning: 2025-08-25T08:51:15.404Z
Learnt from: Mohamed-Hacene
Repo: intuitem/ciso-assistant-community PR: 2422
File: backend/core/serializers.py:1018-1030
Timestamp: 2025-08-25T08:51:15.404Z
Learning: The CISO Assistant project uses a custom permission system where RoleAssignment.get_accessible_object_ids() provides special handling for Permission objects by filtering them by content_type app_label rather than folder hierarchy, since Permission objects don't belong to folders. This allows safe CRUD operations on permissions while preventing privilege escalation by restricting access to only application-specific permissions from allowed apps: "core", "ebios_rm", "tprm", "privacy", "resilience", and "cal".
Applied to files:
backend/core/views.py
🧬 Code graph analysis (1)
backend/core/views.py (3)
backend/crq/models.py (4)
QuantitativeRiskStudy(23-360)QuantitativeRiskScenario(363-524)QuantitativeRiskHypothesis(527-940)run_simulation(595-698)backend/core/models.py (19)
RiskAssessment(4279-4683)create(4123-4125)risk_scenarios(3844-3845)assets(6701-6702)save(1636-1644)save(1760-1763)save(2733-2735)save(3294-3296)save(3369-3376)save(3551-3555)save(3765-3789)save(4136-4138)save(4273-4276)save(4344-4346)save(5007-5033)save(5156-5162)save(6100-6120)save(6649-6662)applied_controls(6705-6706)backend/iam/models.py (3)
RoleAssignment(807-1094)is_access_allowed(837-857)save(349-356)
⏰ 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). (9)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: test (3.12)
- GitHub Check: build_community_frontend
- GitHub Check: build_enterprise_frontend
- GitHub Check: build (3.12)
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
frontend/messages/en.json (2)
2580-2606: ✅ LGTM — Comprehensive and well-structured localization keys for quantitative conversion workflow.The new keys added (lines 2580–2606) properly support the convert-to-quantitative workflow with clear, non-duplicative messaging. Validation errors are specific and helpful (e.g., "probabilityMustBeBetweenZeroAndHundred", "impactValuesMustBeIncreasing"), help texts explain requirements clearly, and status messages guide users through the Monte Carlo simulation process. No duplicate keys were introduced, and the previous review's concerns about key collisions have been successfully avoided.
2274-2275: ✅ Probability percentage variants added appropriately.Lines 2274–2275 introduce "probabilityPercent" and "probabilityPercentHelpText" to support percentage-based input (0–100 range), complementing the existing probability keys. The help text clearly distinguishes this from the 0–1 range variant at line 2047.
backend/core/views.py (2)
1933-1936: Good: RBAC-safe object retrieval.Using self.get_object() enforces the scoped queryset and avoids existence leaks.
2083-2091: No action required — QuantitativeRiskScenario has a folder field through FolderMixin inheritance.QuantitativeRiskScenario inherits from FolderMixin, which defines a folder foreign key field. The code at lines 2083–2091 correctly passes
folder=risk_assessment.folderto thecreate()method. No keyword errors will occur.
| current_hypothesis = QuantitativeRiskHypothesis.objects.create( | ||
| quantitative_risk_scenario=quant_scenario, | ||
| name="current", | ||
| risk_stage="current", | ||
| ref_id=scenario.ref_id or "", | ||
| folder=risk_assessment.folder, | ||
| parameters={ | ||
| "probability": probability_value, | ||
| "impact": { | ||
| "distribution": "LOGNORMAL-CI90", | ||
| "lb": impact_value["lb"], | ||
| "ub": impact_value["ub"], | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
QuantitativeRiskHypothesis likely has no “name” field — creation will crash.
Model summary lists fields; name is absent. Passing name=... will raise TypeError. Drop it.
Apply this diff:
- current_hypothesis = QuantitativeRiskHypothesis.objects.create(
+ current_hypothesis = QuantitativeRiskHypothesis.objects.create(
quantitative_risk_scenario=quant_scenario,
- name="current",
risk_stage="current",
ref_id=scenario.ref_id or "",
folder=risk_assessment.folder,
parameters={
"probability": probability_value,
"impact": {
"distribution": "LOGNORMAL-CI90",
"lb": impact_value["lb"],
"ub": impact_value["ub"],
},
},
)- residual_hypothesis = QuantitativeRiskHypothesis.objects.create(
+ residual_hypothesis = QuantitativeRiskHypothesis.objects.create(
quantitative_risk_scenario=quant_scenario,
- name="residual",
risk_stage="residual",
is_selected=True,
ref_id=f"{scenario.ref_id}_RES" if scenario.ref_id else "",
folder=risk_assessment.folder,
parameters={
"probability": residual_probability_value,
"impact": {
"distribution": "LOGNORMAL-CI90",
"lb": residual_impact_value["lb"],
"ub": residual_impact_value["ub"],
},
},
)Also applies to: 2148-2163
🤖 Prompt for AI Agents
In backend/core/views.py around lines 2109-2123 (and similarly lines 2148-2163),
the QuantitativeRiskHypothesis.objects.create calls include a name=... keyword
that does not exist on the model and will raise a TypeError; remove the name=...
argument from both create calls so only valid model fields
(quantitative_risk_scenario, risk_stage, ref_id, folder, parameters, etc.) are
passed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/core/views.py (1)
2053-2057: Bug: skip condition should be “or”, not “and”.Comment says “without threats or assets” but code only skips when both are missing.
- # Skip scenarios without threats or assets - if not scenario.threats.exists() and not scenario.assets.exists(): + # Skip scenarios without threats or assets + if not scenario.threats.exists() or not scenario.assets.exists():
🧹 Nitpick comments (1)
backend/core/views.py (1)
2138-2147: Surface simulation failures and report them in the response.Currently failures are only logged; API always counts the scenario as converted. Track failures and return them so clients can react (and optionally don’t count failed ones as converted).
- scenarios_converted = 0 - scenarios_skipped = 0 + scenarios_converted = 0 + scenarios_skipped = 0 + scenarios_failed = 0 @@ - except Exception as e: - logger.error( + except Exception as e: + scenarios_failed += 1 + logger.error( f"Failed to run simulation for scenario {scenario.name}: {str(e)}" ) @@ - except Exception as e: - logger.error( + except Exception as e: + scenarios_failed += 1 + logger.error( f"Failed to run simulation for residual scenario {scenario.name}: {str(e)}" ) @@ - return Response( + return Response( { "message": "Conversion successful", "quantitative_risk_study_id": str(quantitative_study.id), "scenarios_converted": scenarios_converted, "scenarios_skipped": scenarios_skipped, + "scenarios_failed": scenarios_failed, }, status=status.HTTP_201_CREATED, )Also applies to: 2183-2191, 2199-2207
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/core/views.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-05T09:18:16.194Z
Learnt from: Mohamed-Hacene
Repo: intuitem/ciso-assistant-community PR: 2472
File: backend/iam/models.py:906-907
Timestamp: 2025-09-05T09:18:16.194Z
Learning: In the get_accessible_object_ids method in backend/iam/models.py, for objects with parent_folder attribute (like Folder objects), the folder association logic treats each folder as being associated with itself, so the mapping is folder_id → folder_id. The code `[(fid, fid) for fid in folder_permissions.keys()]` correctly implements this self-referential mapping rather than querying for child folders.
Applied to files:
backend/core/views.py
📚 Learning: 2025-08-25T08:51:15.404Z
Learnt from: Mohamed-Hacene
Repo: intuitem/ciso-assistant-community PR: 2422
File: backend/core/serializers.py:1018-1030
Timestamp: 2025-08-25T08:51:15.404Z
Learning: The CISO Assistant project uses a custom permission system where RoleAssignment.get_accessible_object_ids() provides special handling for Permission objects by filtering them by content_type app_label rather than folder hierarchy, since Permission objects don't belong to folders. This allows safe CRUD operations on permissions while preventing privilege escalation by restricting access to only application-specific permissions from allowed apps: "core", "ebios_rm", "tprm", "privacy", "resilience", and "cal".
Applied to files:
backend/core/views.py
🧬 Code graph analysis (1)
backend/core/views.py (3)
backend/crq/models.py (4)
QuantitativeRiskStudy(23-360)QuantitativeRiskScenario(363-524)QuantitativeRiskHypothesis(527-940)run_simulation(595-698)backend/core/models.py (18)
RiskAssessment(4279-4683)risk_scenarios(3844-3845)assets(6701-6702)save(1636-1644)save(1760-1763)save(2733-2735)save(3294-3296)save(3369-3376)save(3551-3555)save(3765-3789)save(4136-4138)save(4273-4276)save(4344-4346)save(5007-5033)save(5156-5162)save(6100-6120)save(6649-6662)applied_controls(6705-6706)backend/iam/models.py (3)
RoleAssignment(807-1094)is_access_allowed(837-857)save(349-356)
⏰ 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). (9)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: Analyze (python)
- GitHub Check: build_community_frontend
- GitHub Check: build_enterprise_frontend
- GitHub Check: build (3.12)
- GitHub Check: test (3.12)
🔇 Additional comments (1)
backend/core/views.py (1)
1933-1941: Good RBAC: get_object() enforces scoped queryset and permissions.Replacing direct ORM fetch with self.get_object() is correct and prevents info disclosure.
| probability_map = {} | ||
| try: | ||
| for item in probability_anchors: | ||
| idx = item["index"] | ||
| value = float(item["value"]) | ||
| if not (0 <= value <= 1): | ||
| return Response( | ||
| { | ||
| "detail": f"Probability value must be between 0 and 1, got {value} for index {idx}" | ||
| }, | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
| ) | ||
| probability_map[idx] = value | ||
| except (KeyError, TypeError, ValueError) as e: | ||
| logger.warning(f"Invalid probability anchor format: {e}") | ||
| return Response( | ||
| { | ||
| "detail": "Invalid probability anchor format. Each anchor must have 'index' and 'value' fields, with value between 0 and 1." | ||
| }, | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Coerce anchor index to int to avoid silent skips (key type mismatch).
idx is taken as-is from JSON; if frontend sends "0" (string), membership checks like current_proba in probability_map will fail. Cast index to int when building both maps.
- for item in probability_anchors:
- idx = item["index"]
+ for item in probability_anchors:
+ idx = int(item["index"])
value = float(item["value"])
if not (0 <= value <= 1):
return Response(
@@
- for item in impact_anchors:
- idx = item["index"]
+ for item in impact_anchors:
+ idx = int(item["index"])
central_value = float(item["central_value"])
if central_value <= 0:
return Response(Also applies to: 2003-2018
🤖 Prompt for AI Agents
In backend/core/views.py around lines 1976 to 1997 (and similarly at 2003–2018),
the anchor index is taken directly from JSON which can be a string and later
cause key-mismatch lookups; coerce the index to an int when building the
probability_map (e.g., idx = int(item["index"])) so all map keys are integers,
and let the existing except block continue to catch ValueError/TypeError if
conversion fails; apply the same int coercion to the other map-building block at
2003–2018 to ensure consistent key types.
Summary by CodeRabbit
New Features
UI
Localization