Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions tools/egress-gateway/tool.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,19 @@ action (PRINCIPLE 10). This guarantee rests on two pillars:
all other `substrate:*` tools are network-free by design.

2. **The validator enforces the invariant.** `tools/skill-and-tool-validator/`
check #21 (`no-telemetry-import`) scans the source of every `substrate:*`
tool for network-calling imports (`requests`, `httpx`, `aiohttp`,
`urllib.request`, `http.client`, `socket`) and flags any hit as a SOFT
advisory. A substrate tool that accidentally grows a network import is
caught before it is merged.
check #21 (`no-telemetry-import`) scans every Python file owned by a
`substrate:*` tool — under `src/` and at the tool root alike, excluding
`tests/` — for network-calling imports (`requests`, `httpx`, `aiohttp`,
`urllib.request`, `http.client`, `socket`, in both `import x` and
`from x import` form) and flags any hit as a SOFT advisory. A substrate
tool that accidentally grows a network import is surfaced in the
validator's output before it is merged.

Two limits worth stating plainly. The check is **advisory unless
`--strict`**, so an accidental import can merge on a run that does not
pass the flag — it is a tripwire the reviewer reads, not a gate that
stops the merge. And the library list is deliberately non-exhaustive:
it catches the common accidents, not a determined author.

The declared egress surfaces as of this writing:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@
(re.compile(r"^\s*from\s+urllib\s+import\s+(?:\w+\s*,\s*)*request\b"), "urllib.request"),
(re.compile(r"^\s*(?:import|from)\s+http\.client\b"), "http.client"),
(re.compile(r"^\s*import\s+socket\b"), "socket"),
(re.compile(r"^\s*from\s+socket\s+import\b"), "socket"),
]


Expand Down Expand Up @@ -3637,11 +3638,19 @@ def validate_no_telemetry_imports(root: Path | None = None) -> Iterable[Violatio
if any(e.startswith(_ADAPTER_CONTRACT_PREFIX) for e in entries):
continue # declared contract:* adapter — network is expected

src_dir = tool_dir / "src"
if not src_dir.is_dir():
continue

for py_path in sorted(src_dir.rglob("*.py")):
# Scan every Python file the tool owns, not just ``src/``. Several
# substrate tools keep their Python at the tool root
# (``pr-management-stats/dashboard.py``,
# ``security-tracker-stats-dashboard/render.py``, …), and a
# ``src/``-only scan silently exempted them while
# ``tools/egress-gateway/tool.md`` promised that "a substrate tool
# that accidentally grows a network import is caught before it is
# merged". ``tests/`` is excluded: a test may legitimately import a
# network module to assert it is *not* reachable.
for py_path in sorted(tool_dir.rglob("*.py")):
rel_parts = py_path.relative_to(tool_dir).parts
if "tests" in rel_parts:
continue
if any(part in _LICENSE_SKIP_PATH_PARTS for part in py_path.parts):
continue
try:
Expand Down
52 changes: 52 additions & 0 deletions tools/skill-and-tool-validator/tests/test_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -5199,3 +5199,55 @@ def test_violation_message_mentions_principle_10(self, tmp_path: Path) -> None:
)
violations = list(validate_no_telemetry_imports(root))
assert any("PRINCIPLE 10" in v.message for v in violations)

def test_tool_root_python_outside_src_is_scanned(self, tmp_path: Path) -> None:
# Several substrate tools keep Python at the tool root rather than
# under src/ (pr-management-stats/dashboard.py and friends). A
# src/-only scan exempted every one of them while tool.md promised
# the opposite.
root = self._make_tool(
tmp_path,
name="root-script-substrate",
readme=self._SUBSTRATE_README,
src_files={"root_script_substrate/__init__.py": "# SPDX-License-Identifier: Apache-2.0\n"},
)
tool_dir = root / "tools" / "root-script-substrate"
(tool_dir / "dashboard.py").write_text("# SPDX-License-Identifier: Apache-2.0\nimport requests\n")
violations = list(validate_no_telemetry_imports(root))
assert len(violations) == 1
assert violations[0].path.name == "dashboard.py"
assert "requests" in violations[0].message

def test_from_socket_import_is_flagged(self, tmp_path: Path) -> None:
# Every sibling pattern accepts both `import x` and `from x import`;
# socket accepted only the former.
root = self._make_tool(
tmp_path,
name="from-socket-substrate",
readme=self._SUBSTRATE_README,
src_files={
"from_socket_substrate/__init__.py": (
"# SPDX-License-Identifier: Apache-2.0\nfrom socket import socket\n"
)
},
)
violations = list(validate_no_telemetry_imports(root))
assert len(violations) == 1
assert "socket" in violations[0].message

def test_tests_directory_is_not_scanned(self, tmp_path: Path) -> None:
# A test may legitimately import a network module to assert it is
# not reachable; broadening the scan must not start flagging those.
root = self._make_tool(
tmp_path,
name="tested-substrate",
readme=self._SUBSTRATE_README,
src_files={"tested_substrate/__init__.py": "# SPDX-License-Identifier: Apache-2.0\n"},
)
tool_dir = root / "tools" / "tested-substrate"
(tool_dir / "tests").mkdir()
(tool_dir / "tests" / "test_net.py").write_text(
"# SPDX-License-Identifier: Apache-2.0\nimport requests\nfrom socket import socket\n"
)
violations = list(validate_no_telemetry_imports(root))
assert violations == []