_module_effective_mode's new externally-capable upgrade (a2d3bf) checks only DECLARED allowed_destinations/requires — a module that declares neither stays classified "read" while its handler calls the unconditionally-injected, ungated HTTP helpers (v1.4.0)
Reproduction steps:
- v1.4.0 adds workbench/routes/modules.py's _module_effective_mode() /
_normalize_module_modes() (commit a2d3bf, attributed to dinkarshweta),
specifically to close the historical self-report bypass (bugs_found_v0.99.txt
finding #16 — "a module can self-declare mode:'read' on a command whose
handler performs a real external effect, bypassing the approval airlock
entirely"):
def _module_effective_mode(cmd, manifest):
mode = str((cmd or {}).get("mode") or "read")
if mode in ("write_requires_approval", "write_blocked"):
return mode
egress = bool((manifest or {}).get("allowed_destinations"))
needs_creds = bool((cmd or {}).get("requires"))
if egress or needs_creds:
return "write_requires_approval"
return mode
This upgrades a self-declared "read" ONLY when the manifest declares
allowed_destinations or the command declares requires.
- Every module — regardless of what it declares — receives the SAME
always-injected, unconditional HTTP helpers in its handler namespace
(routes/modules.py __rc_helpers__, ~line 1190-1208): http_post_json,
http_get_json, http_delete_json, http_patch_json, http_post_form. Their
real definitions (workbench/studio_server.py _http_post_json /
_http_method_json, lines ~4801-4900) take a raw url argument and fire
the request with NO consultation of allowed_destinations, no dependency on
command-level requires, and no sandbox network check (module_sandbox.py's
network gate is itself a SEPARATE, ALSO-optional mechanism that only
engages when the manifest additionally declares requires: {network:[...]}
— a module need not opt into that either).
- Run repro_module_effective_mode_undeclared_egress_bypass_v140.py against a
clean v1.4.0 extraction. It drives the REAL, unmodified
_module_effective_mode / _normalize_module_modes with a manifest that
declares neither allowed_destinations nor command requires:
manifest has allowed_destinations: False
command has requires: False
_module_effective_mode(cmd, manifest) -> read
mode after _normalize_module_modes (the load-time upgrade pass) -> read
CONFIRMED
Expected:
Per the fix's own stated goal, quoted in its comment: "an externally-capable
command is therefore NEVER eligible for the no-approval read class." A
command whose handler CAN reach an external destination — which every module
handler can, via the unconditionally-injected HTTP helpers, independent of
any declaration — must not be treated as safely local just because its
manifest happens to omit the two specific fields this check looks for.
Actual:
"Externally capable" is detected purely by DECLARATION (allowed_destinations
present, or requires present), and declaration is exactly the thing a
malicious module author controls and can simply omit. A module command that
declares neither, and whose handler calls http_post_json(url, payload)
directly, keeps its self-declared "read" mode after _normalize_module_modes
— the exact load-time gate this release added specifically so "every
downstream reader (execute_command via resolve_status, the workflow plan via
_module_action_class, the palette, MCP) sees one governed classification."
Every one of those downstream readers still sees "read" for this command,
so (per resolve_status's unchanged default-branch semantics, and
_module_action_class's "read" -> "reversible" mapping) it executes with no
airlock hold, plans as a safe reversible effect, and is callable as a free
MCP read — while actually POSTing to any destination the handler chooses.
Suggested fix:
Detect capability from what the module CAN reach, not what it declares.
Two complementary options:
1. Make external-capability detection unconditional rather than declaration-
gated: any command whose manifest has no explicit, narrow
allowed_destinations block is treated as externally-capable by DEFAULT
(fail closed) — i.e. invert the check so a command is exempted from the
write_requires_approval upgrade only when the manifest POSITIVELY
declares it has no egress (e.g. "allowed_destinations": [] or an
explicit "local_only": true manifest flag), not merely when the field
is absent.
2. Independently, gate the injected HTTP helpers themselves the same way
allowed_destinations is documented to work: refuse (raise) any
http_*_json call whose target host is not in the module's declared
allowed_destinations, for every module — sandboxed or not — rather than
leaving them as bare, ungated urllib wrappers available to every command
regardless of its governance class. This closes the gap at the
capability source instead of only at the classification layer, and also
protects a command that WAS correctly upgraded to
write_requires_approval but whose handler could still reach an
undeclared destination once a human approves it.