fix: close audit items A3 and A4 — error detail depth and JsonObject help - #43
Merged
Conversation
The errors[] fallback exists to surface an entry whose shape we don't
recognise. Dumping it at -Depth 5 assumed the actionable field sits above the
cut — an assumption the fallback is, by definition, in no position to make.
Past the limit PowerShell doesn't error: it substitutes "@{...}", which is not
JSON, writes a truncation warning into the caller's stream, and drops the rest.
A nested {"reason":"tenant not in key scope"} rendered as
{"L8":{"L7":{"L6":{"L5":{"L4":{"L3":"@{L2=}"}}}}}} — the reason gone, replaced
by something unparseable.
This closes audit item A4, which had proposed the opposite resolution (amend
the contract to allow a shallow depth for error rendering). Checking what
truncation actually produces settled it the other way: the guardian rule was
right.
Swept the rest while here — all 30 ConvertTo-Json calls in module/Public and
module/Private are Depth 100, and the only deliberate shallow one is the
documented Format.ps1xml PolicyData preview at Depth 3.
Test mutation-verified against the old depth. 452/0/2.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Six cmdlets documented the parameter but never demonstrated it, so Get-Help <cmdlet> -Examples gave no hint that JSON output existed: Get-InforcerReportRun, Get-InforcerReportType, Get-InforcerUser, Invoke-InforcerAssessment, Invoke-InforcerReport, Save-InforcerReportOutput. docs/CMDLET-REFERENCE.md had covered all six for a while — only the in-shell help was short, which is why this read as a docs gap for longer than it was. Each example matches its own file's indentation and blank-line convention, so Get-Help renders it identically to the examples already there (two styles are in use across module/Public). Guarded by a discovery-time scan of the source rather than a hardcoded list: every cmdlet whose ValidateSet offers JsonObject is checked, so a new one inherits the test for free. 15 today, mutation-verified. Closes audit item A3. 467/0/2. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
Three JSON help examples need correction for valid GUID usage or accurate filtering behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Closes audit items A3 and A4 by preventing deep API error truncation and documenting JsonObject output in cmdlet help.
Changes:
- Raises error serialization depth to 100.
- Adds JSON examples and discovery-based documentation tests.
- Updates comments, version metadata, changelog, and ignore rules.
File summaries
| File | Description |
|---|---|
Tests/Consistency.Tests.ps1 |
Adds depth and help-documentation regression tests. |
module/Public/Save-InforcerReportOutput.ps1 |
Adds a JsonObject help example. |
module/Public/Invoke-InforcerReport.ps1 |
Adds a JsonObject help example. |
module/Public/Invoke-InforcerAssessment.ps1 |
Adds a JsonObject help example. |
module/Public/Get-InforcerUser.ps1 |
Adds a JsonObject help example. |
module/Public/Get-InforcerReportType.ps1 |
Adds a JsonObject help example. |
module/Public/Get-InforcerReportRun.ps1 |
Adds a JsonObject help example. |
module/Private/Format-InforcerErrorDetail.ps1 |
Prevents deep error-detail truncation. |
module/Private/ConvertTo-InforcerSettingRows.ps1 |
Clarifies decoding comments. |
module/Private/ConvertFrom-InforcerBase64Text.ps1 |
Clarifies UTF-8 validation comments. |
module/InforcerCommunity.psd1 |
Bumps the module version to 0.7.2. |
CHANGELOG.md |
Documents the fixes. |
.gitignore |
Ignores local demo exports. |
Review details
Suppressed comments (1)
module/Public/Get-InforcerReportType.ps1:45
- In
JsonObjectmode this cmdlet returns immediately with the raw catalog before applying theKey/Tag/OutputFormatfilters (see the early return at lines 165–172). Therefore this example returns the full catalog, not theActiveUserCountreport type as its description claims; remove-Keyor change the JSON path to apply the filter.
Get-InforcerReportType -Key ActiveUserCount -OutputType JsonObject
Returns the report type as a JSON string (depth 100) instead of objects.
- Files reviewed: 12/13 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Get-InforcerReportRun -IncludeOutputs | ||
| Returns every run with its outputs array embedded (1 extra API call per run). | ||
| .EXAMPLE | ||
| Get-InforcerReportRun -RunId 8842 -OutputType JsonObject |
| Save-InforcerReportOutput -OutputPath ./bulk | ||
| Bulk-downloads every output from every visible run. | ||
| .EXAMPLE | ||
| Save-InforcerReportOutput -RunId 8842 -OutputId 1 -OutputPath ./out -OutputType JsonObject |
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
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.
Two open audit items, both pre-existing on
main, neither touching the public surface.A4 — API validation errors lost the reason that explained them
When an entry in the API's
errors[]array matches no known shape, the module falls back todumping it as JSON so something actionable still reaches the user. It dumped at
-Depth 5.That fallback runs precisely when the entry's shape is unknown, so there was no basis for
assuming the useful field sat above the cut. Past the limit PowerShell doesn't error — it
substitutes
@{...}, which isn't JSON, writes a truncation warning into the caller's stream,and drops the rest:
{"reason":"tenant not in key scope"}
→ {"L8":{"L7":{"L6":{"L5":{"L4":{"L3":"@{L2=}"}}}}}}
Now
-Depth 100, matching every otherConvertTo-Jsonin the module.The audit item had proposed the opposite fix — amend the contract to allow a shallow depth for
error rendering. Checking what truncation actually produces settled it the other way. Swept the
rest while here: all 30
ConvertTo-Jsoncalls inmodule/Publicandmodule/Privateare nowDepth 100, and the only deliberate shallow one is the documented
Format.ps1xmlPolicyDatapreview at Depth 3.
A3 —
Get-Helpnever showed the JSON pathGet-InforcerReportRun,Get-InforcerReportType,Get-InforcerUser,Invoke-InforcerAssessment,Invoke-InforcerReportandSave-InforcerReportOutputall documented-OutputTypebut neverdemonstrated it, so
Get-Help <cmdlet> -Examplesgave no hint that JSON output existed.docs/CMDLET-REFERENCE.mdhad covered all six for a while — only the in-shell help was short,which is why this read as a wider docs gap than it was.
Each example follows its own file's indentation and blank-line convention (two styles are in use
across
module/Public), soGet-Helprenders them identically to the examples already there.Guarded by a discovery-time scan of the source rather than a hardcoded list: every cmdlet whose
ValidateSetoffersJsonObjectis checked, so a new one inherits the test for free. 15 today.Verification
Both new tests are mutation-verified — reverting each fix fails exactly the intended test and
nothing else. 467 passed, 0 failed, 2 skipped. Zero ScriptAnalyzer errors.