feat(discovery): add the ADR Discovery endpoint collector and its README - #55
Closed
pengyuzhang wants to merge 20 commits into
Closed
feat(discovery): add the ADR Discovery endpoint collector and its README#55pengyuzhang wants to merge 20 commits into
pengyuzhang wants to merge 20 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
The manifest named categories but not contents. It said "at least three of the above are installed a second way" and gave locations for skills and agents rather than artifacts, so an implementer could not tell which tools to put on the VMs. A harness cannot execute "and a few others". Every item is now enumerated with a stable id, and the id is the unit the runner executes, that manifest.actual.json records an outcome against, and that a scorecard reports a miss under. 120 entries: 50 AI tools, 29 MCP servers, 19 skills and hooks, 12 agents, 10 negative controls. The vague parts are gone. The eight install-channel variants are named individually, including the usr-merge pair that duplicated every PATH-installed agent on mainstream Linux. Skills are exact paths rather than directories. Agents are the states a machine is left in before the second scan. Negative controls are specific software, including the two that make the phantom checks meaningful: a script whose path merely contains "mcp", and an in-house wrapper that must reach the review queue rather than the inventory. Checked rather than asserted: 120 ids, none duplicated, per-category counts matching the summary table, contiguous numbering, and all 42 catalog entries still covered.
Discovery had no README of its own since the module was extracted. This adds one, derived from the design document, so the folder explains itself without a reader having to reconstruct the argument from the code. It states the four targets and what is deliberately out of scope, the six sources the enumerator queries, the M1-M7 module split with the three cross-cutting concerns, the code layout that split implies and the import rules that keep it, the asset and coverage shapes, the redaction rules, the evaluation layers, and the build order. The status section is explicit that adr_discovery/ is what ships today and the layout below it is the target, so the file cannot be misread as describing the present package tree.
Places the README inside adr_discovery/ so it sits beside the code it describes. Relative links are re-based for the new depth: the tests documents become ../tests/, and LICENSE now resolves to the module's own Apache copy rather than the repository root.
The README moved under adr_discovery/ in the previous commit, which left the capability overview linking at a path that no longer exists.
# 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.
What this adds
Discovery/— the ADR Discovery endpoint collector, as a standalone top-level module alongsideSensor/andDetection/. It inventories the AI binaries, AI agents, MCP servers and skills present on an employee endpoint, fingerprints known tools against a catalog, and scores unknown ones for review rather than dropping them.Standard-library-only on Python 3.11+, so it deploys to hosts that cannot reach PyPI.
Contents
Discovery/adr_discovery/Discovery/adr_discovery/catalog.jsonDiscovery/adr_discovery/README.mdDiscovery/tests/The README
The last three commits add
Discovery/adr_discovery/README.md, written from the module's design document and placed beside the code it describes. It states the four discovery targets and what is deliberately out of scope, the six sources the enumerator queries, the M1–M7 module split with its three cross-cutting concerns, the code layout that split implies and the import rules that hold it in place, the asset and coverage shapes, the redaction rules, the evaluation layers, and the build order.Its status section is explicit that
adr_discovery/is what ships today and that the layout it describes is the target of an in-progress restructure, so the file cannot be misread as documenting the present package tree.The capability overview in the root
README.mdis re-pointed at the new path.Notes for review
main. GitHub reports the PR mergeable, but it is worth updating before merge.Discovery/itself now has no README, so that folder renders without a landing page on GitHub; the overview appears when you openadr_discovery/. Happy to leave a short pointer file atDiscovery/if reviewers would rather have one.coverage.out_of_scoperather than silently omitted, so a clean machine stays distinguishable from an unasked question.🤖 Generated with Claude Code