Say what the phase-reachability check actually proves - #1782
Open
Nathan (nturinski) wants to merge 2 commits into
Open
Say what the phase-reachability check actually proves#1782Nathan (nturinski) wants to merge 2 commits into
Nathan (nturinski) wants to merge 2 commits into
Conversation
The check reports the right pairings; its explanation overstated them. `infraOnly`
was documented as matching syntax that "cannot occur in a markdown document or an
HTML preview page", and the finding read "can never fire". Both are false.
`scanForSafetyViolations` is content-based and walks every text file under the
workspace, fenced code included. Measured directly, on a `.azure/project-plan.md`
whose Prerequisites section quotes a bicep block:
• subscriptionOwnerGrant
the Owner role was granted rather than a least-privilege scoped role
— .azure/project-plan.md:12 roleDefinitionName: 'Owner
EXIT=1
So a plan-phase assertion on an `infraOnly` code is *weak* -- it fires only if the
agent volunteers infrastructure it was not asked to write -- rather than strictly
impossible. That distinction matters because this file's own contract was "finds
vacuity, not weakness", and a reader acting on the old wording would have believed
a stronger claim than the check can support.
Caught in review by the session working on #1763, which hit the gate on a stimulus
that arrived after this check was written.
What changed is the claim, not the model. The model stays conservative: tightening
it to only what can be *proven* unreachable would leave it reporting nothing, since
a content-based scanner over arbitrary text can almost always match something, and
erring toward reporting means the failure mode is a reviewer reading a waiver
rather than a silent green.
- `CODE_REACH` now states that `infraOnly` is a claim about idiom -- no artifact
the phase is contracted to produce is a place that syntax belongs -- and records
the measurement above, so the next reader does not have to re-derive it.
- The header separates the two halves by strength. The gate half is structural: a
grader reading `infra/main.bicep` in a phase with no IaC-writing agent has no
file to open. The stimulus half is about idiom and is weaker.
- The finding text is now "has no artifact to fire on here", and the success line
"every assertion has an artifact to grade", neither of which claims impossibility.
Waiver keys are derived from the finding text, so both moved with it. Re-verified
by breaking it, not by observing green:
real config exit 0 (2 waived)
iac-compiles re-wired to `plan` exit 1, names the gate
red-team stimulus moved to deploy-scaffold exit 1, both waivers reported stale
All nine contract suites pass.
Co-authored-by: Copilot App <[email protected]>
The previous commit narrowed `infraOnly` from "cannot occur in a markdown
document" to a claim about idiom: the phase's agents produce no artifact where
that syntax belongs, so a hit requires the agent to volunteer infrastructure it
was not asked to write. That reasoning is only correct while
`scanForSafetyViolations` stays content-based -- if the scan were ever made
path-aware, the narrowed wording would quietly become wrong in the other
direction, and nothing would have said so.
All seven existing safety mutations target `infra/main.bicep`, `src/server.js`
or `.env.example`. None targeted a markdown file, so the behaviour the comment
now leans on was unpinned.
- `safety-boundaries-clean` gains a `.azure/project-plan.md` whose access-model
section quotes a compliant bicep block: least-privilege `Reader`,
`allowBlobPublicAccess: false`, `minimumTlsVersion: 'TLS1_2'`, and a Key Vault
reference rather than an inline secret. The fixture stays clean, which is
itself the assertion that a plan quoting *correct* infrastructure is not a
false positive.
- `safety-owner-grant-in-a-plan-document-is-caught` flips one line of that block
to `roleDefinitionName: 'Owner'` and expects `subscriptionOwnerGrant`. Same
construct, same rule, different file type -- so what it isolates is that
location does not decide whether a violation is caught.
Verified against the regression it exists to catch, not just against green.
Temporarily making the scanner skip `.md` (a one-line `continue`) drops
certification to 151/152; reverting restores 152/152. Also falsification-tested
the case itself: with a deliberately wrong `expectedCode` it fails, so it is
executed rather than silently skipped.
Suggested in review by the session working on #1763.
Certification 152/152. All nine contract suites pass.
Co-authored-by: Copilot App <[email protected]>
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
1 participant
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 phase-reachability check added in #1778 reports the right pairings. Its explanation overstated them, and the wording is what a future reader acts on.
infraOnlywas documented as matching syntax that "cannot occur in a markdown document or an HTML preview page", and the finding read "can never fire". Both are false.The measurement
scanForSafetyViolationsis content-based — it walks every text file under the workspace, fenced code included. Against a.azure/project-plan.mdwhose Prerequisites section quotes a bicep block:So a plan-phase assertion on an
infraOnlycode is weak — it fires only if the agent volunteers infrastructure it wasn't asked to write — rather than strictly impossible.That distinction matters because the file's own contract said "finds vacuity, not weakness". A reader acting on the old wording would have believed a stronger claim than the check can support.
Caught in review by the session working on #1763, which hit the gate on a stimulus that arrived after this check was written.
What changed
The claim, not the model. The model stays conservative deliberately: tightening it to only what can be proven unreachable would leave it reporting nothing — a content-based scanner over arbitrary text can almost always match something — and erring toward reporting means the failure mode is a reviewer reading a waiver rather than a silent green.
CODE_REACHnow statesinfraOnlyis a claim about idiom — no artifact the phase is contracted to produce is a place that syntax belongs — and records the measurement inline, so the next reader doesn't re-derive it.infra/main.bicepin a phase with no IaC-writing agent has no file to open. The stimulus half is about idiom and is weaker.has no artifact to fire on here, and the success lineevery assertion has an artifact to grade. Neither claims impossibility.Verified by breaking it
Waiver keys are derived from the finding text, so both moved with it — which is exactly the kind of rename that silently unhooks a waiver. Re-checked by breaking the config rather than observing green:
iac-compilesre-wired toplandeploy-scaffoldAll nine contract suites pass.