Skip to content

feat: security checks — 14 rules, standalone-capable (v0.9.0)#38

Merged
nenadvulic merged 25 commits into
mainfrom
feature/security-check
Jun 12, 2026
Merged

feat: security checks — 14 rules, standalone-capable (v0.9.0)#38
nenadvulic merged 25 commits into
mainfrom
feature/security-check

Conversation

@nenadvulic

Copy link
Copy Markdown
Owner

What

Adds an opt-in security: section to .solid.yml: lint now detects 14 insecure Swift/iOS patterns across Keychain, Crypto, Network, Auth and Logging — alongside (or without) architecture rules.

# Minimal config — no architecture required (`layers:` is now optional)
security:
  enabled: true

The 14 rules

Provable patterns (default error): keychainAccessibleAlways, keychainMissingAccessibility, insecureHash (MD5/SHA1), hardcodedSecret, cleartextHTTP (Info.plist ATS off), tokenInUserDefaults, biometryNoErrorHandling, publicPIIInLog, disabledTLSValidation (trust-all handlers).
Heuristics (default warning): printSensitiveData, httpURLLiteral, biometryNoFallback, sensitiveDataInUserDefaults, highEntropySecret.

Severity: per-rule rules.<id>.severity > global severity > built-in default; disable: [ruleID]; unknown IDs are a config error. // solid:ignore <reason> suppresses a finding (reason mandatory).

Architecture

SecurityRule protocol, one struct per rule, one SwiftSyntax parse per file shared by all rules; PlistATSCheck scans Info.plist separately. Findings are regular Violations (new .securityIssue reason + detail), so baseline, text/json/github reporters, exit codes, the GitHub Action and the Claude Code hook work with zero changes.

Also in this PR

  • init --security: standalone security preset (works on empty projects), composable with --tca/--freeze
  • layers: optional + explicit "nothing to check" error on inert configs
  • solid:ignore parsing factored out and anchored (prose mentioning the directive no longer suppresses)
  • README restructured: security visible from the first screen (pitch + Quick start), full 14-rule table; examples/security.solid.yml; CHANGELOG 0.9.0

Real-world calibration (done before this PR)

  • Self-lint (Sources+Tests): clean
  • isowords (285 files): 0 findings — localhost/namespace exemptions verified working
  • Signal-iOS / SignalServiceKit (1408 files): 32 errors → 2 defensible errors after FP fixes — including a genuine hardcoded Giphy API key the rule caught ✨; remaining warnings are real embedded key material (UD trust roots, sticker pack keys), correctly identified
  • Every FP fixed with a regression test first: 212 tests, 0 failures

Release notes

Tag v0.9.0 after merge (release pipeline auto-publishes binaries, Homebrew, GitHub Action). The README install pins (v0.8.0) get bumped in the usual chore: release commit.

…e config errors

- Replace `try?` with `decodeIfPresent` for `layers` and `security` so a present-but-malformed key throws instead of silently becoming nil/[].
- Add `CustomStringConvertible` to `ConfigurationError` covering all four cases with human-readable messages.
- Document `isEnabled(ruleID:)` contract: does not check the master `enabled` flag (engine guards separately).
- Add two failing-first tests: `testMalformedLayersThrowsAtDecode` and `testMalformedSecuritySectionThrowsAtDecode`.
Also adds .securityIssue to the exhaustive switch in GraphBuilder.reasonLabel.
…'key'

Rule now evaluates only the last member component of a.b expressions
and bare identifiers — never the base of a member access — so tokens.count,
token.kind, key/value loop vars, keyWindow and keyPath are silent.
Bare "key" is also removed from the rule-local word list; multiword forms
(apiKey, storeKey) still fire via the adjacent-pair join in matchesSensitiveName.
…namespace URLs

DisabledTLSValidationRule: switch from trimmedDescription to token-joined
text so trivia (comments) cannot suppress findings; add cancelAuthenticationChallenge
token guard so TrustKit-style wrapper delegates (which can reject) are never
flagged; SecTrustEvaluate* matched via hasPrefix on token array.

HttpURLLiteralRule: skip bare-empty host (url.hasPrefix("http://") idiom);
add XML-namespace/DTD authority allowlist (www.w3.org, schemas.android.com,
schemas.microsoft.com, schemas.xmlsoap.org, xmlns.com, purl.org, ns.adobe.com,
www.apple.com) and .dtd path suffix skip — opaque identifiers, never fetched.

195 tests, 0 failures.
- Track usedSecurityPreset flag in InitCommand so the standalone path
  prints "mode: security-only" instead of the misleading "mode: layered,
  modules: 0" line.
- Add blank line in ConfigGenerator.securityPreset() between the two
  header comment lines and the security: block, matching the appended
  variant's formatting.
- Strengthen testSecuritySectionAppendsToGeneratedConfig: build a real
  temp fixture project via makeProject, call generate(mode: .layered),
  append securitySection(), then load and assert security?.enabled == true
  and layers non-empty.
Self-lint + isowords + Signal-iOS (SignalServiceKit, 1408 files) triage:

- highEntropySecret: skip all-distinct-character literals — alphabet/charset
  tables (e.g. the base64 alphabet in our own rule) are high-entropy by
  construction but never secrets; real keys repeat characters.
- hardcodedSecret: exempt identifier-shaped values — literals echoing their
  own variable name, and word phrases separated by _ - / or space where each
  word is letters plus at most 3 trailing digits (storage key names, header
  names like x-signal-checksum-sha256, REST paths like v2/keys/signed).
  Random keys keep firing via interior digits or long digit runs.
- biometryNoErrorHandling: skip bare statement calls whose result is unused
  (the documented idiom to populate biometryType makes no auth decision).

Signal-iOS production errors: 28 -> 2, both defensible (a real hardcoded
Giphy API key; SHA1 usage pending human confirmation). isowords: clean.

Each fix carries a regression test reproducing the real-world false
positive (211 tests green).
- HttpURLLiteralRule: remove dead "::1" from exemptHosts array (split on
  ":" mangled both raw http://::1/... and bracket http://[::1]:8080/...
  forms so neither ever matched). Check for IPv6 loopback before the
  port-strip step: exempt when host starts with "[::1]" or equals "::1".
  Two new TDD tests confirm both forms are correctly exempted.
- SecurityRuleRegistry: replace three-line TRANSITIONAL doc comment with
  a one-liner — the registry is now complete (13 Swift rules + cleartextHTTP).
@nenadvulic nenadvulic merged commit 5b19dda into main Jun 12, 2026
2 checks passed
@nenadvulic nenadvulic deleted the feature/security-check branch June 12, 2026 07:29
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.

1 participant