Tags: microsoft/DevSkim
Tags
Close SDL coverage gaps and fix rules that could never match (#780) * Ship the Android and XSLT rules that were never embedded 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 * Give the two distinct DS440011 rules separate IDs 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 * Close the banned API gap against SDL ADM.10082 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 * Cover the deserializers ADM.10010 names as unsafe 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 * Extend XXE coverage beyond Objective-C and Swift 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 * Use the JSONPath, YAMLPath and XPath support the engine already has 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 * Add missing language definitions and target React files 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 * Detect private keys and provider tokens, and fix a JSONC regression 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 * Give every rule a self-test, which surfaced three dead rules 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 * Add changelog entry for the SDL coverage work Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8 * Make DevSkim's overrides survive the containment change in AppInspector #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 * Record the override compatibility fixes in the changelog 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 * Move to ApplicationInspector 1.10.1 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 * Record the 1.10.1 move in the changelog 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 * Use obviously fake sample values in the token self-tests 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 * Add boolean expression rules and update security rules in ApplicationInspector (#787) * Add handoff notes for the boolean expression rule work PR #780 deliberately left out the boolean expression rules. Application Inspector 1.10.1 shipped the feature, but a rule supplying both `expression` and `conditions` behaved correctly at scan time while the rule verifier failed its must-not-match samples, and the verifier is what DefaultRulesTests runs. Correct rules could not merge. That is fixed in 1.10.2 by ApplicationInspector#656, which routes self tests through the same capture reduction the analyzer uses. This commit carries the work that was already done but had nowhere to live, so the next person does not rebuild it: - The merged DS440016, which collapses the two same-id entries in tls_generic.json into one rule using `(curlFlag AND NOT tls13) OR p0 OR ...`. This is the curl/wget case from upstream's own test suite, and retires one of the duplicate ids. Verified byte-identical in meaning to the version built earlier. - The remaining soundness-gap rules to write, with the reason they need the feature: the old shape can only express P AND NOT (C0 OR C1), "no mitigation at all", where cookie flags and HSTS need P AND NOT (C0 AND C1), "any required mitigation missing". The partially hardened case is the one being missed. - The constraints that are expensive to rediscover. Chief among them: boolean operators over pattern labels alone are nearly useless, since a finding comes from exactly one pattern, so `a AND NOT b` is identical to `a` and `a XOR b` is identical to `a OR b`. All the real expressiveness needs patterns and conditions in one expression. Both commands quoted in the notes were run against this branch before committing: the embedded-resource check reports 155 shipped rules with none missing, and rule verification is silent. The file is a working note and says so; the follow-up work should delete it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8 * Update ApplicationInspector dependencies and add new security rules - Bump Microsoft.CST.ApplicationInspector.Logging and Microsoft.CST.ApplicationInspector.RulesEngine to version 1.10.2 in project files. - Introduce new rules for cookie security attributes (DS610001) and HSTS directives (DS610002) with corresponding guidance documentation. - Implement a boolean expression rule (DS440016) to detect hard-coded SSL/TLS protocols in code. - Remove obsolete handoff document for boolean expression rules. - Update existing TLS rules to reflect new conditions and expressions. - Add new JSON rule files for HTTP headers security checks. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8 * Fix React comment handling and pip requirements coverage * Correct Kubernetes, SQL Server, and NuGet rule matching * Fix multiline Python XML and YAML safety checks * Scope HTML link protections to each anchor tag * Respect HTTP header boundaries and quoted cookie values * Validate HSTS directive structure and reject duplicates * Fix React and Razor language routing * Fix untagged private registry image detection * Match connection string options case-insensitively * Fix package source rule coverage * Scope HTML link suppression to each anchor * Fix XXE rules for FSharp and lxml * Document review fixes * Fix nested PyYAML loads and raw header apostrophe handling * Allow apostrophes in unquoted headers and document Razor rule scoping * Address review: comment mappings, TLS false positives, VB casing, guidance * Update hardcoded TLS rules to include additional cipher list patterns * Update guidance and rules for secret detection, privilege escalation, and deserialization checks per review comments. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fc65bf3-6ed4-40fe-b21f-848c191b8bc8
Update NuGet package versions in project files (#703) * Update NuGet package versions in project files Bumped several package dependencies to newer versions across multiple .csproj files, including Microsoft.CST.ApplicationInspector, Microsoft.Extensions.Logging, Serilog, MSTest, and Microsoft.CodeAnalysis. This ensures compatibility with the latest features and bug fixes from these libraries. * Update Changelog.md
Add release npmrc and update registry handling (#700) * Add release npmrc and update registry handling Renamed .npmrc to .npmrc.pr and added .npmrc.release for release builds. Updated package.json scripts to support separate registry URLs for pre-release and release, and modified updatePackageLock.js to accept registry URL as an argument for more flexible registry rewriting. * Update Changelog.md * Add restore-net-release script to package.json Introduces a new npm script 'restore-net-release' for restoring .NET dependencies using the release configuration. Updates 'pipeline-pack-release' to use this new script for improved release build handling. * Update devskim-vscode-release.yml
Bumps npm dependencies and DevSkim .NET dependencies, Fix #697 (#698) * Bumps npm dependencies * Update Changelog.md * Fix #697 Populate the Description or Recommendation fields in the markdown description since that is what ends up rendered when used in github code scanning per report in #697 of inability to customize message with custom rules. * Clean up and changelog update * Add unit tests for SarifWriter help field logic Introduces SarifWriterTests to verify SARIF help field population for rules with various combinations of recommendation, description, and rule info. Tests cover fallback logic, markdown formatting, and edge cases such as empty or whitespace recommendations. * Update DevSkim-DotNet/Microsoft.DevSkim.CLI/Writers/SarifWriter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Refactor SARIF rule markdown description logic Extracted markdown description building into a new BuildMarkdownDescription method for improved readability and maintainability. The logic for constructing the SARIF rule's Help.Markdown field is now encapsulated in a dedicated helper function. * Refactor SarifWriterTests to use local writers Replaces class-level StringWriter and SarifWriter fields with local variables in each test method. This improves test isolation and resource management by using 'using' statements for disposable objects. * Refactor SARIF output parsing in tests Replaced calls to the ParseSarifOutput helper with direct usage of JObject.Parse in SarifWriterTests. Removed the now-unused ParseSarifOutput method for simplification. * Rename SarifWriter test methods for clarity Test method names in SarifWriterTests.cs were updated to use descriptive, behavior-driven naming. This improves readability and makes test purposes clearer for future maintenance. * Remove unused Patterns property in SarifWriterTests Eliminated the Patterns property from the test case object initialization in SarifWriterTests.cs, as it was not required for the test. * Refactor SARIF rule text description logic Moved the logic for building SARIF rule text descriptions into a dedicated BuildTextDescription method for improved readability and maintainability. * Add test for empty markdown in SARIF rule help Introduces a unit test to verify that when a rule has no recommendation and no rule info, the SARIF 'help.markdown' field is empty or null, while 'help.text' falls back to the rule description. * Update changelog for v1.0.63 with fixes and tests Added details for version 1.0.63 including a fix for Sarif Markdown recommendation value population (#697), new test cases for SarifWriter, and updated dependencies. Fixed some section header levels to improve formatting. * Remove redundant test for SARIF markdown help content Deleted the test 'When_rule_has_recommendation_and_rule_info_then_markdown_includes_both' from SarifWriterTests.cs as it was redundant with `When_rule_has_recommendation_and_rule_info_then_markdown_is_properly_formatted` * Dependencies in checked in package-lock file should use npmjs repository The internal repository is substituted during pipeline build to allow for external contributor use * Refactor SARIF help URI construction and update tests Introduced a CreateHelpUri method in SarifWriter to safely construct help URIs for DevSkim rules, handling null or empty RuleInfo values. Updated related unit tests to use the new baseHelpUri constant for consistency and maintainability. * Update tests to use SarifWriter.CreateHelpUri Replaces references to SarifWriter.baseHelpUri with SarifWriter.CreateHelpUri in SarifWriterTests to ensure help URIs are generated consistently. This improves test accuracy and future-proofs against changes in URI construction. * Rename baseHelpUri to BaseHelpUri in SarifWriter Updated the constant baseHelpUri to use PascalCase (BaseHelpUri) for consistency with naming conventions. Adjusted references to the constant accordingly. * Update SarifWriterTests to use exact string assertions Changed tests to use Assert.AreEqual with expected markdown and help text strings instead of Assert.IsTrue with Contains. This ensures stricter validation of the output format. * Update Changelog.md * Refactor SarifWriterTests to reuse helpUri variable Replaces repeated calls to SarifWriter.CreateHelpUri with a local helpUri variable in test assertions for expectedMarkdown. This improves readability and reduces redundant method calls. * Change BaseHelpUri to private constant Updated the visibility of the BaseHelpUri constant from public to private in SarifWriter.cs to restrict its access within the class. * Change CreateHelpUri to internal access modifier The CreateHelpUri method in SarifWriter was changed from public to internal to restrict its visibility within the assembly. This helps encapsulate implementation details and limits external usage. * Update SarifWriter.cs * Expose internals to test project and update method visibility Added InternalsVisibleTo for Microsoft.DevSkim.Tests in the CLI project file to allow unit testing of internal members. Changed CreateHelpUri from public to internal in SarifWriter to restrict its visibility to within the assembly. * Remove unused variable in AnalyzeTest.cs Deleted the unused 'oneUpPath' variable from the test setup to clean up the code. * Remove unused variable in SuppressionsTest Deleted the unused 'oneUpPath' variable from the test method to clean up the code. * Remove unused exception variable in regex creation Eliminated the unused exception variable in the catch block of the regex creation method. Added a TODO comment noting the need to refactor for logging since the logger is not accessible in the static context. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Move GitHub Release tasks to separate explicitly release jobs (#692) * Move GitHub Release tasks to separate explicitly release jobs
PreviousNext