Skip to content

Allow explicitly targeted rules in build files - #653

Merged
Giulia Stocco (gfs) merged 1 commit into
mainfrom
gfs-fix-build-file-tag-filter
Aug 3, 2026
Merged

Allow explicitly targeted rules in build files#653
Giulia Stocco (gfs) merged 1 commit into
mainfrom
gfs-fix-build-file-tag-filter

Conversation

@gfs

Copy link
Copy Markdown
Contributor

Summary

  • Factor the existing universal-rule predicate into Rule.IsUniversal and reuse it from rule selection and both processing paths.
  • Preserve build-file noise suppression for universal rules, while allowing rules that explicitly declare applies_to or applies_to_file_regex to emit non-Metadata tags without -A.
  • Clarify --allow-all-tags-in-build-files, add synchronous and asynchronous regression coverage, and remove three now-unnecessary test workarounds.

Background

The build-file filter was intended to prevent generic feature rules from producing incidental findings in build/configuration files. Because it applied to every matching rule, it also suppressed rules deliberately scoped to build languages or filenames, making 22 default rules silent unless users supplied -A.

The project's XML tests were also working around this behavior with AllowAllTagsInBuildFiles = true; the overrides are now removed where those rules explicitly target pom.xml, AndroidManifest.xml, or .config files. Universal JSON/XML tests retain the option.

Revived default rules

  • CI: AI002100 TravisCI, AI002200 CircleCI, AI002300 Azure Pipelines, AI017300 Bamboo
  • Build tools: AI016800 Maven, AI016900 Ant, AI017000 Gradle, AI017100 Jenkins, AI017200 SBT, AI017600 Ivy, AI017700 Leiningen, AI017800 VisualStudio
  • Platforms: AI028500 NETCore, AI028600 NETStandard, AI028700 Mono
  • AI libraries: AI070003 GenerativeAI, AI070302 MCP
  • Other explicitly targeted rules: AI002600 Azure DataStorage, AI011700 Oracle SQL, AI031565 Hashicorp Packer, AI034700 OS ACL Write Unsafe, AI041800 Ava

Behavior change

Scans can now report additional tags from rules that explicitly target build-type files. Universal rules remain filtered in build files by default, so the original anti-noise behavior is preserved; -A only opts universal rules into non-Metadata findings.

Scanning this repository with the old and fixed engines against the same source tree using --no-file-metadata changed uniqueTags from 76 to 79. No tags were removed; these were added:

  • CloudServices.Code.CI.Microsoft.Azure (AI002300)
  • Development.Build.VisualStudio (AI017800)
  • Platform.Microsoft.NETStandard (AI028600)

Validation

  • dotnet build succeeded.
  • The .travis.yml / pom.xml / .csproj / build.gradle repro emits CloudServices.Code.CI.TravisCI, Development.Build.Maven, and Platform.Microsoft.NETStandard without -A; the universal Application.Target.Framework tag remains suppressed.
  • dotnet test AppInspector.Tests/AppInspector.Tests.csproj -f net9.0: 351 passed, 0 failed.

PR #651 ("Add WebApp.API ruleset for detecting exposed HTTP/REST APIs") depends on this fix and will be rebased onto this branch.

Allow explicitly targeted rules to report build-file findings while preserving suppression for universal rules.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f0f9dbe0-e8ba-472b-b645-eda10038e683
@gfs

Copy link
Copy Markdown
Contributor Author

SDL Security Review

🤖 Automated COMPASS SDL review. Generated by the COMPASS sdl-security-review skill against the changed lines in this PR. This is not a human review — treat it as advisory input, not a substitute for maintainer sign-off.

Scope: 7 files, +111/−12. Review covers changed lines only in application source; test and doc-comment changes noted for completeness.

Findings

  • 🔴 Blocking: none
  • 🟡 Warning: none

Per-file notes

AppInspector.RulesEngine/Rule.cs — new IsUniversal property

[JsonIgnore]
public bool IsUniversal =>
    (FileRegexes is null || FileRegexes.Count == 0) &&
    (AppliesTo is null || AppliesTo.Count == 0);

Get-only and marked [JsonIgnore], so it cannot be populated from rule JSON and is not emitted by packrules or other serialized rule output. This is the security-relevant detail: IsUniversal now participates in a filtering decision, so if it were settable from deserialized rule content, a malformed or attacker-supplied third-party ruleset could flip the build-file suppression behavior and either mask findings or force noise into scan results. As written the value derives solely from AppliesTo/FileRegexes, closing that path.

No regex is constructed here — the predicate only inspects collection counts, so no ReDoS surface is added.

AppInspector.RulesEngine/AbstractRuleSet.csGetUniversalRules() refactor

The extracted predicate is logically identical to the inline expression it replaces (null-or-empty on both FileRegexes and AppliesTo). No change to which rules are classified as universal, so no behavioral drift for existing callers.

AppInspector.RulesEngine/RuleProcessor.cs — build-file suppression, sync + async paths

if (!_opts.AllowAllTagsInBuildFiles &&
    languageInfo.Type == LanguageInfo.LangFileType.Build &&
    oatRule.AppInspectorRule.IsUniversal &&
    (oatRule.AppInspectorRule.Tags?.Any(v => !v.Contains("Metadata")) ?? false))

The filter is narrowed, never widened — it now suppresses strictly fewer findings than before. For a security-characterization tool this reduces false negatives rather than introducing them; AI034700 (OS ACL Write Unsafe) and the CI/cloud/build-platform rules were previously silent in build files unless the user passed -A.

Both call sites were updated consistently (AnalyzeFile and the ProcessBoundary async path). Worth calling out explicitly: updating only one side would leave synchronous and asynchronous scans reporting different results, and divergence between scan code paths is a recurring source of missed findings in analysis tooling.

No injection, deserialization, path-handling, network, cryptographic, or memory-safety surface is touched by this change.

AppInspector.CLI/CLICmdOptions.cs, AppInspector/Commands/AnalyzeCommand.cs

Help text and XML doc comment only. No security impact.

AppInspector.Tests/RuleProcessor/BuildFileRuleTests.cs, XmlAndJsonTests.cs

Test code only. No hardcoded credentials or secrets in fixtures; the sample payload is an inert JSON marker string. Removing AllowAllTagsInBuildFiles = true from the three XML tests makes them assert against the default configuration, which is the correct posture for tests covering a suppression default.

Informational (non-security)

Consumers that gate CI on Application Inspector output — unique tag counts, --fail-fast, or exit-code thresholds — may observe additional tags after this change. Per the PR description the change is additive (three tags added, none removed), but it can still alter downstream gate outcomes.

Verdict

APPROVE — no blocking or warning-severity SDL issues in the changed lines.

@gfs
Giulia Stocco (gfs) merged commit 5ce863b into main Aug 3, 2026
15 checks passed
@gfs
Giulia Stocco (gfs) deleted the gfs-fix-build-file-tag-filter branch August 3, 2026 17:49
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.

2 participants