Close SDL coverage gaps and fix rules that could never match - #780
Open
Giulia Stocco (gfs) wants to merge 15 commits into
Open
Close SDL coverage gaps and fix rules that could never match#780Giulia Stocco (gfs) wants to merge 15 commits into
Giulia Stocco (gfs) wants to merge 15 commits into
Conversation
android.json and xslt_scripting.json were absent from the explicit <EmbeddedResource> allowlist in Microsoft.DevSkim.csproj, which is hand-maintained rather than a wildcard. DS180000, DS180001, DS180002 and DS132781 therefore parsed and validated on disk but were not part of DevSkimRuleSet.GetDefaultRuleSet(), so they never ran for any user. This is also why the dangling rule_info references in those two files never failed CI: DefaultRulesTests iterates the shipped set, so it never saw them. The missing guidance is the likely reason the rules were held back originally. Add both files to the allowlist and fix what that surfaces: - Write the four missing guidance documents. - Point DS180001 and DS180002 at their own guidance. They were shifted by one, referencing DS180000.md and DS180001.md respectively. - Correct DS132781's applies_to from "CSharp" to "csharp". Language names are matched by exact string against languages.json, which defines "csharp"; verified that the rule reports nothing until this is fixed. - Correct DS180000's XPath. It bound the default namespace to the Maven POM namespace and matched //default:application, but AndroidManifest.xml has no default namespace, so the rule could not fire on a real manifest. Its self-test used a manifest with xmlns set to the Maven namespace, so it passed while testing something that does not occur in practice. Bind only the android prefix, match //application, and replace the self-tests with a realistic manifest plus a negative case for an absent attribute. Shipped rule count moves from 123 to 127 and no rule now references missing guidance. Verified by scanning a real AndroidManifest.xml, a Java source file and a C# source file with no -r argument, so the matches come from the embedded default ruleset. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
Two unrelated rules shared the ID DS440011: "OpenSSL: Hard-coded SSL/TLS Protocol" in TLS/tls_generic.json, whose guidance is DS440001.md, and "OpenSSL: Do not hardcode SSL/TLS versions within an application." in cryptography/hardcoded_tls.json, whose guidance is DS440000.md. SARIF emits one tool.driver.rules entry per rule ID, so the collision left a single entry carrying only the first rule's metadata. Findings from the hardcoded_tls.json rule were reported with the other rule's name and a helpUri pointing at DS440001.md, sending readers to the wrong guidance document. Suppressing either ID also suppressed the other. Renumber the hardcoded_tls.json rule to DS440017, which now emits its own SARIF entry linking to DS440000.md. The other repeated IDs are deliberate and are left alone. DS440016 (x2) and DS148264 (x3) are each one logical rule split across entries, by condition presence and by target language respectively. Every copy shares a name and a guidance file, so the single SARIF entry describes them correctly, and the shared ID means one suppression directive covers the whole rule. Splitting those would regress suppression without fixing anything. Also normalise DS450003's severity from "manualreview" to "ManualReview" for consistency with the other 44 manual-review rules. Enum parsing is case-insensitive, so this is a readability change rather than a fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
ADM.10082 enumerates 194 banned APIs and names DevSkim as a detection tool for it. Comparing that list against the shipped rules found 113 that DevSkim did not report: 108 absent outright and 5 present only in the wrong case. Since the catch-all DS154189 uses RegexWord, which is case-sensitive, StrCpy and StrCatBuff were not matched by the lowercase lstrcpy and strcatbuff entries already in the alternation. Extend DS154189 from 80 to 178 alternatives, covering the Windows string helpers (StrCpy*, StrCat*, StrNCpy*, StrNCat*, lstr* variants), the _mbs* and _t* variants, sprintfA/W and wsprintfA/W, the makepath and splitpath families, the itoa family, CharToOem and OemToChar, and the remaining scanf variants. The alternation is now sorted case-insensitively so future additions land in a predictable place. Four groups are given their own IDs rather than being folded into the catch-all, because each has a distinct rationale and a different useful severity, and because a separate ID is what lets a team suppress one class without losing the rest: - DS154190, the IsBad*Ptr family. Reported at important because these cannot work as documented: probing a stack guard page disarms it, and a wrong-but-mapped pointer is reported as valid. Includes the A/W exports, which is what real code calls. - DS154191, CopyMemory and RtlCopyMemory. Restricted to the Windows aliases because bare memcpy is already reported by DS121708, which also carries a fix-it; adding it again would double-report. - DS154192, the unbounded length family. strlen was missing entirely. wcslen and _tcslen move here from DS154189 so that the same defect is not reported at two severities depending on which variant is used. BestPractice, since strlen on a terminated buffer is fine. - DS154193, class_addMethod and class_replaceMethod. Objective-C only, ManualReview, because swizzling is a design decision rather than a defect. Each new rule has guidance and positive and negative self-tests. No overrides are declared: none of these names remain in DS154189, so an override would suppress nothing while adding a relationship that has to be reasoned about later. Verified by scanning a fixture containing all 113 previously-missed names. All 113 are now reported, no line draws more than one rule, and a fixture of the safe replacements (strcpy_s, memcpy_s, strnlen_s, snprintf_s) produces no findings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
The five shipped deserialization rules cover pickle, Java readObject, PHP unserialize, Ruby Marshal, and Newtonsoft TypeNameHandling. ADM.10010 names several more as unapproved or unsafe, none of which were detected. Add rules for the ones the requirement calls out by name: - DS425050: torch.load, joblib.load, dill.load and the marshal module. ADM.10010 lists these specifically for AI model files obtained from GitHub or Hugging Face. They all run code during load, which makes a downloaded checkpoint executable content rather than data. - DS425060: yaml.load and yaml.load_all without a safe loader. The requirement states PyYAML load cannot be used safely and that safe_load must be used instead. A negated same-line condition excuses an explicit Loader=SafeLoader, and the regex does not match yaml.safe_load, so both safe forms stay quiet. - DS425070: BinaryFormatter, SoapFormatter, NetDataContractSerializer, LosFormatter and ObjectStateFormatter. Critical, because one reachable call with attacker-controlled input is generally enough for code execution. ADM.10010 approves only System.Text.Json, XmlSerializer, DataContractSerializer, DataContractJsonSerializer, Newtonsoft.Json and protobuf, and notes that most built-in .NET serializers not on that list cannot be used safely. - DS425080: JavaScriptSerializer and SimpleTypeResolver. The requirement records that JavaScriptSerializer stopped being approved on 2023-03-10. Both names are matched so the resolver is still caught when the serializer is constructed elsewhere. - DS425090: Boost Property Tree. ADM.10010 approves Boost but singles this out for denial of service on nested payloads. ManualReview, since it is reasonable for trusted local configuration. Each has guidance and positive and negative self-tests. Verified against a Python and a C# fixture: the unsafe forms report and the approved alternatives (json.load, yaml.safe_load, an explicit SafeLoader, JsonSerializer.Deserialize, DataContractSerializer) produce nothing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
The three shipped external-entity rules only target Objective-C and Swift, matching shouldResolveExternalEntities. Every other ecosystem was uncovered, including .NET, whose safe configuration ADM.10010 spells out in its own worked example. - DS132782: DtdProcessing.Parse and the legacy ProhibitDtd = false. The inverse of the DtdProcessing.Prohibit that ADM.10010's example sets. Catches entity expansion as well as external entities, since expansion needs only DTD processing. - DS132783: XmlResolver assigned a resolver. The other half of the same example, which sets XmlResolver to null. XmlSecureResolver is included because its code-access-security restriction is not enforced on .NET Core or later, so it looks like a mitigation without being one. - DS132784: Java parser factories with no hardening anywhere in the file. Covers DocumentBuilderFactory, SAXParserFactory, XMLInputFactory, TransformerFactory and SchemaFactory, all of which resolve external entities by default. - DS132785: PHP libxml_disable_entity_loader(false) and LIBXML_NOENT. The latter is included because its name suggests it suppresses entities when it in fact substitutes them. - DS132786: lxml parsers configured with resolve_entities, load_dtd or no_network=False. On DS132784, the negated conditions are worth being explicit about. Six hardening indicators are each negated over same-file scope, which the engine evaluates as "fire when none of them is present". That is the sound direction for this question: it cannot claim a file is safe, only that it shows no evidence of hardening. A file that hardens one factory and leaves a second unhardened will not report. The rule is ManualReview for that reason, and the guidance states the limitation rather than leaving a reader to assume a clean result means every parser is safe. Verified on Java, C#, and PHP fixtures: the unhardened forms report, and adding disallow-doctype-decl, DtdProcessing.Prohibit with a null XmlResolver, or LIBXML_NONET silences them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
The engine supports ymlpaths, jsonpaths and xpaths, but no shipped rule
used ymlpaths or jsonpaths at all, so every YAML and JSON configuration
requirement in the SDL was undetectable in practice. This is not a
language gap: yaml, json and xml were already defined in languages.json.
The capability was simply unused.
Kubernetes Security Baseline (ADM.10210), eight rules, the first to use
ymlpaths. Each path is declared for the Pod, Deployment/StatefulSet/
DaemonSet/Job, and CronJob roots, and for both containers and
initContainers, since the same control appears at a different depth in
each workload kind.
DS200000 privileged: true critical
DS200001 allowPrivilegeEscalation: true important
DS200002 hostPID / hostIPC / hostNetwork important
DS200003 readOnlyRootFilesystem: false moderate
DS200004 runAsNonRoot: false important
DS200005 image unpinned moderate
DS200006 SYS_ADMIN / SYS_PTRACE / ALL added important
DS200007 hostPath volume ManualReview
Each carries a same-file condition requiring apiVersion, so the rules do
not fire on unrelated YAML that happens to share a key name.
DS200005 reports only an image with no tag or an explicit :latest, not
every image lacking a digest. Flagging all tagged images would have
contradicted the rule's own description and produced a finding on
essentially every manifest.
Package sources (ADM.10205), two rules:
DS205000 nuget.config <packageSources> with no <clear /> element, via
xpaths and a negated same-file condition. Without <clear />
the sources here are added to those inherited from machine and
user level config, which is the precondition for dependency
confusion.
DS205001 --extra-index-url and PIP_EXTRA_INDEX_URL, which the
requirement names explicitly as prohibited.
DS114352 gives the orphaned "Encryption Marked Optional" guidance a rule
at last, using jsonpaths over $.ConnectionStrings.* to find Encrypt=False,
TrustServerCertificate=True, sslmode=prefer/allow/disable, MySQL
SslMode=Preferred/None and JDBC useSSL=false. The guidance file was a TODO
stub and has been written properly.
Both new rule files are registered in the csproj EmbeddedResource
allowlist. That list is hand-maintained rather than a wildcard, and
omitting a file there is exactly what left four rules unshipped before.
Verified that all 51 rule files are embedded, with no duplicate or
dangling entries.
Verified against Pod and Deployment fixtures, a hardened manifest that
produces no findings, tagged and digest-pinned images, and nuget.config
with and without <clear />. This repository's own nuget.config is
correctly not flagged.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
Two separate gaps, both costing coverage on rules that already exist. React was defined but untargeted. languages.json already mapped .jsx and .tsx to javascriptreact and typescriptreact, but zero rules named those languages, while 7 named javascript and 5 named typescript. Because applies_to matches on the exact language name, React source received none of those rules. Adding the two names to the 7 rules that target their non-React counterparts restores coverage that was intended rather than adding anything new. Confirmed by scanning a .tsx file, which now reports DS189424 for eval and did not before. HTML did not exist as a language, which made the engine's Html pattern scope unreachable and left guidance/DS610000.md orphaned: the document for a rule nobody could write. Adding html lets that rule exist, so DS610000 now ships against guidance that was already in the repository. Its stub has been expanded to explain reverse tabnabbing rather than only naming the attribute. Also added: dockerfile, terraform, bicep, kotlin, scala, dart, toml, gradle, msbuild (.props/.targets), dotenv and makefile, with comment syntax for each so comment-scoped patterns behave. 32 rules declare no applies_to and therefore apply to every known language, so each new language extends those 32 rules to a file type DevSkim previously skipped entirely. Verified by scanning a Terraform file and a Dockerfile, both of which now report hard-coded TLS versions. The extensionless conventions needed the file-names property rather than extensions, which no DevSkim language definition used before. A file named Dockerfile has no extension, so .dockerfile alone matched nothing in practice; the same applies to .env, Makefile, Cargo.toml and the shell dotfiles. Verified that a plain Dockerfile is now scanned. Widening applies_to expands scope silently, so this was checked for regressions: the full suite passes and every existing rule's self-tests still hold. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
AccessControl.10100 forbids secrets in code. DevSkim shipped two rules
for it, both keyed on 30 or more lowercase hex characters, so anything
base64 or provider-prefixed went undetected. That excludes most modern
credential formats.
Three rules added, chosen for precision rather than coverage. The
requirement directs teams to CredScan, SPMI and GitHub secret scanning,
and DevSkim should not pretend to replace them; its value here is
catching a credential in the editor before it is ever committed. So these
match only formats with issuer-assigned prefixes and fixed structure,
which ordinary code does not produce, rather than general entropy
heuristics that belong in a dedicated scanner.
- DS173238: PEM private key blocks. Excludes public keys and
certificates.
- DS173239: GitHub, AWS, Google, Slack, Stripe, npm, SendGrid and GitLab
token formats.
- DS173240: Azure Storage AccountKey connection strings and shared access
signatures.
Added a pem language for .pem, .key, .crt and the id_rsa family of
filenames. Without it DS173238 could not fire on the files private keys
actually live in, since those have no extension and DevSkim skips file
types it does not recognise.
Also fixes a regression introduced in the previous commit. DS114352 used
jsonpaths, and any rule using jsonpaths makes the engine parse every JSON
file it scans. VS Code's .vscode/launch.json, tasks.json and
extensions.json are JSONC, so they fail a strict parse, and the engine
logs that at error level. Scanning this repository produced four such
lines, and a .vscode directory is close to universal.
DS114352 is now a plain regex. The connection string tokens it matches
are distinctive enough that path scoping added little, and dropping it
means the rule also covers web.config, C# source, .env files and YAML
rather than appsettings.json alone. No rule uses jsonpaths now, so the
JSON parse path is never entered and the repository scans with zero parse
errors.
The eight ymlpaths rules are kept. The same error-level logging applies
to unparseable YAML, but that was checked against a Helm chart, whose
{{ .Values.x }} placeholders parse as scalars and produce no errors. The
underlying issue is that the engine logs a document that does not parse
at error rather than debug level; that belongs upstream in
ApplicationInspector and is noted with the other engine findings.
Verified: the new rules fire on a private key file, a GitHub token, an
AWS key and an Azure connection string, and stay silent on
process.env.GITHUB_TOKEN and DefaultAzureCredential. Scanning DevSkim
itself produces no findings outside the rules' own example strings in
secrets.json, which the pre-existing DS117838 already did.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
32 rules shipped with no must-match test, so nothing verified they could match anything at all. Writing those tests found three rules and patterns that never fired, in two cases because of a defect in the rule itself. - DS191340 (sensitive data in NSUserDefaults) used $1 as a backreference. .NET spells that \1; $ is an end-of-line anchor, so the pattern required end-of-line followed by "1" and could never match any input. Corrected to \1, and the rule now reports a password written to NSUserDefaults. - DS440016's --secure-protocol= pattern was typed as "string", which applies word boundaries. A leading hyphen is not a word character, so the boundary could never be satisfied and wget --secure-protocol=SSLv3 went unreported. Changed to "substring". - DS440060 (Node hard-coded TLS) had an empty patterns array and an explicit comment saying it was encompassed by DS440000 and DS440010. A rule with no patterns cannot produce a finding, but still occupies an entry in SARIF tool metadata, so it is removed rather than left as a rule that appears to exist. Writing the negative tests was also informative. Several TLS rules report TLS 1.3 as readily as TLS 1.0, because they match any hard-coded version rather than any weak one. That is consistent with their stated intent, "do not hard-code TLS protocol versions", so the rules are left alone and the must-not-match samples use code that pins no version at all, which is the behaviour actually being recommended. Every rule now has a positive self-test. must-not-match is left where it would only restate the positive case; it is most valuable where a rule has a guard condition, and those all have one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
#654 ApplicationInspector PR #654 unifies override suppression on containment. DevSkim uses the sync path, which in 1.9.50 suppresses when the overridden match merely *starts* inside the overriding one (RuleProcessor.cs:245), while the async path already requires containment (line 443). Adopting the PR therefore makes DevSkim's path stricter, and findings suppressed today can reappear. 38 DevSkim rules use overrides; no rule shipped with ApplicationInspector does, which is why the PR could not observe this. Measuring every override pair found two that regress, both DS440016 over DS440000, and both traceable to a defect in DS440016 rather than to the engine change: --(sslv2|sslv3|tlsv1|tlsv11|tlsv1\.1|tlsv1\.2) .NET alternation is leftmost-first, not longest-match, so tlsv1 always wins and the tlsv11, tlsv1\.1 and tlsv1\.2 branches are unreachable. On curl --tlsv1.1 the rule reported --tlsv1 rather than --tlsv1.1, and the truncated span is why DS440000's match of tlsv1.1 extended past it. Reordering longest-first fixes the reported span and the containment relationship together. Likewise --secure-protocol= matched only the flag name, leaving DS440000's match of the value outside it; it now covers the value. Separately, DS140021 (strlen) overrode DS154189, but the banned API commit moved strlen into DS154192, so the override became inert and strlen(s) reported twice at the same severity. Repointed at DS154192. DS140021 keeps precedence because it matches a complete call and carries fix-its. Verified strlen(s) now reports once and wcslen(w) still reports via DS154192. Re-measured across all 38 override pairs after these fixes: regressions 0 (was 2) contained 18 inert 15 (the overridden rule never matches the same input) The inert group is mostly the dedicated banned-API rules over DS154189 and the platform TLS rules over DS440000, where the general rule does not match what the specific one matches, so the override does nothing under either semantics. No DevSkim override relationship now regresses under containment. This was measured by comparing match spans under 1.9.50 rather than by building against the PR branch, since nuget.org is unreachable here, so a build-and-diff run is still worth doing before #654 merges. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
Heading is 1.0.108, which is the git height this commit produces, rather than the 1.0.107 that HEAD produced before it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
1.10.1 is the first release carrying the boolean expression support from
ApplicationInspector#654, along with four rules-engine bug fixes: string
patterns previously always reported pattern index 0, conditions were only
judged against the first capture, the async path over-reported by taking a
union rather than an intersection, and override suppression used different
overlap rules on the sync and async paths.
That last one matters here. 38 DevSkim rules use overrides and no rule
shipped with ApplicationInspector does, so DevSkim is the first consumer
to exercise it. The sync path DevSkim uses previously suppressed when the
overridden match merely started inside the overriding one and now requires
containment. The preceding commit already brought every DevSkim override
pair into containment, so this bump lands on a ruleset that is ready for
it.
Also makes DS440017's self-test robust. Its condition looks for "openssl"
in code scope, but the sample I wrote put it in a /* */ comment:
/* openssl configuration */
SSL_CTX_set_cipher_list(ctx, "AES256-SHA256");
That passed under 1.9.50 only because the verifier's fallback language has
no comment styling, so the comment was treated as code. Under 1.10.1 it is
correctly recognised as a comment and the condition no longer matches. The
sample now puts openssl in a string literal, which is code under any
language and does not depend on which fallback the verifier picks.
Validation. This environment cannot reach nuget.org and cannot
authenticate to the configured feed, so the published package could not be
restored. Instead ApplicationInspector was built from source at the v1.10.1
tag (b1bbf91, the #654 merge commit) together with OAT 1.2.95 built from
its own source, with no dependency downgraded, and its 88 expression tests
pass. The whole DevSkim ruleset was then run through that engine's
verifyrules: after the DS440017 fix there are no self-test failures. The
remaining messages are artifacts of running DevSkim rules through
ApplicationInspector's CLI, which does not know DevSkim's languages
(batch, dotenv, cobol, packages.config) and rejects the duplicate rule ids
DevSkim uses deliberately.
The ruleset now verifies clean under both 1.9.50 and 1.10.1, and the full
DevSkim suite passes at 1.9.50 with the DS440017 fix in place, so the
change is not carrying a rule regression into the upgrade.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
Heading is 1.0.110, the git height this commit produces. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
GitHub push protection flagged DS173239's must-match samples as a Slack
API token, which is the rule doing its job on its own test data. The
samples were realistic enough to look like live credentials.
Replace them with values that are unmistakably placeholders while still
exercising the same patterns: ghp_EXAMPLENOTAREALTOKEN... still satisfies
gh[pousr]_[A-Za-z0-9]{36,} and xoxb-EXAMPLE-NOT-A-REAL-TOKEN still
satisfies xox[abprs]-[0-9A-Za-z-]{10,}. The AWS sample is left alone as it
is the example key from AWS's own documentation.
Rule self-tests for credential detection should not themselves look like
credentials, since they end up in every clone and every scanner's corpus.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
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.
Why
The DevSkim ruleset is intended to cover the Microsoft SDL. Auditing the shipped rules against the 172 current SDL requirement documents found real gaps, and more importantly found that several shipped rules could never have matched anything.
The most significant finding: four rules existed in the repository but had never run for any user.
Microsoft.DevSkim.csprojembeds rule files through a hand-maintained<EmbeddedResource>allowlist rather than a wildcard, andandroid.jsonandxslt_scripting.jsonwere missing from it. The shipped set was 123 rules, not the 127 on disk. This is also why their danglingrule_inforeferences never failed CI: the tests iterate the shipped set, so they never saw them.What changed
Shipped rules go from 123 to 155, and every rule now has a positive self-test (previously 32 had none).
Rules that could never match. Writing those missing self-tests found three more dead rules mechanically, none of which were visible by reading the ruleset:
DS191340$1as a backreference. .NET spells that\1, and$is an end-of-line anchor, so the pattern required end-of-line followed by1.DS440016--secure-protocol=typed asstring, which is word-boundary anchored. A leading hyphen is not a word character.DS440060patternsarray, so it could not produce a finding while still occupying a SARIF metadata entry.DS180000AndroidManifest.xmlhas none.DS132781applies_to: ["CSharp"]; language names match exactly againstlanguages.json, which definescsharp.DS180000is worth a second look during review: it had a self-test, and the self-test passed. The sample had been written to fit the broken XPath rather than to describe a real manifest, so it locked the defect in instead of catching it.Coverage.
RegexWorddoes not match.DS154189goes from 80 to 178 alternatives, with four groups split into their own IDs where the rationale and useful severity differ.ymlpathssupport, which no shipped rule had used. Not a language gap;yaml,jsonandxmlwere already defined.javascriptreactandtypescriptreactwere defined and mapped to.jsx/.tsxbut named by no rule, so React source received none of the JS/TS rules. Thirteen languages added; because 32 rules declare noapplies_to, each addition extends those 32 rules to a previously skipped file type. Extensionless files (Dockerfile,Makefile,id_rsa) needed thefile-namesproperty, which no language definition had used.ApplicationInspector 1.10.1. Picks up the boolean expression support from ApplicationInspector#654 plus four rules-engine fixes. One of those matters here specifically: override suppression on the sync path DevSkim uses now requires full containment rather than "starts inside". 38 DevSkim rules use
overridesand no rule shipped with ApplicationInspector does, so DevSkim is the first consumer to exercise it. Every override pair was measured and brought into containment before the bump, so the upgrade lands on a ruleset that is ready for it.Worth a careful look
DS132784(Java XXE) is deliberately unsound in the safe direction. Six hardening indicators are each negated oversame-filescope, so it fires only when none is present. A file that hardens one factory and leaves a second unhardened will not report. It isManualReviewfor that reason and the guidance states the limitation rather than letting a clean result imply every parser is safe.DS440011was two unrelated rules sharing an ID, which made SARIF serve one rule's findings under the other's name andhelpUri; that one is split.DS440016andDS148264are each one logical rule split across entries and are deliberately left alone, because the shared ID is what lets a single suppression cover the whole rule.DS114352uses a plain regex rather thanjsonpathson purpose. An earlier revision usedjsonpaths, which makes the engine parse every JSON file it scans;.vscode/*.jsonis JSONC and fails a strict parse, so a near-universal repository layout produced error-level log output. No rule usesjsonpathsnow.Verification
Full suite passes on net8.0, net9.0 and net10.0. Every new rule was checked by scanning fixtures rather than by self-tests alone, including negative fixtures of the recommended safe alternatives. Scanning DevSkim itself produces no findings outside the rules' own example strings.
Since the published 1.10.1 package could not be restored in the environment used here, ApplicationInspector was built from source at the
v1.10.1tag together with OAT 1.2.95, with no dependency downgraded, and the whole ruleset was verified against it: no self-test failures under either 1.9.50 or 1.10.1.Follow-up, not included here
Boolean-expression rules are designed but not included. A rule that supplies both
expressionandconditionsbehaves correctly at scan time but is reported as a self-test failure by the rule verifier, which is what CI runs, so such a rule cannot pass today. This reproduces on ApplicationInspector's own documented example and is being raised upstream. TLS consolidation is also deferred, since much of that sprawl exists because the engine could not express per-pattern conditions, and consolidating before that lands would mean rewriting the same rules twice.