Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# Changelog

## Unreleased

- Project checks: app-specific pins under `checks:` in `storehound.yaml`, for
`Info.plist` keys (with nested keys) and `AndroidManifest.xml` attributes,
with the operators `exists`, `absent`, `equals`, `absent_or_equals`,
`not_equals`, `contains`, `none_of` and `matches`. Each check needs a
`reason`, defaults to `error`, and is reported in text, JSON and SARIF as
`check:<name>` with the file and line of the value (or of its parent when
it is missing). `ignore`, `rules` and `--explain check:<name>` accept it.
A mistake in a check is a configuration error with its line (exit code 2).
- The production-shaped fixture pins five values with `checks:`.

## 0.2.0

Renamed from imaji_store_preflight. Same rules and CLI; the command is now
Expand Down
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,25 @@ including how `android-target-sdk` behaves on a Dart-only runner.
`storehound --explain <rule-id>` prints the full why, fix, options
and documentation links for one rule.

### Your own checks

Pins only your app cares about go under `checks:` in `storehound.yaml`, each
with a reason, and are reported like any rule as `check:<name>`:

```yaml
checks:
no-backup:
file: android_manifest
element: application
attribute: android:allowBackup
equals: "false"
reason: "Tokens live in shared preferences."
```

They work on `Info.plist` keys and `AndroidManifest.xml` attributes; see
[Project checks](https://github.com/saimskywalker/storehound/blob/main/doc/configuration.md#project-checks)
for the operators.

## Is it noisy on a real app?

It should not be. CI runs it with `--fail-on info` on
Expand Down
93 changes: 92 additions & 1 deletion doc/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ storehound [options] [project-dir]
--brief Text output without the why/fix explanations
--[no-]color Colour text output (default: when stdout is a terminal and NO_COLOR is unset)
--list-rules List every rule
--explain=<rule-id> Explain one rule
--explain=<rule-id> Explain one rule, or a project check as check:<name>
--version Print the version
-h, --help Show the usage
```
Expand Down Expand Up @@ -102,6 +102,97 @@ code 2) with the line number, so a typo cannot silently switch a check off.
| `android-picker-queries` | `plugins` (extra file-picker packages) |
| `android-target-sdk` | `target_sdk`, `deadlines` (`api`, `from`), `warn_days` |

## Project checks

Pins that only your app cares about, the kind usually kept as hand-written
tests that parse `Info.plist` or `AndroidManifest.xml`, go under `checks:`.
Each one is reported like a built-in rule, with the rule id `check:<name>`:

```yaml
checks:
encryption-exempt:
file: info_plist
key: ITSAppUsesNonExemptEncryption
equals: false
reason: "Export compliance is answered in App Store Connect; keeps each upload from asking."

background-modes-pinned:
file: info_plist
key: UIBackgroundModes
equals: [audio, remote-notification]
reason: "The modes App Review accepted. A new one needs review notes first."

ats-no-arbitrary-loads:
file: info_plist
key: NSAppTransportSecurity.NSAllowsArbitraryLoads # a dot steps into a nested dictionary
absent_or_equals: false
reason: "All traffic is HTTPS."

no-backup:
file: android_manifest
element: application
attribute: android:allowBackup
equals: "false"
severity: warning
reason: "Tokens live in shared preferences."

no-media-permissions:
file: android_manifest
element: uses-permission
attribute: android:name
none_of: [android.permission.READ_MEDIA_IMAGES, android.permission.READ_MEDIA_VIDEO]
reason: "The app uses the system photo picker only."
```

| Key | Meaning |
|---|---|
| `file` | `info_plist` (`paths.info_plist`) or `android_manifest` (the main manifest, `paths.android_manifest`) |
| `key` | Info.plist only. A top-level key, or a dotted path into nested dictionaries. For a key that contains dots, list the parts: `[NSAppTransportSecurity, NSExceptionDomains, example.com]` |
| `element`, `attribute` | AndroidManifest only. Every element with that name, anywhere in the manifest, and the attribute's name as written, e.g. `android:allowBackup` |
| `reason` | Required: why the value is pinned. Shown as the "why" in every report and in `--explain` |
| `severity` | `error` (default), `warning` or `info` |

Each check has exactly one operator:

| Operator | Info.plist value | AndroidManifest attribute |
|---|---|---|
| `exists: true` | is present | at least one element sets it |
| `absent: true` | is not present | no element sets it |
| `equals: <value>` | is present and equal; a list equals an array with the same items in any order | every element sets it to the value |
| `absent_or_equals: <value>` | is missing or equal | every element that sets it uses the value |
| `not_equals: <value>` | is missing or different | no element sets it to the value |
| `contains: [<values>]` | is an array that holds every listed value | the elements' values together hold every listed value |
| `none_of: [<values>]` | is none of the values (for an array: no item is) | no element's value is listed |
| `matches: <regex>` | is a string the regular expression matches | every element sets a value it matches |

Values are compared with their type: `equals: false` matches `<false/>`, not
`<string>NO</string>`, and `equals: "42"` does not match `<integer>42</integer>`.
Manifest attributes are text, so quote values there (`equals: "false"`).
`matches` finds the pattern anywhere in the value; anchor it with `^` and `$`
to match the whole value.

Findings point at the value's line; when the value is missing, at its
parent: the enclosing dictionary's key in Info.plist, the element in the
manifest, or `<manifest>` when no element has that name. A check whose file
does not exist is a finding too, so a moved file cannot switch it off
silently. A check can be named in `ignore` (`rule: check:no-backup`) and under
`rules:` like any rule, and `storehound --explain check:no-backup` prints it.
To switch a check off for a while without deleting it, disable it under
`rules:` with a reason, as for a built-in rule:

```yaml
rules:
check:no-backup:
enabled: false
reason: "Backups are back on until the token moves to the keystore."
```

Mistakes in a check (an unknown file or operator, a value of the wrong type,
a missing `reason`) are configuration errors with the line number, exit
code 2.

The privacy manifest and entitlements are not supported yet.

## Use it as a library

```dart
Expand Down
34 changes: 29 additions & 5 deletions lib/src/cli.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'finding.dart';
import 'report/machine_reporters.dart';
import 'report/text_reporter.dart';
import 'rules/all_rules.dart';
import 'rules/project_checks.dart';
import 'version.dart';

/// Exit codes of the command-line tool.
Expand Down Expand Up @@ -81,7 +82,9 @@ ArgParser _parser() => ArgParser(usageLineLength: 80)
..addFlag('list-rules', negatable: false, help: 'List every rule and exit.')
..addOption(
'explain',
help: 'Explain one rule (why it exists, how to fix it) and exit.',
help:
'Explain one rule (why it exists, how to fix it) and exit. '
'A project check is `check:<name>`.',
valueHelp: 'rule-id',
)
..addFlag('version', negatable: false, help: 'Print the version and exit.')
Expand Down Expand Up @@ -127,15 +130,19 @@ int runCli(
out.writeln(renderRuleList(builtInRules));
return ExitCodes.ok;
}
if (args.option('explain') case final id?) {
final explain = args.option('explain');
if (explain != null) {
for (final rule in builtInRules) {
if (rule.id == id) {
if (rule.id == explain) {
out.writeln(renderExplain(rule));
return ExitCodes.ok;
}
}
err.writeln('Unknown rule `$id`. Run --list-rules to see them all.');
return ExitCodes.usage;
// A project check lives in the configuration, which is loaded below.
if (!explain.startsWith(checkRulePrefix)) {
err.writeln('Unknown rule `$explain`. Run --list-rules to see them all.');
return ExitCodes.usage;
}
}

var projectArg = args.option('project')!;
Expand Down Expand Up @@ -184,11 +191,28 @@ int runCli(
err.writeln('Invalid configuration: $e');
return ExitCodes.usage;
}
if (explain != null) {
for (final check in config.checkRules) {
if (check.id == explain) {
out.writeln(renderExplain(check));
return ExitCodes.ok;
}
}
final name = explain.substring(checkRulePrefix.length);
err.writeln(switch (config.source) {
null => 'Unknown check `$explain`: no configuration file was found.',
final source =>
'Unknown check `$explain`: $source has no `$name` '
'under `checks:`.',
});
return ExitCodes.usage;
}
if (args.option('fail-on') case final failOn?) {
config = PreflightConfig(
rules: config.rules,
ignores: config.ignores,
paths: config.paths,
checkRules: config.checkRules,
source: config.source,
failOn: failOn == 'never' ? null : Severity.tryParse(failOn),
);
Expand Down
Loading