feat(discovery): ADR Discovery as a standalone top-level module - #53
Closed
pengyuzhang wants to merge 15 commits into
Closed
feat(discovery): ADR Discovery as a standalone top-level module#53pengyuzhang wants to merge 15 commits into
pengyuzhang wants to merge 15 commits into
Conversation
Adds ADR Discovery as a fifth capability: finding which AI tools are present on an endpoint, as distinct from collecting telemetry from the tools already known to be there. Marked as not included in the current open-source release, consistent with how ADR Prevention is listed.
Discovery answers the prior question the other four capabilities assume away — which AI tools are on an endpoint at all — so it reads more naturally ahead of Observability. Reorders the list and the lead-in sentence to match; no wording changes to the items themselves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GK2DWwdgH8UcGQLMBYpRsT
Implements Plane A of ADR Discovery: an inventory of the AI tools and agents present on an endpoint, following the architecture in the design doc. Probes read the machine only through an injected DiscoveryEnv (filesystem root, process table, socket table, registry view, HTTP prober, subprocess runner), so the pipeline can be pointed at a fixture world and graded rather than only run. Stage 0 DiscoveryEnv bounded reads, depth caps, symlink-loop safe Stage 1 probes mcp, cli_agent, app, runtime, extension Stage 2 openworld weak signals over what the catalog rejected Stage 2b process the agent-to-MCP-server binding, observed Stage 3 resolver union-find on identity keys, conflict blocking Stage 4 liveness running / installed / declared_only Stage 5 diff deltas per asset_id, plus fleet fan-out Fingerprints ship as catalog.json rather than code, so a newly released tool is a catalog update instead of a client release. Redaction runs in the collector before a snapshot is written: argv keeps flag names and drops their values, URLs lose query strings, env blocks keep names only, and personal paths are denied centrally in BaseProbe. Run with `adr-sensor discover`; `--dry-run --explain` prints exactly what would leave the machine, per probe and per field.
…suite
Implements every case in Appendix A of the design document: 192 cases across
AI tools, MCP servers, skills and AI agents, run as a scorecard rather than a
pass count. 365 checks, all passing.
The suite drove the module rather than the other way round. What it added:
probes/agent_artifact.py skills, commands, output styles, hooks across 30+
lifecycle events and 5 handler types, plugins and
what they bundle, instruction files and their
import graph, Cursor rules
probes/scheduler.py launchd, cron, systemd timers, Windows tasks, CI
workflows, dispatch sessions, cloud delegation
probes/identity.py which account an agent authenticates as, and how
probes/location.py devcontainer and remote-workspace declarations
and extended the existing probes to the current landscape: twelve more CLI
agents, AI browsers as their own kind, model weights as inventory, every
Chromium profile and Firefox XPIs, nvm/mise/uv/go install channels, WSL
locations; MCP configs for Windsurf, Zed, VS Code, goose, Cline and JetBrains,
enterprise managed settings on three platforms, MDM policy by preference domain
and registry key, .mcpb bundles, and richer supply-chain verdicts.
Three resolver corrections came out of cases that disagreed with each other:
- conflict is checked across the whole merged group, not pairwise, so two
users' agents no longer unite through one system-wide binary
- attributes (state dirs, model stores, ports) bind to the install they
belong to instead of standing alone as duplicate assets
- version precedence is runtime over packaging, and a disagreement is
recorded rather than silently resolved
Verified on a real endpoint as well as on fixtures: 31 assets, 0 errors, 2.3s,
inside the 3s budget.
Every finding reproduced, including the one I could not reproduce at first - that was my test placing the canary a directory too high, not the report being wrong. Fixes in the order they matter, with a regression case for each. Filesystem boundary, the two critical ones. Paths are canonicalized before access, and both root containment and the deny-list are enforced on the resolved target rather than on the name we were handed. A permitted ~/.claude.json symlinked into ~/Documents no longer reads; a relative segment inside a config no longer climbs out of the tree. A denied path asked for outright is refused quietly, because enumerating a home directory meets those constantly; a permitted path that resolves into one is recorded, because that is the shape of a deliberate bypass. Resolver conflict is now a property of the merged group for every identity field, not just for owner. A bridge observation sharing a key with two unrelated tools no longer unites them - which was the exact failure the docstring on conflicts() warns about, left half-fixed when the owner case was addressed. Production collection: listening sockets are collected (every port-based detection passed on fixtures and was inert on a real endpoint), the process table is filtered to this user rather than merely described as being, and subprocess output is bounded as well as timed. Detection hardening: download-and-execute recognizes the spellings it actually takes; hostnames are parsed and compared on a dot boundary, so evilcorp.example is not corp.example and api.openai.com.evil.test is not a provider; redaction covers JWTs, bearer headers, PEM blocks and credential flags by name; the nix identity drops the version so an upgrade reads as a version change. Reporting: the per-config cap applies wherever servers come from, and caps, walk truncation and refusals appear in snapshot coverage. A file sitting exactly on the read ceiling is whole, not truncated. Listener probing is concurrent. Adding real socket collection took the live scan to 10.7s; a stalled port now costs a shared timeout rather than one each, and the scan is back to 2.8s inside the 3s budget. 406 checks pass across 208 cases, including 18 new hardening regressions.
All eight reproduce. Fixed in the order they hurt, with a regression each. Failure isolation is now per record rather than per probe. The module promises partial results plus an error record and enforced that at probe granularity while failures happen per entry, so one malformed env block - a list where a map belongs - erased every valid server beside it. That is a denial of inventory an attacker can arrange with one config line. Runtime MCP identification requires MCP-specific evidence. "npx" is not evidence: it runs eslint far more often than it runs a server, and "server" as a substring matched my-server-test.py. Ordinary children of an agent no longer become high-severity undeclared findings. What strictness would have cost is paid back by correlation: a declared server whose name says nothing is recognized the moment a config on the host declares the command that is running. Project approval compares path components. A raw prefix test said /dev/application lives under /dev/app, extending one project's approvals to a neighbour that merely shares the first letters of its name. Approval is also three-valued now - declared-but-unapproved is not a synonym for enabled. Input validation: a string where an argument array belongs is one argument rather than nine characters, which was changing both identity and the pinning verdict; catalog fingerprints claimed twice are rejected at load and reported rather than silently resolving to whichever entry came last. Snapshot invariants: the resolver guarantees unique asset ids and marks any it had to disambiguate; a diff refuses two hosts unless asked explicitly, refuses an ambiguous snapshot, and normalizes every fingerprint field, so a stdio server with no endpoint no longer raises. 429 checks pass across 216 cases. Live scan: 3.0s, 31 assets, no errors, and no findings on a machine that has nothing to find - which is the point.
All seven reproduce on 0706a11, including both halves of the critical one. Configuration arguments are now redacted the way command lines already were. Sanitizing control characters is not redaction, and a config's argument array carries credentials as readily as an argv does: --token and a bearer header both reached the snapshot verbatim. Identity is built from the launch as written and storage from the launch redacted, because an identity derived from redacted text stops matching the moment a launch carries a credential flag. That split is also what fixes correlation. The two channels were normalizing differently - a config writes /usr/local/bin/node, a process table reports node - so a declared server that was plainly running was never recognized. One canonical launch identity now serves both, with the executable reduced to a basename and .exe dropped. Record-level isolation and type validation now apply to every MCP source rather than to the main config loop alone. A managed policy of the wrong shape, or one bundle manifest with a string where an object belongs, used to return an empty inventory with the valid discoveries inside it. Environment expansion parses whole variable tokens in one pass: substituting by substring rewrote the names of other variables, so $PATH_EXTRA became /bin_EXTRA once PATH was set. realpath() applies the same denial decision as every other entry point, so where a denied symlink points is no longer disclosed by the one method that skipped the check. And the check-then-open race is closed by comparing the opened descriptor against the validated target: a path swapped in between is now a refusal with a recorded reason rather than a read. 446 checks pass across 225 cases. Live scan: 2.8s, 31 assets, no errors.
All six defects and the accuracy concern reproduce on 2c5a096. Credential-flag redaction now normalizes syntax before deciding. --api_key and --api-key are one flag, and an inline value may be separated by = or by :. The colon form was worse than reported: it leaked its own value and left the parser one token out of step, so the next real flag was eaten as a value and that value walked out in the clear. A flag is never treated as another flag's value now. Supply-chain pinning parses options before picking an operand. The image was "the first argument containing / or :", which a volume mount or a published port satisfies, so docker run -v /host:tag vendor/server:latest was called pinned - a wrong verdict in the direction that hides risk. The same fix covers npx and uvx, where a registry URL or a cache path supplied an apparent version. Ambiguity now resolves toward unpinned, and the shared parser replaced a second copy that had grown in the process probe. Registry policy is validated after decoding, the way the macOS preference payload already was. Windows executable identity is case-folded, so the same node.exe reported in different casing no longer splits config from runtime. Hostnames come from a URL parser rather than a split on the first colon, which was turning http://[::1]:8000 into "[". And a bundle that declares nothing runnable is recorded as a malformed bundle rather than as an MCP server with an empty command - inventing a server inflates the count and can raise a finding about something that cannot run. 463 checks pass across 231 cases. Live scan: 2.8s, 31 assets, no errors.
All six reproduce on cc96b79, and one of them is the consequence of an earlier fix rather than an oversight. Identity no longer contains secret material. Round three made identity use unredacted arguments so the config and runtime channels would agree; that made rotating a token read as an uninstall followed by an install, and made the public asset id a hash of a credential. Both channels now redact through the same function *inside* server_identity, so callers cannot disagree about it and a rotated secret leaves identity untouched. The handwritten TOML subset is gone. It split quoted keys on their dots and arrays on commas inside strings, so [mcp_servers."team.server"] became a server named "team" with no command and no arguments - silently. tomllib is used where available with tomli behind it, added as a dependency for Python 3.9. The YAML subset that remains now reports constructs it cannot represent instead of mis-parsing them. Pinning reads specifications rather than looking for an "@": 1.x, 1.2.*, beta, npm:other, workspace:* and github:user/repo are all mutable and were all called pinned. Docker option parsing is inverted to recognize booleans and treat everything else as value-bearing, because a table of value-taking options is only ever as complete as the day it was written - --runtime was already missing. Ambiguity resolves toward unpinned. Redaction handles a value that begins with a hyphen and Windows /flag:value syntax, and one normalization now serves identity, correlation and classification alike, so DOCKER.EXE and C:\Tools\npx.exe no longer fall through to "unknown, pinned". A live scan then caught a false positive the round-four tightening had missed: a shell's argv is arbitrary user text, and a snapshot wrapper whose command line mentioned a path containing "mcp" was being reported as an undeclared server. A shell is never an MCP server; one a config genuinely declares is recovered by correlation. 481 checks pass across 237 cases. Live scan: 2.8s, 31 assets, no errors, and the two MCP servers this machine actually declares.
…ence Found by scanning a real Ubuntu 24.04 container rather than a fixture. /bin is a symlink to usr/bin on any usrmerge system, so one binary has two spellings. The npm bin symlink is relative, so resolving it against the literal parent produced /lib/node_modules/... by way of /bin and /usr/lib/node_modules/... by way of /usr/bin: two merge keys for one file, and every PATH-installed agent counted twice on every mainstream Linux distribution. Relative link targets now resolve against the canonicalized parent, and the PATH scan deduplicates directories by identity rather than by spelling, which also stops it walking every binary on the box twice. /usr/lib/node_modules joins the global npm roots. Both the distro nodejs package and the NodeSource builds install there, so its absence meant no package observation was ever raised on mainstream Linux: the version had to come from executing the binary, and the pkg: merge key was never available. config_scope resolves by precedence rather than by arrival order. A server pushed by enterprise policy that the user had also declared reported as "user" because that config happened to be read second, inverting the one field an operator uses to tell corporate policy from something an employee added. Packaging: adr_sensor imports the observability plane lazily, so the collector runs on an endpoint that has only the standard library, and catalog.json is declared as package data so the installed wheel actually contains it.
Discovery was never part of the Sensor. It shared a package only because that is where it was first written: nothing under discovery/ imported from adr_sensor, and nothing in adr_sensor reached into discovery except one subcommand dispatch in the CLI. The two planes have different dependencies and different deployment stories, and the repository layout now says so. Sensor/adr_sensor/discovery/ becomes Discovery/adr_discovery/, and Sensor/tests/discovery/ becomes Discovery/tests/. The package ships its own pyproject with an adr-discovery console script in place of `adr-sensor discover`. The tomli dependency and the catalog.json package-data entry move with it, because both existed only for discovery and the Sensor wheel should not carry either. With discovery gone the lazy-import scaffolding in adr_sensor is dead. Its whole purpose was to let the collector run on an endpoint that has only the standard library, without dragging in tabulate by way of .observer. The package __init__ and the CLI both go back to importing the observability plane directly. CI gains a Discovery job on the same 3.9 through 3.13 matrix as the Sensor, behind a stable "Discovery tests" check name. Nothing about the collector's behaviour changed. 490 fidelity checks pass across 241 cases from the new location, the Sensor's 123 tests are unaffected, both packages lint clean, a live scan still reports 31 assets with no errors, and the built wheel contains catalog.json.
The suite is the least self-explanatory part of Discovery. It reads like a fixture pile until you notice that every case builds a real endpoint on disk and runs a real scan through the same entry point the CLI uses, and that the output is a scorecard rather than a pass count on purpose. tests/README.md covers the World DSL, the expectation helpers, the five groups, and what makes phantom and counting cases carry more weight than presence checks. Writing it turned up a bug in the runner. `only` was argv[1] unconditionally, so `run_suite.py -v` filtered for cases whose id starts with "-v" and reported "0/0 checks pass" - the one invocation that asks to see every check silently ran nothing, and said so in a way that looked like success. Only a bare word is a filter now. 490 checks across 241 cases still pass, and the four documented invocations all do what the README says they do.
…-end run The old tests/README.md documented the fixture harness - the World builder, the expectation helpers, how to add a case. Useful, but it described how the fast suite is implemented rather than how the collector is measured, and someone asking "is Discovery accurate?" found no answer in it. tests/README.md is now the end-to-end methodology. Three VMs, one per operating system, because launchd, the Windows registry, Task Scheduler and GUI app bundles are exactly the surfaces four probes exist to read and no container has any of them. A run restores a clean snapshot, scans for a baseline, installs a manifest of real tools, scans again, and scores the delta against the manifest. The comparison is the result. The manifest is grouped the way the collector reports: AI tools split by evidence channel, MCP servers split into declaration sites and launch forms, the programmable surface, and agents by liveness. It covers all 42 catalog entries - a catalog entry with no manifest row is a tool we claim to recognize and never verify. Negative controls are a first-class table, since without them a collector that reports everything scores perfectly. Scoring separates DUP from TP rather than folding it in. A tool installed once and reported twice is not a partial success; it inflates a fleet inventory, and it is the defect class that has recurred most. The fixture-harness documentation moves to tests/FIXTURE_SUITE.md, unchanged apart from a pointer. The two instruments are complementary: fixtures have a perfect oracle and only contain what someone imagined, so they catch regressions; the VM run has input nobody predicted, so it discovers defects. Defects found there should come back as R-group cases.
…lone-module # Conflicts: # README.md
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
2 participants
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.
Adds the ADR Discovery endpoint collector and makes it a top-level module alongside
Sensor/andDetection/, rather than a subpackage of the Sensor.What is here
The collector.
Discovery/adr_discovery/inventories the AI tools present on an endpoint: desktop apps, CLI coding agents, IDE and browser extensions, local model runtimes, MCP servers, and the skills, hooks and scheduled jobs that make an agent programmable. Ten probes feed a six-stage pipeline — enumerate, fingerprint, infer, resolve, rank, report — against a 42-entry catalog, with an open-world scorer that queues probable-AI-but-unrecognized surfaces for review instead of dropping them.Its own package.
adr-discoveryconsole script, ownpyproject.tomland lockfile. Thetomlidependency and thecatalog.jsonpackage-data entry moved off the Sensor, since both existed only for discovery. Standard-library-only on Python 3.11+, so it deploys to hosts that cannot reach PyPI.Why it is not part of the Sensor. Nothing under
discovery/ever imported fromadr_sensor; the only coupling in the other direction was a singlediscoversubcommand dispatch. The two planes have different dependencies and different deployment stories.Testing
Two instruments, documented separately:
Discovery/tests/FIXTURE_SUITE.md— the fast per-commit suite. 241 cases build synthetic endpoints on disk and run a real scan through the samediscover()the CLI uses. 490 checks, about four seconds, any CI box.Discovery/tests/README.md— the end-to-end measurement. Three VMs (macOS, Ubuntu, Windows), a manifest of real tools installed per OS covering all 42 catalog entries, scored as TP/FP/FN/DUP against what the collector reports. This one is a spec; it is not implemented yet.They are complementary. Fixtures have a perfect oracle but only contain situations someone imagined, so they catch regressions. The VM run has input nobody predicted, so it discovers defects — which is not theoretical: the usr-merge duplication and the missing
/usr/lib/node_modulesroot in this branch were both found by scanning a real Ubuntu container while 400+ fixture checks stayed green.CI gains a Discovery job on the same 3.9–3.13 matrix as the Sensor.
Verification
catalog.jsonNote on #52
This supersedes #52, which proposed the README
Discovery/row againstmainback when the directory did not exist. The same README changes are included here, and the links now resolve. #52 should be closed rather than merged.🤖 Generated with Claude Code