Skip to content

fix(oscal): use the correct sectionID in list export - #1092

Open
Devansh-567 wants to merge 1 commit into
OWASP:mainfrom
Devansh-567:fix/oscal-sectionid
Open

Devansh-567 wants to merge 1 commit into
OWASP:mainfrom
Devansh-567:fix/oscal-sectionid

Conversation

@Devansh-567

@Devansh-567 Devansh-567 commented Sep 12, 2026

Copy link
Copy Markdown

PR fix

While looking through the OSCAL export code, I noticed that list_to_oscal() was using doc.section when creating the sectionID property.

This meant that the exported OSCAL data could contain the section text as both section and sectionID, while the actual sectionID value was never exported.

The issue is a small field mix-up here:
name="sectionID",
value="".join(doc.section.splitlines()).strip(),

This should use doc.sectionID instead.

What changed

  • Updated list_to_oscal() to use doc.sectionID for the sectionID OSCAL property.
  • Fixed test_list_to_oscal so it actually tests list_to_oscal() nstead of document_to_oscal().
  • Made the test data use different values for section and sectionID, so this kind of mix-up will be caught in the future.

Testing

I verified that the new test fails with the old implementation and passes with the fix.

The full test suite passes with:

  • 998 passed, 6 skipped, 0 failures

Signed-off-by: Devansh-567 <devansh.jay.singh@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: a37359e6-11ba-4193-b9a1-63b04048824b

📥 Commits

Reviewing files that changed from the base of the PR and between 5a3c384 and 910815f.

📒 Files selected for processing (2)
  • application/tests/oscal_utils_test.py
  • application/utils/oscal_utils.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Summary by CodeRabbit

  • Bug Fixes
    • Standard document exports now preserve the correct section identifier in control properties.
    • Improved coverage verifies section names, section identifiers, metadata titles, and generated control counts.

Walkthrough

Changes

OSCAL section mapping

Layer / File(s) Summary
List conversion mapping
application/utils/oscal_utils.py, application/tests/oscal_utils_test.py
list_to_oscal now populates sectionID from doc.sectionID. The test validates distinct section and section identifier values across four generated controls.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 91081

The OSCAL export now preserves the intended section identifier mapping with direct regression coverage; no unresolved merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the main fix: using the correct sectionID during OSCAL list export.
Description check ✅ Passed The description accurately explains the sectionID field mix-up, the implementation fix, the test updates, and the reported test results.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Devansh-567

Copy link
Copy Markdown
Author

Hi @northdpole, are there any active reviewers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant