CS-424 [Improvement] Include a default justification at all times on the SoA#2921
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
chasprowebdev
changed the title
Contributor
|
@cubic-dev-ai please review it |
Contributor
@chasprowebdev I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 8 files
Confidence score: 3/5
- There is a concrete runtime risk in
apps/api/src/soa/utils/soa-answer-parser.ts: callingtrimwithout verifying the justification value is a string can throw on non-string JSON and break answer parsing for affected requests. apps/api/src/soa/utils/constants.tshas a medium-impact fallback gap where many controls can still returnnull, so some YES/default flows may persist without a justification instead of a safe generic default.- Given the two medium-to-high severity issues (6–7/10) with strong confidence, this looks like some regression risk rather than a merge-blocker if those paths are uncommon.
- Pay close attention to
apps/api/src/soa/utils/soa-answer-parser.ts,apps/api/src/soa/utils/constants.ts- parser type-guarding and fallback defaults directly affect justification persistence reliability.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Contributor
|
@cubic-dev-ai please review it |
Contributor
@chasprowebdev I have started the AI code review. It will take a few minutes to complete. |
Contributor
|
@cubic-dev-ai review it |
Contributor
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
Ultrareview completed in 10m 26s
3 issues found across 8 files
Confidence score: 3/5
- There is a concrete user-facing data quality risk: in
apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsx, YES answers can be persisted with blank justification, which can leave incomplete SOA records. - Rendering/default behavior is inconsistent in
apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/SOATableRow.tsxbecause??treats empty strings as valid, so fallback justification is skipped and users see—. - Parser logic in
apps/api/src/soa/utils/soa-answer-parser.tsaccepts insufficient placeholder-like YES justifications as valid, which can prevent intended default justification fallback across API flows. - Pay close attention to
apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsx,apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/SOATableRow.tsx, andapps/api/src/soa/utils/soa-answer-parser.ts- justification validation/fallback rules are misaligned and can propagate blank or weak values.
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="apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsx:146">
P2: YES answers can be saved without a non-empty justification, allowing blank justifications to persist.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Comment on lines
+146
to
+152
| if (isApplicable === false && (!justification || justification.trim().length === 0)) { | ||
| setError('Justification is required when Applicable is NO'); | ||
| justificationTextareaRef.current?.focus(); | ||
| return; | ||
| } | ||
|
|
||
| await executeSave(false, justification); | ||
| await executeSave(isApplicable, justification); |
Contributor
There was a problem hiding this comment.
P2: YES answers can be saved without a non-empty justification, allowing blank justifications to persist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsx, line 146:
<comment>YES answers can be saved without a non-empty justification, allowing blank justifications to persist.</comment>
<file context>
@@ -152,13 +143,13 @@ export function EditableSOAFields({
const handleJustificationSave = async () => {
- if (!justification || justification.trim().length === 0) {
+ if (isApplicable === false && (!justification || justification.trim().length === 0)) {
setError('Justification is required when Applicable is NO');
justificationTextareaRef.current?.focus();
</file context>
Suggested change
| if (isApplicable === false && (!justification || justification.trim().length === 0)) { | |
| setError('Justification is required when Applicable is NO'); | |
| justificationTextareaRef.current?.focus(); | |
| return; | |
| } | |
| await executeSave(false, justification); | |
| await executeSave(isApplicable, justification); | |
| const normalizedJustification = justification?.trim() ?? ''; | |
| if (isApplicable !== null && normalizedJustification.length === 0) { | |
| setError('Justification is required'); | |
| justificationTextareaRef.current?.focus(); | |
| return; | |
| } | |
| await executeSave( | |
| isApplicable, | |
| normalizedJustification.length > 0 ? normalizedJustification : null, | |
| ); |
Contributor
There was a problem hiding this comment.
Yes answers can be saved without any justification.
Contributor
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
Contributor
|
@cubic-dev-ai Ultrareview |
Contributor
@chasprowebdev Starting ultrareview - a deeper analysis than a regular review. I'll post findings when complete. |
Contributor
There was a problem hiding this comment.
Ultrareview completed in 10m 4s
2 issues found across 8 files
Confidence score: 3/5
- There is a concrete user-impacting risk in
apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsx: save errors fromsaveAnswerare swallowed, sohandleJustificationSavecan close the dialog as if successful even when the save failed. apps/api/src/soa/utils/soa-answer-parser.tshas a moderate logic gap where YES defaults can still yield a null justification when closure is empty, so the intended always-justify behavior is not fully enforced.- Given a medium-severity, high-confidence UX/data integrity issue plus a secondary parser inconsistency, this carries some regression risk and is better treated as a cautious merge.
- Pay close attention to
apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsxandapps/api/src/soa/utils/soa-answer-parser.ts- error handling on save flow and null-justification edge cases should be verified.
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="apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/documents/statement-of-applicability/components/EditableSOAFields.tsx:146">
P2: YES answers can be saved without a non-empty justification, allowing blank justifications to persist.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Contributor
|
@cubic-dev-ai Ultrareview |
Contributor
@chasprowebdev Starting ultrareview - a deeper analysis than a regular review. I'll post findings when complete. |
Contributor
|
🎉 This PR is included in version 3.67.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
3 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.
This is an automated pull request to merge chas/soa-justification into dev.
It was created by the [Auto Pull Request] action.
Summary by cubic
Ensures every SoA control always has a justification by defaulting YES answers to ISO 27001:2022 family justifications (with a generic fallback) and persisting/showing them for both Applicable and Not Applicable. Also keeps the justification editor open if a save fails and guarantees a non-null default for YES. Addresses CS-424.
New Features
INCLUSION_JUSTIFICATIONSandgetInclusionJustification; pass the control closure to parsing/defaults; inject a family or generic default; save justification for both YES and NO.Bug Fixes
Written for commit 7f564df. Summary will update on new commits.