Skip to content

feat(cli): load rules database from file - #78

Merged
Muawiya-contact merged 2 commits into
Coding-Moves:mainfrom
alexsmolya:codex/issue-68-rules-file
Sep 7, 2026
Merged

feat(cli): load rules database from file#78
Muawiya-contact merged 2 commits into
Coding-Moves:mainfrom
alexsmolya:codex/issue-68-rules-file

Conversation

@alexsmolya

Copy link
Copy Markdown
Contributor

What & why

Fixes #68

diskern scan can now accept an optional external rules database:

diskern scan <path> --rules ./my-rules.json

Without --rules, the CLI continues to use the embedded rules database. A supplied file is deserialized through the existing RulesDb model, with explicit errors for unreadable files and malformed JSON. Human-readable output identifies the external rules source, while JSON output remains valid on stdout. The change stays at the CLI boundary and does not move classification logic out of diskern-core.

The user-visible feature is documented in CHANGELOG.md.

Validation

  • cargo fmt --all
  • cargo +1.96.0 clippy -p diskern-core -p diskern-cli --all-targets -- -D warnings
  • cargo +1.96.0 test -p diskern-core -p diskern-cli
  • git diff --check

All passed. The focused suite includes regression coverage for embedded rules, valid external rules, missing files, and malformed JSON.

Checklist

  • cargo fmt --all and the required core/CLI clippy validation are clean
  • The required core/CLI test suite passes
  • Commits are small and focused (one logical change each)
  • Doesn't weaken a safety principle (read-only scans, quarantine over deletion, deterministic verdicts)
  • Workspace-wide GUI validation was not run; it is outside the repository-required core/CLI gate

@Muawiya-contact

Copy link
Copy Markdown
Member

@alexsmolya Welcome, and thanks for the contribution! Nice to have you here. 🙌

I went through the implementation, the tests, the contributing guide and the docs. The change is nicely scoped: it stays at the CLI boundary, fails before the scan starts if the file is bad, and keeps stdout clean in JSON mode. Test coverage for embedded / valid / missing / malformed is good to see.

Left a few comments inline. Only the docs one really blocks, the rest we can talk through. Once that's sorted I'm happy to approve and merge.

Comment thread CHANGELOG.md
Comment thread crates/diskern-cli/src/main.rs Outdated
Comment thread crates/diskern-cli/src/main.rs
Comment thread crates/diskern-cli/src/main.rs
@Muawiya-contact

Copy link
Copy Markdown
Member

On the docs point, this is the table I meant:

| Flag | Default | Effect |
| ----------- | ------- | ------------------------------------------------------- |
| `--top N` | `5` | Findings shown per category; `0` shows every one. |
| `--verdict` | all | `safe`, `review`, `risky` or `protected`. Duplicate sets have no verdict, so they are omitted when this is set. |
| `--json` | off | Full report as JSON; the flags above don't apply. |

A row right under --json would do it, something like:

| --rules | embedded | Load the rules database from a JSON file instead of the embedded one. |

@Muawiya-contact

Copy link
Copy Markdown
Member

@alexsmolya Great turnaround, thanks for picking these up so quickly! 🙌

Both fixes landed in diskern-core rather than the CLI, which is the right side of the engine/frontend line, and the test that drops a real windows/installer/setup.msi under a safe **/*.msi rule and checks it still comes back protected is the exact scenario I had in mind, written better than I described it.

Two small leftovers, neither blocking:

  • The --help text for --rules still says "instead of the embedded database". The README row now describes it correctly, so the flag description is the odd one out.
  • validate() catches bad globs but not an empty "patterns": [], which still leaves a rule that can never match, the same thing the function is there to prevent.

Push those when you get a chance and I'll take it from there.

And welcome to Coding Moves, really glad to have you here. 🚀

@Muawiya-contact
Muawiya-contact merged commit fc0378d into Coding-Moves:main Sep 7, 2026
15 checks passed
@Muawiya-contact

Copy link
Copy Markdown
Member

@alexsmolya Quick follow-up so you don't spend time on this: I picked up the two leftovers myself rather than hold the PR for another round trip, so there's nothing left for you to push. They're in #97 — the --help wording for --rules, and validate() now also rejecting an empty "patterns": [], which slipped through for the same reason a bad glob used to.

This is merged, and it closes #68. Thanks again — turning the protected-rule case into a regression test is the part that'll keep paying off long after the flag itself is old news. Looking forward to the next one.

@Muawiya-contact

Copy link
Copy Markdown
Member

@alexsmolya Your change went out in v0.2.0 today, and you're in the release notes 🎉

Thanks again for turning the review around so fast. Honestly the best bit was the test you added for the protected rule case, not the fix itself.

I've left comments on #94 as well, that one's a big piece of work. When you're through it, #69 is a --exclude flag which is pretty close to what you built here, and #71 is adding rules for whatever caches live on your own machine. We've got a few other repos too if you fancy something other than Rust. one-concept is Python, Arion_OS is a 64-bit OS in C++ and assembly. And if you spot something broken anywhere, an issue is just as useful as a PR.

If you like the project, a ⭐ helps people find it. See you on the next one 🚀

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.

Let the CLI load a rules file, so rules can be tested without a rebuild

2 participants