Skip to content

Declare accepted indicator types on process(), not in defaults.py - #289

Merged
spierenburg merged 2 commits into
mainfrom
feat/accepts-on-handler
Jul 26, 2026
Merged

Declare accepted indicator types on process(), not in defaults.py#289
spierenburg merged 2 commits into
mainfrom
feat/accepts-on-handler

Conversation

@spierenburg

Copy link
Copy Markdown
Collaborator

What

The indicator-type filter that routes a shared bind like @ioc lived in an ACCEPTS list in each module's defaults.py — a declaration separate from the process() that actually consumes those types, so the two could silently drift.

This moves the contract onto the handler with a @cmdutils.handles(...) decorator, so the accepted types sit next to the code that looks them up and can't fall out of sync.

# CIDR: netblocks are looked up via the check-block endpoint, single addresses via check.
@cmdutils.handles(cmdutils.IP, cmdutils.IPV6, cmdutils.CIDR)
def process(command, channel, username, params, files, conn):
    ...

Honorable mention

The idea comes from @TTycho's typing-for-modules branch, which declares each command's accepted types directly on the function signature (def search(parameters: list[ASN], ...)) rather than in a side table. Credit to him for the core insight — the type contract belongs on the handler. This PR adapts it to our current architecture rather than porting it wholesale:

  • Decorator, not a type annotation. TTycho's indicator is the whole typed parameter; our fixed process(command, channel, username, params, files, conn) signature has params also carrying non-indicator options (abuseipdb's max-age, hibp's search-type), so a bare params: list[IP] hint would misrepresent it — and restructuring the universal signature across ~40 modules is riskier than the drift it fixes.
  • Stdlib-only. Keeps our existing cmdutils.classify() classifier so the dependency-free python -m unittest runner still works (his branch pulls in validators + tldextract).

Changes

  • cmdutils.handles(*types) attaches the declared types to process(); the loader reads that attribute through normalise_accepts into the command entry the dispatcher already gates on. No decorator → accept-anything, unchanged and backwards-compatible.
  • matterbot.py: read the decorator off process() instead of defaults.ACCEPTS, and drop the settings.py ACCEPTS override — what indicator types a module's code handles is a code fact, not deployment config.
  • Migrated all 16 annotated modules atomically (removing the defaults.ACCEPTS read breaks any un-migrated one). Relocated the substantive per-module WHY (netblock endpoint, IPv4-only, URL scheme gate) next to the decorator; dropped generic boilerplate.
  • Tests read the decorator via AST (dependency-free) instead of defaults.ACCEPTS, so the shipped-invariant checks still catch a bad edit to a real module.

Testing

python -m unittest tests.test_cmdutils39/39 pass (3 new decorator tests + the 16-module routing-invariant checks that read the real decorators).

Note: the full suite has pre-existing failures unrelated to this change (confirmed by stashing this work and re-running) — feedparser not installed in the runner, and a separate syntax error in modules/fortinet/feed.py:95 already on main. Neither is touched here.

@spierenburg

Copy link
Copy Markdown
Collaborator Author

Merge order — blocked on #290.

The failing stdlib unittest check here is not caused by this change. It's a pre-existing syntax error in modules/fortinet/feed.py on main (a bad merge left an unterminated try: — fixed in #290). Confirmed by stashing this branch's changes and re-running the suite: the identical failures=1, errors=6 appears without any of my changes.

This PR touches only commands/ + the loader + tests/test_cmdutils.py — nothing under modules/fortinet/. Its own tests pass (tests.test_cmdutils → 39/39).

Merge path: merge #290 first, then this branch gets rebased on main and CI goes green. Holding the rebase until #290 lands so the two stay cleanly separated.

The indicator-type filter that routes a shared bind like @IOC lived in an
ACCEPTS list in each module's defaults.py -- a separate declaration that could
silently drift from what the command's process() actually looks up.

Move the contract onto the handler itself with a @cmdutils.handles(...)
decorator, so the accepted types sit next to the code that consumes them and
cannot fall out of sync. (Idea from TTycho's typing-for-modules branch, which
puts accepted types on the function signature; adapted to our stdlib-only
classifier and our fixed process() signature, where params also carries
non-indicator options so a bare type annotation would misrepresent it.)

- cmdutils.handles(*types): attaches the declared types to process(); the loader
  reads that attribute through normalise_accepts into the command entry the
  dispatcher already gates on. No decorator -> accept-anything, unchanged.
- matterbot.py: read the decorator off process() instead of defaults.ACCEPTS
  (and drop the settings.py ACCEPTS override -- the type a module handles is a
  code fact, not deployment config).
- Migrate all 16 annotated modules; relocate the substantive per-module WHY
  (netblock endpoint, IPv4-only, URL scheme gate) next to the decorator.
- Tests read the decorator via AST (dependency-free) instead of defaults.ACCEPTS,
  so the shipped-invariant checks still catch a bad edit to a real module.
@spierenburg
spierenburg force-pushed the feat/accepts-on-handler branch from 10f3e4f to 6633e62 Compare July 26, 2026 12:31
@spierenburg
spierenburg merged commit 3914e60 into main Jul 26, 2026
5 checks passed
spierenburg added a commit that referenced this pull request Aug 4, 2026
Following #289, the opt-in is declared with @cmdutils.aitool on process()
rather than AITOOL in defaults.py, so the README, the AI: config block and
the _registry() comment all still described the old shape.

Also states the property the move buys, which the old prose did not: the
decorator cannot be set from defaults.py/settings.py, so the config lists
can only ever subtract from the exposed set -- they cannot widen it past
what a developer marked safe.
@spierenburg
spierenburg deleted the feat/accepts-on-handler branch August 4, 2026 21:07
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.

1 participant