* tests: add GAP-01 tar security and GAP-02 SHA256 verification test suites GAP-01 (test_safe_extract.py): - CWE-22 traversal via ../ in tar header names (3 cases) - Absolute path rejection in tar entries (2 cases) - Symlink hardlink skip (2 cases each) - Hardlink skip - Deep traversal rejection - Deep valid path extraction - Empty tar noop - Normal operation smoke test - zipfile placeholder (documents no zip hardening yet) GAP-02 (test_sha256_verification.py): - _is_hex validation (4 cases) - _sha256_file empty/small/large/binary/not-found (5 cases) - _walk_files excludes dirs/deterministic/set equality (3 cases) - verify_plugin_sha256 empty plugin/excludes plugin.yaml/invalid format (3 cases) - compute_plugin_sha256 stable/deterministic order/content changes exclusion (4 cases) - CLI verify-sha256 exit zero/nonzero/file-not-dir/error message (4 cases) - Round-trip compute→verify (1 case) - Mismatch returns False (1 case) Total: 37 new test cases, all passing. 180 passed / 1 skipped across full suite (excluding broken conftest import in test_call_peer_errors.py). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: add KI-007 (_is_hex TypeError gap) and KI-008 (test_call_peer_errors conftest) KI-007: _is_hex raises TypeError on non-strings instead of returning False; guard with isinstance(value, str) check. KI-008: test_call_peer_errors.py imports tests.conftest which doesn't exist; fix import or create conftest.py stub. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Molecule AI SDK Lead <sdk-lead@agents.moleculesai.app> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
249 lines
9.2 KiB
Markdown
249 lines
9.2 KiB
Markdown
# Known Issues — molecule-sdk-python
|
|
|
|
Issues identified in source but not yet filed as GitHub issues (GH_TOKEN
|
|
unavailable in automated agent contexts). Each entry has: location,
|
|
symptom, impact, suggested fix.
|
|
|
|
Format per entry:
|
|
```
|
|
## KI-N — Short title
|
|
|
|
**File:** `<path>:<line>`
|
|
**Status:** TODO comment / identified / partially fixed
|
|
**Severity:** Critical / High / Medium / Low
|
|
**Platform phase:** (optional — which Phase 30 sub-phase is affected)
|
|
|
|
### Symptom
|
|
...
|
|
|
|
### Impact
|
|
...
|
|
|
|
### Suggested fix
|
|
...
|
|
---
|
|
```
|
|
|
|
---
|
|
|
|
## KI-001 — RemoteAgentClient does not implement inbound A2A server
|
|
|
|
**File:** `molecule_agent/client.py`
|
|
**Status:** Known limitation; not yet implemented
|
|
**Severity:** Medium
|
|
**Platform phase:** Phase 30.8b
|
|
|
|
### Symptom
|
|
`RemoteAgentClient` can call other workspaces via A2A (outbound), but cannot
|
|
receive inbound A2A calls. Any workspace that tries to delegate to or message
|
|
this agent will get a connection refused or timeout.
|
|
|
|
### Impact
|
|
Agents running outside the platform's Docker network via `molecule_agent` are
|
|
one-directional. Platform agents cannot push work to them — the remote agent
|
|
must poll or be provisioned with a publicly reachable webhook endpoint.
|
|
|
|
### Suggested fix
|
|
Add an `A2AServerMixin` class that exposes a `FastAPI` or `flask` route
|
|
(`POST /a2a/inbound`) and runs in a background thread alongside the client's
|
|
heartbeat loop. Register the inbound URL with the platform via the
|
|
`/registry/discover` update endpoint when the server starts. See Phase 30.8b
|
|
in the platform `PLAN.md`.
|
|
|
|
---
|
|
|
|
## KI-002 — Delegation has no server-side idempotency key enforcement
|
|
|
|
**File:** `molecule_agent/client.py` (client-side SHA256 key)
|
|
**Status:** Partially mitigated client-side (SHA256 rounded-to-minute)
|
|
**Severity:** Medium
|
|
**Platform phase:** Phase 30.6
|
|
|
|
### Symptom
|
|
The client generates an idempotency key as `SHA256(task + current_minute)`, but
|
|
the platform's `POST /workspaces/:id/delegate` endpoint does not enforce
|
|
idempotency server-side. Two identical tasks sent within the same calendar
|
|
minute produce duplicate processing if the platform accepts both.
|
|
|
|
### Impact
|
|
A workspace container restart mid-delegation (e.g. liveness probe restart) that
|
|
fires the same delegation request twice will result in duplicate side-effects
|
|
(double commits, double API calls, double messages) if the platform has not yet
|
|
stored the first delegation's result.
|
|
|
|
### Suggested fix
|
|
Platform-side: accept an optional `idempotency_key` field in
|
|
`POST /workspaces/:id/delegate`, check for existing non-failed delegation with
|
|
the same `(workspace_id, idempotency_key)`, return HTTP 200 with existing ID
|
|
instead of creating a new row. Client-side key generation is correct; it is
|
|
the server that needs to honor it.
|
|
|
|
---
|
|
|
|
## KI-003 — `_safe_extract_tar` silently skips all symlinks
|
|
|
|
**File:** `molecule_agent/client.py:_safe_extract_tar`
|
|
**Status:** By design (security posture)
|
|
**Severity:** Low (misleading behavior)
|
|
|
|
### Symptom
|
|
When extracting plugin tarballs, `_safe_extract_tar` silently skips any entry
|
|
that is a symlink. This means plugin tarballs that legitimately use symlinks
|
|
for shared assets (e.g., `assets/logo.png -> ../shared/logo.png`) will be
|
|
silently omitted from the extracted plugin directory with no error or warning.
|
|
|
|
### Impact
|
|
Some valid plugins may appear to install successfully but be missing files at
|
|
runtime. This can manifest as confusing "file not found" errors that are hard to
|
|
trace to the install step.
|
|
|
|
### Suggested fix
|
|
Emit a `logger.warning()` for each skipped symlink so operators can see what
|
|
was dropped. Alternatively, allow safe relative symlinks (those resolving
|
|
within the extraction root) while blocking absolute symlinks and `..`-escaping
|
|
symlinks. Document the behavior in the plugin authoring guide.
|
|
|
|
---
|
|
|
|
## KI-004 — Token file races between concurrent instances of RemoteAgentClient
|
|
|
|
**File:** `molecule_agent/client.py` (token caching)
|
|
**Status:** Identified
|
|
**Severity:** Low
|
|
|
|
### Symptom
|
|
Multiple `RemoteAgentClient` instances sharing the same `workspace_id` write to
|
|
the same token cache file (`~/.molecule/<workspace_id>/.auth_token`). If two
|
|
instances start simultaneously, the file read/write is not atomic — one
|
|
instance may read a partially-written token or overwrite a valid token with an
|
|
older one.
|
|
|
|
### Impact
|
|
On a cold start with multiple workers for the same workspace, some workers may
|
|
fail to register because their token is stale. The platform refuses to issue a
|
|
second token when one exists on disk.
|
|
|
|
### Suggested fix
|
|
Use a file-based lock (e.g. `fcntl.flock` or `portalocker`) around token read
|
|
and write operations. Alternatively, use per-process token storage (in-memory)
|
|
and only write to disk as a recovery fallback.
|
|
|
|
---
|
|
|
|
## KI-005 — `validate_plugin` does not check for secrets in bundle manifests
|
|
|
|
**File:** `molecule_plugin/manifest.py:validate_manifest`
|
|
**Status:** Not yet implemented
|
|
**Severity:** High
|
|
|
|
### Symptom
|
|
`validate_manifest` does not scan the `env:` or `secrets:` fields of a
|
|
`plugin.yaml` for hardcoded credentials (API keys, passwords, tokens). Plugin
|
|
authors could accidentally commit secrets into what should be a generic bundle.
|
|
|
|
### Impact
|
|
Secrets committed to a plugin manifest are visible in the repo and any tarball
|
|
published to PyPI or the plugin registry. Per platform constraints
|
|
(`constraints-and-rules.md`), bundles must never contain secrets.
|
|
|
|
### Suggested fix
|
|
Add a `validate_no_secrets()` check in `validate_manifest` that scans all
|
|
string values in the manifest for patterns matching common secret formats
|
|
(`sk-`, `ghp_`, ` Bearer `, 32+ char hex strings, etc.). Return a
|
|
`ValidationError` with level `HIGH` if any are found, even in example or
|
|
placeholder values. Add a corresponding test with a manifest containing a
|
|
known secret pattern.
|
|
|
|
---
|
|
|
|
## KI-006 — Plugin content integrity not verified client-side (RESOLVED)
|
|
|
|
**File:** `molecule_agent/client.py:verify_plugin_sha256`, `molecule_plugin/manifest.py:validate_manifest`
|
|
**Status:** ✅ Implemented — see SDK PR on `docs/add-claude-md` branch
|
|
**Severity:** Medium (mitigated by platform-side pinned-ref enforcement from molecule-core PR #1019)
|
|
|
|
### Symptom
|
|
`install_plugin()` downloaded and extracted plugin tarballs with no client-side
|
|
content verification. A compromised platform registry serving a tampered tarball
|
|
under a valid pinned-ref would pass `_safe_extract_tar` (no `..` or absolute
|
|
paths) but could contain a malicious `setup.sh`.
|
|
|
|
### Resolution
|
|
Added:
|
|
- `verify_plugin_sha256(plugin_dir, expected)` — computes a content-addressed
|
|
manifest hash over sorted `(relative_path, SHA256(content))` pairs; deterministic
|
|
regardless of extraction order or timestamps.
|
|
- `install_plugin()` reads `plugin.yaml → sha256` after atomic rename and before
|
|
`setup.sh`; mismatches raise `ValueError` and delete the plugin directory.
|
|
- `PLUGIN_YAML_SCHEMA` gains an optional `sha256` field (64-char lowercase hex).
|
|
- `validate_manifest()` validates `sha256` format when present.
|
|
|
|
Platform-side (molecule-core PR #1019) enforces source integrity (pinned git SHAs
|
|
or semver tags). SDK-side closes the content-integrity gap. Together they cover
|
|
both the "which code was fetched" and "did it arrive intact" axes.
|
|
|
|
Authors should add `sha256` to their `plugin.yaml` (generate with
|
|
`python -m molecule_agent verify-sha256 <plugin-dir>`) and commit it alongside
|
|
the plugin content.
|
|
|
|
---
|
|
|
|
## KI-007 — `_is_hex` raises `TypeError` on non-string arguments instead of returning `False`
|
|
|
|
**File:** `molecule_agent/client.py:_is_hex`
|
|
**Status:** Identified
|
|
**Severity:** Low
|
|
|
|
### Symptom
|
|
`_is_hex` is called inside `verify_plugin_sha256` after a length check. When
|
|
passed a non-string argument (e.g. `None`, an `int`, a `list`), `int(value, 16)`
|
|
raises `TypeError: int() can't convert non-string with explicit base` instead of
|
|
returning `False`. `verify_plugin_sha256` would surface a confusing `TypeError`
|
|
rather than a descriptive validation error.
|
|
|
|
### Impact
|
|
Any bug passing a non-string `expected` to `verify_plugin_sha256` produces a
|
|
confusing `TypeError` instead of the intended `ValueError`. Low-probability
|
|
edge case (function is internal), but violates the principle that validator
|
|
functions should never raise unexpected exceptions.
|
|
|
|
### Suggested fix
|
|
Guard at the top of `_is_hex`:
|
|
```python
|
|
def _is_hex(value: str) -> bool:
|
|
if not isinstance(value, str):
|
|
return False
|
|
try:
|
|
int(value, 16)
|
|
return True
|
|
except ValueError:
|
|
return False
|
|
```
|
|
|
|
---
|
|
|
|
## KI-008 — `test_call_peer_errors.py` fails collection due to missing `tests/conftest.py`
|
|
|
|
**File:** `tests/test_call_peer_errors.py:19`
|
|
**Status:** Identified
|
|
**Severity:** Low
|
|
|
|
### Symptom
|
|
`pytest tests/` fails to collect any tests:
|
|
```
|
|
ModuleNotFoundError: No module named 'tests.conftest'
|
|
```
|
|
The file imports `from tests.conftest import _CaptureHandler` using the
|
|
`tests.` package prefix. The cloned repo has no `conftest.py` and uses a
|
|
convention inconsistent with the rest of the test suite (which uses root-relative
|
|
imports).
|
|
|
|
### Impact
|
|
CI running `pytest tests/` errors before collecting any tests at all. Requires
|
|
`--ignore=tests/test_call_peer_errors.py` to run the full suite.
|
|
|
|
### Suggested fix
|
|
Either create `tests/conftest.py` with the `_CaptureHandler` stub definition,
|
|
or change the import to use a direct module import (`from conftest import
|
|
_CaptureHandler`) consistent with the rest of the suite.
|