Land milestones 4–7 on main (+ exe provenance fix) - #8
Merged
Merged
Conversation
Watches configured paths and emits normalized create / modify / delete / rename events. Two backends behind one collector. - A polling directory-scan backend that runs on every platform. This is the CPU-cheap fallback the design doc promises and the reason the collector is testable off Linux: scan and diff are pure functions over snapshots, the same shape process.nim uses. - inotify decoding for Linux, with the *decoding* kept pure — mask mapping and rename-cookie pairing are ordinary functions with tests — and only the syscalls behind 'when defined(linux)'. That block is type-checked with --os:linux in CI, so it cannot rot unnoticed. Decisions worth knowing: - A rename is emitted as ONE file_rename with both paths, matched by inode, rather than a delete plus a create. To a rule those would read as two unrelated events. Pairing is refused when the inode is 0 or ambiguous. - An unpaired IN_MOVED_FROM expires into a plain delete. A file moved out of a watched directory never gets its pair, and waiting forever for one would lose the event entirely. - A recursive watch selects polling even on Linux. inotify is not recursive; faking it means one watch per directory and losing anything created in a subdirectory between the walk and the watch. - 'available' requires EVERY configured path, not the subset that happens to work. Partial coverage that looks healthy is the failure mode that matters. - A scan that hits max_entries stops and records that it was truncated in raw.scan_truncated, rather than hanging or eating memory quietly. - File events carry no actor: neither inotify nor a scan knows which process was responsible, and raw.attribution says 'none' rather than guessing. 91 tests. The inotify path compiles for Linux but has not been run on a real kernel from this machine; CI covers it.
Reads the kernel's socket tables and turns changes into normalized net_connect / net_listen / net_accept events, correlated to a pid where that is possible. procnet.nim is pure parsing, which is where most of the risk lives: - Addresses are hex and LITTLE-ENDIAN PER 32-BIT WORD. 0100007F is 127.0.0.1, not 1.0.0.127. IPv6 is four little-endian words, so 2001:db8::1 is stored as B80D0120... — a whole-string reversal gets that wrong while still passing a ::1 test, so the suite tests a global address specifically. - Output is RFC 5952 canonical, so rules can match literal address strings. - State codes, ports, uid and the socket inode all parsed and tested, including malformed, truncated and header lines. network.nim diffs consecutive tables and attributes sockets to processes: - socketKey deliberately excludes state, so syn_sent -> established is one connection rather than two, and includes the inode so a reused four-tuple is correctly new. - Direction is partly inference and says so. SYN_SENT and LISTEN are facts; ESTABLISHED is a judgement based on whether the local port is also listened on. Every event carries raw.direction_basis with the reasoning, and the two knowable wrong cases are documented. - Teardown states emit nothing. Seeing FIN_WAIT first means the poll missed the connection's life, and dating the event at teardown would mislead. - Unattributed connections are still emitted, with pid UnknownId and a raw.pid_reason distinguishing 'no inode in the table' from 'cannot read another user's /proc/<pid>/fd — run as root'. The uid is still known even when the pid is not, so ownership is not lost. - /proc/*/fd is only walked when a new socket actually appeared, so a quiet interval costs nothing. 117 tests.
The rules that justify the whole pipeline: each step alone is ordinary, and it is the sequence, from the same actor, inside a time window, that means something. Correlation rules already parsed and validated since M2; this makes them fire. One CorrelationState per rule, holding partial matches in an insertion- ordered seq rather than a hash table — iteration order IS alert order, so determinism is structural instead of incidental. advance() does prune -> advance -> seed, in that order; seeding last is what stops one event walking a brand-new partial through two steps at once. The design decisions, each with a test pinning it: - The window is anchored on the FIRST event of the chain, not the previous one. 'window: 60s' should mean the whole chain happened inside a minute, which is what the rule author is saying. A per-hop anchor lets a ten-step chain paced at 59s gaps span ten minutes while calling itself a one-minute rule, and lets a partial match live forever, breaking the memory bound too. The cost — a chain of individually fast steps whose total span exceeds the window is a false negative — is documented and asserted. - An event advances EVERY partial match it can. Picking one is quieter but silently discards a chain that genuinely completed, which contradicts this project's own position that silent loss is unacceptable. The honest cost is near-duplicate alerts; dedup belongs downstream. - A completed chain is removed and does not re-fire, or a beaconing implant would re-alert on every callback. - An empty join means all events share one bucket. That is distinct from a join field being ABSENT on an event, which makes it unjoinable entirely — counted separately. Eviction is bounded and counted, like the bus. Window expiry is not reported as loss — a window closing is the rule working — but hitting the per-rule cap is. The cap evicts the OLDEST partial, the opposite of the bus dropping the newest, and the comment says why: the bus must never stall a collector, whereas here every candidate is in hand and the oldest is nearest its window closing, so it has the least chance left of completing. reset() clears partials and counters both; a half-finished chain surviving into the next trace would be a fabricated detection. One existing test needed updating, not because the code regressed but because the test's premise expired. 'a correlation rule raises nothing through the single-event path' was written when correlation was inert, and its rule — process then network, no join — genuinely completes twice on the sample trace now. Narrowing its second step to net_accept keeps the real intent (an incomplete chain stays silent) and the reasoning is in a comment so the next reader does not think it was weakened to go green. 45 new tests. Known cost: prune and advance are O(partials) per event per rule. Bounded by the cap so it is a fixed ceiling, but at real event rates the partials want indexing by join key. Documented on the type rather than hidden.
The pieces that turn a library into something someone can actually run. config.nim — YAML configuration, parsed into plain data so it can be validated and tested without starting anything. Every setting has a defensible default, so 'argus run' works with no config at all. A misspelled key is REJECTED, never ignored: a silently-dropped setting is the worst kind of config bug, because everything looks configured and nothing is. The default watch list is four explicit paths rather than the whole filesystem — cheaper, and less of a privacy problem than collecting everything and filtering later. runner.nim — the only module that knows all five pipeline stages at once, and it does nothing but assembly. A bad rule pack refuses to start rather than running with silently fewer detections. A failing sink is counted and reported but never stops detection: losing the ability to write an alert file is not a reason to stop noticing things. Shutdown stops collectors first and then drains, so a clean exit reports what it collected instead of discarding it. CLI — doctor, config, run, plus the existing rules/replay/check-trace/demo. 'run' takes --config, --rules, --duration and --json, handles Ctrl-C by flushing rather than dropping, and prints what is running BEFORE the first event, so a user learns a collector failed while there is still time to act. rules/ — 22 starter rules across process, file, network and correlation, every one with a MITRE ATT&CK technique. They are deliberately simple and the comments say where they are weak: several exclude package managers, which is also the obvious way to evade them; correlation joins on pid, so a chain that crosses a process boundary will not link. scripts/demo.sh — writes a script into its own temp directory, runs it (it prints one line), removes the directory. Nothing outside it is touched, no privilege is needed, and on a host without /proc it says so and stops at the replay step rather than pretending. CI runs it end to end on Linux. Verified end to end: replaying the sample trace against the shipped pack fires five single-event rules plus ARG-CORR-001, which links the shell spawn to the outbound connection from the same pid — the whole pipeline in one line of output. The three benign apt writes to /etc in that trace fire nothing, and a test asserts it, because noise is how a real deployment gets muted. 136 new tests, 721 total. Among them: the shipped rule pack is held to the same standard as the engine (every rule loads, is reachable, carries a resolvable ATT&CK URL, and detects what it claims), and argus.example.yaml must parse back to exactly defaultConfig() — which already caught it drifting on one line.
The process collector marked an executable path as argv[0]-derived — and therefore attacker-controlled — by comparing actor.exe to cmdline[0]. Those are identical whenever a program is invoked by an absolute path, which is completely ordinary, so genuinely kernel-sourced paths were flagged untrustworthy. The consequence was silent: every rule written to exclude argv[0]-derived paths (ARG-PROC-005 in the starter pack, 'process executing from a world-writable directory') simply never fired on a real host, while looking perfectly healthy. procfs.readProcess knows which source it used, because it is the code that tries the symlink and falls back. It now records that as ProcSnapshot. exeFromLink and the collector reads the flag instead of guessing after the fact. Found by running the milestone 7 demo on Linux in CI: a copy of /bin/sleep staged in a temp directory and executed did not raise the alert it should have. Two regression tests pin it, one on either side of the boundary.
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.
Milestones 4–7 →
mainThis PR exists because the stack never reached
main. PRs #4–#7 all merged, but each one's base was the branch below it, and #3 landed inmainbefore #4 landed inm3. So the chain merged into itself and stopped:maincurrently has M1–M3 only.This branch is the tip of that stack — the exact content you already reviewed and approved in #4, #5, #6 and #7 — plus one bug fix. Nothing else has changed.
The one new commit
fix: record exe provenance where it is known, not by comparisonThe process collector marked an executable as
argv[0]-derived — and therefore attacker-controlled — by comparingactor.exetocmdline[0]. Those are identical whenever a program is invoked by an absolute path, which is completely ordinary, so genuinely kernel-sourced paths were flagged untrustworthy.The consequence was silent:
ARG-PROC-005("process executing from a world-writable directory") excludesargv[0]-derived paths, so it simply never fired on a real host — while the rule listed fine, the tests passed, and everything looked healthy.procfs.readProcessknows which source it used, because it is the code that tries the symlink and falls back. It now recordsProcSnapshot.exeFromLink, and the collector reads that flag instead of guessing after the fact. Two regression tests, one either side of the boundary.Found by running the M7 demo on Linux in CI — a copy of
/bin/sleepstaged in a temp directory and executed did not raise the alert it should have. That is the demo doing its job.Also fixed since #7 was opened
Two of the three shipped correlation rules could never fire. Both paired a file step with a process step joined on
actor.pid, but file events carry no actor at all — neither inotify nor a directory scan knows which process made a change — sojoinKeyreturns empty and the event can neither seed nor advance a chain.They loaded without complaint and were permanently dead: exactly the silently-never-fires failure this project says it will not ship. Rewritten to correlate process and network events only, with:
rules/04-correlation.yaml,Verification
730 tests, 0 failures.
nimble lintclean.Replaying the sample trace against the shipped pack:
CI runs
scripts/demo.shend to end on Linux, where all three collectors are available and inotify fires on a real kernel.