Skip to content

Fix missing background color coding in AAD report when provider JSON lacks report data keys - #2258

Draft
stark256-spec wants to merge 1 commit into
cisagov:mainfrom
stark256-spec:fix/aad-report-color-coding
Draft

Fix missing background color coding in AAD report when provider JSON lacks report data keys#2258
stark256-spec wants to merge 1 commit into
cisagov:mainfrom
stark256-spec:fix/aad-report-color-coding

Conversation

@stark256-spec

@stark256-spec stark256-spec commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🗣 Description

Proposed solution for: #2242

💭 Motivation and context

When Invoke-SCuBACached is run against a provider JSON file that lacks one of the AAD report data keys (cap_table_data, risky_applications, risky_third_party_service_principals, severity_score_weights) — e.g. an export created by an older ScubaGear version — the AAD report renders with no Pass/Fail background colors, as described in #2242.

Root cause (two layers):

  1. Report generation (PowerShell 5.1): CreateReport.psm1 calls ConvertTo-Json $SettingsExport.risky_applications etc. directly. On Windows PowerShell 5.1, ConvertTo-Json $null fails parameter binding ("Cannot bind argument to parameter 'InputObject' because it is null"). The error is statement-terminating only, so the report still generates — but the variable is left empty, producing an empty JSON data island: <script type='application/json' id='risky-apps-json'> </script>. (PowerShell 7 returns the literal null here, which is why this only reproduces on 5.1 — the version reported in the issue.)

  2. Report rendering (JavaScript): IndividualReport.js loads all JSON data islands at the top of the DOMContentLoaded handler with getJsonData, which throws on invalid JSON. JSON.parse("") throws, the exception aborts the whole handler, and colorRows() is never reached — so every policy table renders with a white background. The server-rendered tables are unaffected, matching the screenshot in the issue.

Note the unit test stub ProviderSettingsExport.json already reproduces the data shape that triggers this: it contains cap_table_data but none of the risky app keys.

Changes

  • CreateReport.psm1: fall back to the JSON literal "null" when a report data key is absent from the provider export, instead of passing $null to ConvertTo-Json. The report scripts already handle null islands (buildExpandableTable returns early).
  • Utils.js: add tryGetJsonData, which returns null and logs to the console instead of throwing, so one bad data island cannot abort the rest of report rendering.
  • IndividualReport.js: use tryGetJsonData for the data island loads, so colorRows() (and the rest of the handler) always runs.
  • New-Report.Tests.ps1: assert that every JSON data island embedded in the generated AAD report is non-empty, valid JSON. Because the CI unit test job runs under Windows PowerShell 5.1 (shell: powershell), this test fails on main and passes with this fix.

🧪 Testing

  • Invoke-Pester PowerShell/ScubaGear/Testing/Unit/PowerShell/CreateReport/ — 73/73 passing
  • Invoke-PSSA (repo PSScriptAnalyzer config) — 0 errors, 0 warnings
  • Verified tryGetJsonData degrades gracefully for all three failure modes (empty island, missing element, null island) and that buildExpandableTable(null, ...) returns early as before

I did not have access to the specific agency provider file referenced in the issue, so it would be great if @tkol2022 could confirm the fixed report renders with colors against that file. Any malformed or missing data island now degrades to a console error instead of wiping out the report's color coding.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

@stark256-spec

Copy link
Copy Markdown
Contributor Author

While working on this fix I noticed a separate, unrelated bug in the report scripts and kept it out of this PR to avoid scope creep.

The dark mode default flag never applies. The dark-mode-flag data island contains a raw true/false (from $DarkMode.ToString().ToLower() in CreateReport.psm1), so JSON.parse returns a boolean. But both IndividualReport.js and ParentReport.js compare it to a string:

const darkMode = getJsonData('dark-mode-flag') === "true";

true === "true" is always false, so running with -DarkMode $true never defaults the report to dark mode; only the manual toggle (via sessionStorage) works. The fix would be comparing against the boolean (=== true) in both files.

If that sounds right to you, I am happy to open a separate small PR for it so the changes stay clean.

@FollyBeachGurl FollyBeachGurl added the public-reported This issue is reported by the public users of the tool. label Jul 2, 2026
@FollyBeachGurl FollyBeachGurl added this to the Riptide milestone Jul 7, 2026
@FollyBeachGurl

Copy link
Copy Markdown
Collaborator

@mitchelbaker-cisa is scuba diving

@FollyBeachGurl
FollyBeachGurl marked this pull request as draft July 28, 2026 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

public-reported This issue is reported by the public users of the tool.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants