v2026.09.25.001: engine results reach the payload and the score - #2
Merged
Merged
Conversation
Invoke-SafeBlock runs its block with & $Block, which is a child scope. The Assessment Engine set $avProduct, $edrProduct, $defStatus, $bitlockerWarn, $osEolWarn and $wuLastWarn with bare assignments, so each one made a local that was discarded when the block returned. Since v1.002 the payload has reported antivirus 'NONE DETECTED', edr 'None detected' and defender 'Unknown' on every device, and the BitLocker, OS EOL and Windows Update penalties have never applied. MachineInfo, built inside the block, was right throughout. The payload now reads antivirus, edr and defender from MachineInfo, so they are null when the engine did not run. The three warn flags are $Script:-scoped. The script-level defaults are gone, and $avProduct is assigned on every branch. Scoring: BitLocker off (-15), an end-of-life build (-20) and no Windows Update install in over 30 days (-15) now apply for the first time. Each flag is set only by a positive detection. The Defender DISABLED rule (-20) is removed rather than switched on. Live, it would take 20 from every box whose third-party AV turns Defender off, and on a box with no AV it would count twice alongside the -25 rule. The same bug kept two Persistence Engine counters at zero, so their "none found" summary lines appeared even after a removal. This is log text only. tests/Test-EngineScope.ps1 runs Phase 2, the scoring and the payload fields verbatim under StrictMode 2 with mocks, across 13 scenarios. It also AST-checks the whole script for any variable set bare inside an Invoke-SafeBlock and read outside it. It fails 41 assertions against v2026.09.24.001 and passes here. It is a mock test, not a Windows run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
This PR is stacked on #1. Its base is
fix/checkin-identity-utf8, so the diff shows only this change.main. If v2026.09.24.001: device_id survives the engine; report sent as UTF-8 #1 is squash-merged, rebase this branch ontomainbefore merging it.The bug
Invoke-SafeBlockruns its block with& $Block, which is a child scope.$avProduct,$edrProduct,$defStatus,$bitlockerWarn,$osEolWarnand$wuLastWarn. Each assignment made a local copy that was discarded when the block returned.antivirus: "NONE DETECTED",edr: "None detected"anddefender: "Unknown";MachineInfois built inside the block, so it and the log had the real values all along.Changes
antivirus,edranddefendernow read$Script:MachineInfo[...], like every other machine field. When the engine did not run, they are null instead of a default reported as fact.$Script:BitLockerWarn,$Script:OsEolWarnand$Script:WuLastWarnare set with$Script:inside the engine and read with it in the scoring.$avProduct,$edrProductand$defStatusare now local to the engine, so nothing outside can read a stale copy.$avProductgets an explicitelse, because an unset local would throw in theMachineInfoliteral under StrictMode.$runKeysRemovedand$policyRemoved, so "no malware Run keys found" and "no browser policy hijacks found" were logged even after a removal. These are now$Script:counters. The effect is on log text only.Scoring: what changes when this ships
$Script:HasActiveAv, already$Script:)bitlockerisNot availableand there is no penalty.defenderreadsDISABLED. Live, the rule would take 20 points from the boxes with the most protection.os_eolstrings. See the follow-ups.What it does in Battlefield
fleet.pyattn), the A/B compliant count, the dashboard average and the score trend.conditions_frominbf/alerts.pyraises alerts from High findings, the IOC count, agent health and failed actions. It never readsantivirus,edr,defenderor the score. The BitLocker finding was already sent (Medium) because it is added inside the engine, so no new alerts or High-finding emails come from this change.LATEST_SK_VERSIONinbattlefield.envwhen this ships.Predicting it from production before merging
This query is read-only and has not been executed; I had no database here.
os_eol,bitlockerandlast_wu_installhave always been reported correctly, so the latest stored run has what it needs. The day count is as of that run, so the Windows Update column is a lower bound.For a before/after grade table, group the final
SELECTbygrade, new_grade.Tests
New
tests/Test-EngineScope.ps1, in the style ofTest-DeviceIdentity.ps1:antivirus,edr,defenderandbitlockervalues, the exact score, and the BitLocker finding.Invoke-SafeBlockis read from somewhere that block does not enclose. That is this bug in general form, and it is how the two Persistence Engine counters were found.Results:
$avProductelse.Test-BiosDateandTest-DeviceIdentitystill pass.Before merging: one real run
Run this in an elevated Windows PowerShell 5.1. Pick a machine with something to see: BitLocker off, a stale Windows Update, or a third-party AV/EDR. RLG-JANE-PC or RLG-DCFS also cover #1.
When it finishes, check the newest
C:\ProgramData\ShellKnight\Logs\ShellKnight_*.log:Assessment Engine skippedand noDevice identity skippedline.antivirus,edranddefenderin the newestC:\ProgramData\ShellKnight\JSON\*.json.SECURITY GRADEreflects any BitLocker, EOL or Windows Update warning logged above it.Battlefield push OK - run_id: <n>.In Battlefield, the device should show a v2026.09.25.001 run with real AV and EDR pills. Then remove the test task with
schtasks /delete /tn SK-Test /f.Left alone (follow-ups)
os_eolstrings.Password minimum length is 0 (CIS 1.1.1), which becomes a Battlefield alert. That is a collection failure scored as a vulnerability, the case ADR 0009 rules out. The test pins the current -20 for those scenarios.🤖 Generated with Claude Code