Skip to content

[WIP] Add boolean-expression rules for ApplicationInspector 1.10.2 - #786

Closed
Giulia Stocco (gfs) with Copilot wants to merge 1 commit into
mainfrom
copilot/add-boolean-expression-rules
Closed

Giulia Stocco (gfs) with Copilot wants to merge 1 commit into
mainfrom
copilot/add-boolean-expression-rules

Conversation

Copilot AI commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.


This section details on the original issue you should resolve

<issue_title>Add boolean-expression rules now that ApplicationInspector 1.10.2 fixes the verifier</issue_title>
<issue_description>Follow-up to #780, which closed a set of SDL coverage gaps but deliberately left out the boolean-expression rules.

Why this was deferred

ApplicationInspector 1.10.1 shipped boolean expression support, but a rule supplying both expression and conditions behaved correctly at scan time while the rule verifier reported its must-not-match samples as failures. DefaultRulesTests.ValidateDefaultRules runs that verifier, so a correct rule could not merge, and writing rules that break CI was not worth doing.

That is fixed in ApplicationInspector 1.10.2 by ApplicationInspector#656, which moves the capture reduction into a shared CaptureFilter so the verifier and the analyzer answer the same question. 1.10.2 is published.

Work

Branch gfs-sdl-boolean-expression-rules is pushed and stacked on #780. It contains HANDOFF-boolean-expression-rules.md with the full detail; summary:

  1. Bump Microsoft.CST.ApplicationInspector.RulesEngine and ...Logging from 1.10.1 to 1.10.2.
  2. Confirm the verifier fix is real before building on it. The handoff note gives a rule that fails under 1.10.1 and should pass under 1.10.2. If it still fails, stop.
  3. Apply the merged DS440016, given in full in the handoff. It collapses the two same-ID entries in tls_generic.json into one rule via (curlFlag AND NOT tls13) OR p0 OR ..., retiring a duplicate ID. This was built and reviewed already; it just had nowhere to live.
  4. Write the soundness-gap rules. The old shape can only express P AND NOT (C0 OR C1), meaning "no mitigation at all", where cookie flags and HSTS need P AND NOT (C0 AND C1), meaning "any required mitigation missing". The partially hardened case is what is currently missed: a cookie with Secure but no HttpOnly is a real finding DevSkim cannot report.
  5. Consider tightening DS132784 (Java XXE) from file scope to factory scope, which would justify raising it above ManualReview.
  6. Changelog entry, and delete the handoff file.

Note on environment

This needs to run somewhere with access to the package feed. The machine the original work was done on could reach neither nuget.org nor the configured Azure DevOps feed, which is why 1.10.2 could not simply be picked up there. Codespaces or any environment with normal restore access is fine.

Worth knowing before starting

Two findings from the original investigation that are easy to lose time on:

  • Boolean operators over pattern labels alone are nearly useless. A finding comes from exactly one pattern, so at any finding every other pattern label is false. a AND b is rejected outright, a AND NOT b is identical to plain a, and a XOR b is identical to a OR b. All the real expressiveness needs patterns and conditions in the same expression.
  • When testing with ApplicationInspector's own CLI, filter SARIF results by rule ID. Its output includes findings from its built-in rules, and ordinary test content like curl http://x trips several. "The SARIF has results" is not evidence your rule matched.

The handoff file also covers the hand-maintained <EmbeddedResource> allowlist, expression syntax constraints, and the nbgv-derived changelog heading.</issue_description>

Comments on the Issue (you are Copilot in this section)

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add boolean-expression rules now that ApplicationInspector 1.10.2 fixes the verifier

2 participants