fix(control-testing): a control test could be created but never tested - #360
Merged
Conversation
The Control Testing page imports useCreateControlTest and nothing else.
useRecordControlTestResult and useDeleteControlTest exist in
use-simulation-api.ts, and the API has PATCH /control-tests/{id}/result behind
validation:write — the page just never called it. The row rendered status and
last_tested_at and offered no action.
So a control test created through this page stayed 'untested' forever,
last_tested_at read 'Never', and the page's own passed/failed/untested summary
was permanently 100% untested. control_tests has 0 rows on the live database.
Recording effectiveness over time is the entire point of the feature.
Adds an Actions column with a Record-result dialog: outcome, evidence, notes.
It shows the control's expected_result while the tester chooses, since that is
what they are comparing against and it was otherwise only visible in the create
form. Gated with <Can permission={Permission.PentestWrite}>, matching the
server's middleware.Require(permission.PentestWrite) — both resolve to
"validation:write", checked rather than assumed.
The vocabulary moves to features/simulation/vocabulary.ts, mirroring
features/controls/vocabulary.ts, which exists because the compensating-controls
form offered Preventive/Detective/Corrective/Compensating while the database
accepted segmentation/identity/runtime/detection/other — zero overlap, every
create a 500.
Control tests fail more quietly than that: control_tests has no CHECK
constraint, so before api#417 the server stored whatever string arrived. A
near-miss like 'passed' persisted and then matched neither `status === 'pass'`
nor `'fail'` in this page's summary — a control that HAD been tested read as
neither passed nor failed, with no error anywhere.
The tests assert the offered values against the backend list, that every
backend status except 'untested' is reachable (a status with no way to record
it is one the product claims and cannot deliver), and that no near-miss is
offered. Verified they fail by changing 'pass' to 'passed': "the form offers
"passed", which the API rejects".
'untested' is deliberately not offered: it is the initial state, not an
outcome. Recording it would stamp last_tested_at while setting the status back
to untested, which reads as "tested, result: not tested".
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.
The Control Testing page imports
useCreateControlTestand nothing else.useRecordControlTestResultanduseDeleteControlTestsit inuse-simulation-api.ts, and the API has hadPATCH /control-tests/{id}/resultbehindvalidation:writeall along. The page just never called it. The row renderedstatusandlast_tested_atand offered no action.So a control test created through this page:
untested, foreverNever, forevercontrol_testshas 0 rows on the live database. Recording effectiveness over time is the entire point of the feature.The change
An Actions column with a Record-result dialog — outcome, evidence, notes.
It shows the controls
expected_resultwhile the tester picks an outcome, since that is exactly what they are comparing against and it was otherwise only visible in the create form.Gated with
<Can permission={Permission.PentestWrite}>, matching the serversmiddleware.Require(permission.PentestWrite). Both resolve to"validation:write"— checked, not assumed; a UI gate that disagrees with the servers either hides a button that would work or shows one that 403s.Why the vocabulary moved to its own module
Mirrors
features/controls/vocabulary.ts, which exists because the compensating-controls form offered Preventive/Detective/Corrective/Compensating while the database acceptedsegmentation/identity/runtime/detection/other— zero overlap, every create a 500.Control tests fail more quietly than that.
control_testshas no CHECK constraint, so before api#417 the server stored whatever string arrived. A near-miss likepassedpersisted happily and then matched neitherstatus === passnorfailin this pages summary — a control that had been tested read as neither passed nor failed, with no error anywhere. Quieter is worse.The tests assert:
untestedis reachable — a status the API accepts with no way to record it is one the product claims and cannot deliverPass,passed,PARTIAL,not-applicable) is offerednot_applicablein a dropdown is a leaked implementation detail)Verified the tests fail by changing
passtopassed:untestedis deliberately not offered: it is the initial state, not an outcome. Recording it would stamplast_tested_atwhile setting the status back to untested — "tested, result: not tested".Verification
tsc --noEmit— cleaneslinton the changed page and the new module — cleanvitest run— 902 tests, 56 files, all passingPairs with api#417, which stops an unknown status being stored at all. Either half is useful alone; together the value can only be set from a fixed list and only that list can be stored.