fix: path confinement, error sanitization, secret redaction - #1
Merged
Conversation
…nd streaming Class agents built with LangChain abstractions (ChatPromptTemplate, MessagesPlaceholder, HumanMessage/AIMessage/ToolMessage/SystemMessage, RunnableConfig, prompt | llm chains) were losing data at several boundaries: - AgentomaticCheckpointer used json.dumps(obj, default=str), which stringified BaseMessage objects to their repr() on checkpoint reload, breaking the LangGraph add_messages reducer and any resumed chain. Now uses LangGraph's own JsonPlusSerializer so messages, tool calls, datetimes, etc. round-trip as real objects. - langchain_adapter.messages_to_dict dropped tool_call_id/tool_calls/name, breaking OpenAI/Anthropic tool-call protocol continuity across turns. - langchain_adapter._dict_to_lc mis-classified "system" role dicts as HumanMessage, corrupting prompt/history reconstruction. - The generic REST/Studio output path (router_factory.coerce_agent_invoke_payload, AgentGraph.astream/astream_studio_events) had no message-aware serializer, so a class agent returning raw BaseMessage objects in state_to_output() could crash the JSON response or silently stringify messages. Added to_jsonable/json_default/message_to_dict helpers and wired them in. - The `--template langchain` scaffold didn't actually demonstrate ChatPromptTemplate/MessagesPlaceholder/RunnableConfig despite its own description promising it; rewritten to match, using a real prompt | llm chain with an explicit RunnableConfig per invocation. - CLAUDE.md / agent_guide.py's template enum was missing `langchain`. Also fixed ruff formatting drift across the tree and added regression tests for each of the above (checkpoint message round-trip, tool-call fidelity, system-role handling, streaming/REST serialization of raw messages). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
… pass Broad audit across auth, storage, pipelines, the optimize engine, and deploy tooling; fixes the concrete bugs found in each area. Security: - Timing-safe comparisons (hmac.compare_digest) for the API-key auth middleware and the control-plane token check, replacing plain `!=`. - Rate limiter no longer trusts a client-supplied X-Forwarded-For header by default — any caller could rotate it per request to get a fresh bucket and fully bypass the limiter. New opt-in `trust_proxy_headers` (env: AGENTOMATIC_RATE_LIMIT_TRUST_PROXY_HEADERS) for deployments behind a real proxy that overwrites the header. Storage: - Deleting a thread orphaned its LangGraph checkpoints (CheckpointModel has no FK to ThreadModel, since a checkpoint's thread_id need not correspond to a registered chat thread) — delete_thread() now explicitly cleans up checkpoints (and, on MemoryStore, feedback) instead of relying on cascade. - SQLite silently ignored the ondelete="CASCADE" on FeedbackModel/ SuspendedStateModel because FK enforcement is off by default per connection — now enabled via PRAGMA foreign_keys=ON on every connection. - list_checkpoints() had no default cap; a caller (including LangGraph's own checkpointer.alist()) omitting limit could load a thread's entire checkpoint history into memory. Both backends now cap at 1000 by default. Pipelines (DAG scheduling): - A step whose declared upstream FAILED would still run under a pipeline-level on_error="continue" policy, consuming missing/stale output instead of being skipped. Failures now cascade: a step is skipped when any declared upstream ended up FAILED, and that skip cascades to its own dependents. Condition-based skips are unaffected (existing behavior kept). Optimize engine: - FewShotBootstrapOptimizer.propose() could raise ZeroDivisionError when k_examples resolves to 0 (e.g. min(4, len(eval_results)) with empty results) — the `len(usable) < k_examples` guard never fires for k=0. Guard now also checks k_examples <= 0. Deploy: - The distroless docker-compose healthcheck used `curl -f ...`, but gcr.io/distroless/python3-debian12 has no shell and no curl — the container would report permanently unhealthy. Now uses the venv Python (present in the image) with urllib for both the generated compose file and the repo-root Dockerfile.distroless (which had no HEALTHCHECK at all). Misc: - tasks/progress.report_stage_sync() scheduled a fire-and-forget asyncio task with no reference kept anywhere; asyncio only holds a weak reference, so the task could be garbage-collected mid-execution, silently dropping the progress report. Now held in a module-level set until it completes. Regression tests added for every fix above; ruff/mypy/pytest (1977 passed, 47 skipped — unchanged, all live-server tests)/mkdocs all clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
… gap
Second deep-audit pass (A2A/tasks, connections/plugins/endpoints, control
plane/stacks/config, Studio backend/ingestion), fixing the two critical/high
findings plus two smaller correctness issues.
CRITICAL — Studio debug API auth bypass:
- Both AuthMiddleware and JWTAuthMiddleware skip-path sets contained the
bare prefix "/studio", intended only to exempt the static UI shell. But
path_is_skipped() does prefix matching, so "/studio" also exempted the
entire Studio debug REST API — /studio/agents/{name}/threads/{id}/state,
/resume, /update_state, etc. — from auth entirely. With auth or JWT
enabled (including --require-auth-globally), any unauthenticated caller
could read or mutate any agent's live run state, thread history, and
resume interrupted executions. Fixed to skip only "/studio/ui" (the same
pattern already used for "/docs" — the Swagger UI shell is public, the
API it documents is not).
HIGH — control-plane agent disable/enable bypassed via slug/name alias:
- An agent whose manifest slug differs from its folder name is mounted
under both (`_mount_agent_router`, so Studio's slug-based addressing
doesn't 404). ControlPlaneState tracked disabled agents by whichever
literal string was passed to POST /control/agents/{name}/disable, so
disabling by name left the slug-mounted alias (and vice versa) fully
live. disable_agent/enable_agent now resolve and toggle both aliases.
HIGH — TaskManager.shutdown() didn't wait for cancelled tasks:
- asyncio.Task.cancel() only schedules CancelledError at the task's next
suspension point; shutdown() closed the store immediately after, racing
a cancelled task's _finalize() (which persists its terminal status)
against the store's own disposal. Now gathers the tasks before closing.
MEDIUM — connection double-init race:
- DatabaseConnection/CustomConnection.initialize() checked-then-built with
an `await` in between and no lock, so two concurrent first-callers (e.g.
two requests racing at cold start) could each build and leak their own
engine/client, breaking the documented "one client per process" contract.
Now guarded with an asyncio.Lock + double-checked locking.
One existing test (test_jwt_defaults_include_studio) explicitly asserted
the buggy bare-"/studio" skip as expected behavior — rewritten to lock in
the fix instead.
Regression tests added for every fix; ruff/mypy/pytest (1985 passed, 47
skipped — unchanged, live-server tests)/mkdocs all clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…I hygiene Found by actually booting the platform and driving it over HTTP, then locking each finding down with an end-to-end test. Class agents + LangChain (the headline fixes): - `messages` and `thread_id` never reached `input_to_state()` on ANY HTTP path. `_input_from_state` dropped them as "conversation bookkeeping", so the scaffolded langchain template's history and RunnableConfig thread handling was dead code in production: a MessagesPlaceholder could never see prior turns. Both are now forwarded (BaseGraphAgent's node adapter too). - Agent-returned `messages` were silently DISCARDED from the API response. `_FRAMEWORK_RESULT_KEYS` filtered `messages` out of `output`, but AgentInvokeResponse has no `messages` field — so a conversational class agent's messages had nowhere to go and vanished. Now flows to `output.messages`. - Tool-call fidelity was destroyed on the chat path by a THIRD hand-rolled dict→message converter (after the two fixed previously): it dropped `tool_calls` and turned a `tool` turn into a HumanMessage, breaking the call/result pairing providers require on the next turn. The chat handler now delegates to the canonical `dict_to_messages`, and the memory manager's own converter learned tool_calls/ToolMessage/role aliases. - Class agents registered via `register_agent(class_instance=...)` had no `graph_fn`, so Studio showed an empty graph and streaming failed with "Agent has no graph_fn". It is now derived from the instance. That path also bypassed slug indexing; it now keeps the registry index consistent. CLI: - `agentomatic run` was broken under `uv run` (and any console-script install): uvicorn resolves "main:app" against sys.path, which does not include the project dir, so the documented quick-start failed with "Could not import module main". Now passes app_dir and exports PYTHONPATH for --reload. - The `langchain` template shipped in the registry but was missing from the CLI's hardcoded --template choices, making it unreachable. Choices are now derived from the registry so they cannot drift again. - Templates generated lint-dirty code (unused imports, f-strings without placeholders, over-long lines). All 23 templates are now clean. OpenAPI / observability: - Mounting each agent under both folder name and slug produced a duplicate operationId per route (~205 UserWarnings; duplicate ids break client codegen) and doubled the advertised surface. The slug mount is a compatibility alias, so it is excluded from the schema — still routes at runtime. Suite warnings: 142 -> 6; documented paths roughly halved. - OpenTelemetry console export defaulted ON whenever no OTLP endpoint was set (i.e. most deployments), dumping a full JSON span document to stdout for every request. Now opt-in via AGENTOMATIC_OTEL_CONSOLE. New end-to-end coverage: - tests/test_e2e_langchain_class_agent.py — a class agent using ChatPromptTemplate + MessagesPlaceholder + an LCEL chain + @tool + real message objects, driven through REST invoke/chat/SSE/Studio with a genuine FakeListChatModel runnable (LangChain is not mocked). - tests/test_studio_bundle_alignment.py — parses every API path the shipped Studio bundle calls out of its JavaScript and asserts each resolves to a real backend route: a permanent drift alarm for the checked-in UI bundle. - tests/test_template_scaffold_quality.py — every template must emit parseable, lint-clean code, and the langchain template must actually use the abstractions it advertises. - tests/test_production_readiness.py — operationId uniqueness, alias routing, opt-in OTel console, and the run-command sys.path fix. 2040 passed, 47 skipped; ruff/mypy/mkdocs clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…tack secrets
Three vulnerabilities, each reproduced by executing it against a running
platform rather than inferred from reading code.
CRITICAL — unauthenticated arbitrary file read/write via ingestion:
`MarkdownIngestRequest.source` / `output_dir` / `output_filename` were
caller-supplied strings passed straight to `Path(...)` with no confinement
anywhere in the tree. Confirmed working: `source=/etc/passwd` copied the file
contents into an attacker-chosen directory (auth is off by default, so this
needed no credentials); `output_dir` created and wrote to any writable path;
`output_filename=../../x` escaped the output directory and could clobber an
existing file — RCE-adjacent if aimed at a module on an import path.
New `agentomatic.ingestion.paths` confines every caller-supplied path to an
ingestion root (`AGENTOMATIC_INGESTION_ROOT`, defaulting to the CWD so ordinary
relative paths keep working). Symlinks are resolved before the check, and
`output_filename` must now be a bare filename. All four attacks above are
rejected with actionable errors; in-root ingestion is unaffected.
MEDIUM — credentials leaked through exception messages:
`f"Agent invocation failed: {exc}"` went straight into HTTP responses, so a
driver's DSN reached any caller who could trigger a failure. Verified: a
`postgres://user:HUNTER2@db:5432` credential appeared verbatim in /invoke,
/chat and the SSE stream. New `agentomatic.core.errors.client_safe_detail`
logs the full exception (with traceback) server-side and returns a sanitised
payload carrying a correlation `error_id`. `AGENTOMATIC_DEBUG_ERRORS=1`
restores raw text for local development. Pipeline YAML *validation* messages
are deliberately left verbatim — that feedback is about input the caller just
sent and carries no server state.
MEDIUM — stack secrets printed and written in clear text:
`stack show` echoed the raw YAML, so a literal `api_key` or a DB URL with
embedded credentials landed in terminal scrollback and CI logs. Worse,
`.env.example` (conventionally committed) was generated with literal secrets
copied in. New `agentomatic.stacks.redaction` masks secret-looking values
while leaving `${ENV_VAR}` indirections visible — they are the thing an
operator is usually trying to verify — and preserves comments and structure.
`stack show --reveal` prints verbatim when explicitly requested.
Also from the packaged-wheel release gate (which otherwise passed — Studio
assets are correctly packaged and served):
- `studio/serve.py` used printf `%s` with loguru's `{}` formatter, printing a
literal "%s" instead of the mount path.
- `doctor`'s install hint was unquoted (`pip install agentomatic[ui]`), which
is glob syntax in zsh/bash.
New env vars documented in CLAUDE.md and the agents-guide primer.
2055 passed, 47 skipped; ruff/mypy/mkdocs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…lint-clean
Verified by rendering every template in the registry and checking the output
the way a user's own CI would — from inside the scaffolded directory, under
ruff's *default* settings (a fresh project has no ruff config, so the defaults
are what its first CI run enforces).
Functional bug — the `plugin` template generated code that crashed on import:
`_plugin_predict_py` and `_plugin_eval_py` derived the class name with
`name.replace("_", "").title()` ("ag_plugin" -> "Agplugin") while `_plugin_py`
used `.replace("_", " ").title().replace(" ", "")` ("AgPlugin"). The generated
`predict.py`/`eval.py` therefore imported classes that were never defined:
ImportError: cannot import name 'AgpluginPlugin' from 'ag_plugin.plugin'
`py_compile` never caught this because the files are syntactically valid — only
an actual import surfaces it. Both now use the same transform as `_plugin_py`,
and the test suite imports the rendered scripts rather than just compiling them.
Lint cleanliness — 15 findings across 6 templates, all now fixed at the source:
- 11 E501 long lines wrapped (several interpolate the agent name, so they got
longer still for any name longer than the sample).
- 4 I001 import-ordering issues: `pydantic`/`langchain_core`/`langgraph` were
split into their own group above `agentomatic`, but isort sorts them into one
third-party block.
- E402 in the `train.py`/`eval.py` scripts: these genuinely must mutate
`sys.path` before importing project modules, so they now carry a file-level
suppression explaining why, rather than being waived by the test.
The quality gate is correspondingly tightened: the E402 allowance is removed
entirely (nothing needs waiving now), `I` is added to the selected rules, and
ruff runs with `cwd` set to the scaffolded directory. That last part matters —
running it from the repo root let ruff's isort resolve `src/agentomatic` and
misclassify `agentomatic` as a first-party import, so the gate was checking
something a user would never see.
All 16 templates: render, compile, import, and lint clean.
2055 passed, 47 skipped; ruff/mypy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…iet OTel
Found by booting a scaffolded project and exercising the live API.
Studio resume crashed and leaked the failure verbatim:
`POST /studio/agents/{name}/threads/{id}/resume` assumed a LangGraph runnable.
Agentomatic's own lightweight `AgentGraph` has no `astream_events`, so the call
raised AttributeError *inside* the SSE body — returning HTTP 200 with the raw
internal message `'AgentGraph' object has no attribute 'astream_events'`. It now
fails fast with 501 and an actionable message, and the SSE error path routes
through `client_safe_detail` like the other routes rather than echoing
`str(exc)`.
Scaffold leaked the server's filesystem layout:
`agentomatic new /srv/apps/my_proj` derived the platform title and description
from the full path, and both are published via `/openapi.json`,
`/.well-known/agent.json`, `/studio/info` and `/api/v1/control`. Only the final
path segment is used now.
Scaffold pointed at a collector that isn't there:
the generated `.env.example` set `OTEL_EXPORTER_OTLP_ENDPOINT=localhost:4317`
unconditionally, so a fresh project logged a stream of `Transient error
StatusCode.UNAVAILABLE` / `Failed to export traces` retries out of the box. It
is now commented out with an explanation, alongside the console-export flag.
The `full` template produced an agent that could not be invoked:
a module-level `router` in an agent's `api.py` REPLACES *every* auto-generated
endpoint, so the flagship template shipped an agent whose only route was a stub
`/status` — no `/invoke`, `/chat`, `/invoke/stream`, `/card` or `/health`. The
example router is now named `custom_router` (which the registry does not pick
up), so the agent keeps its endpoints while the file still demonstrates — and
now explains — the takeover pattern.
Also verified, no change needed: all four deploy variants (full/minimal x
standard/distroless) emit valid compose YAML, run non-root, and use a
healthcheck their image can actually execute — the distroless one a curl-free
Python check, the standard one curl, which that image installs.
2059 passed, 47 skipped; ruff/mypy/mkdocs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…ent dead plugins
Found by booting a scaffolded project: the plugin feature was unusable
end-to-end, and the platform reported success while it was broken.
A scaffolded plugin was dead on arrival:
the template overrode `load_model()` without calling `await super().load_model()`,
so `BaseMLPlugin._is_loaded` never flipped. `/api/v1/plugins/{name}/predict`
answered `503 "not loaded yet"`, `/health` reported the whole platform
`degraded`, and `POST .../reload` did not recover it — all while startup logged
`✅ Plugin '...' loaded successfully`. The template now calls super(), and the
platform additionally stamps the flag itself after `load_model()` returns
(new `BaseMLPlugin.mark_loaded()`, also used by `reload_model`). A plugin author
who forgets super() can no longer end up with a permanently 503 plugin — the
base class documents the requirement, but a footgun that silently disables the
feature and mislabels it "loaded" shouldn't depend on reading that docstring.
Every scaffolded plugin registered under the same name:
the template set no `plugin_name`, so it inherited `BaseMLPlugin`'s default and
mounted at `/api/v1/plugins/default_plugin/*`. Two scaffolded plugins would
silently collide. It now sets `plugin_name`/`plugin_version` from the agent name.
The `full` template's schema contradicted its own agent:
`schemas.py` required `answer: str` while the agent returned `response`, so
every invoke logged an output-validation warning. The schema now matches.
Test isolation fix (not cosmetic — it was hiding the plugin bug):
the new end-to-end plugin test passed alone but failed in the full suite. The
registry imports plugins as `<plugins_dir.name>.<pkg>.plugin` via `sys.path`,
so writing files into a tmp dir depends on ambient interpreter state. New
`tests/conftest.py` helper puts the right directory on `sys.path`, calls
`importlib.invalidate_caches()` (the files are created after interpreter start),
and evicts the modules afterwards. Verified stable across repeated full runs.
2065 passed, 47 skipped; ruff/mypy/mkdocs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…L pass Log and startup hygiene found while exercising a live scaffolded server. None of these break correctness, but all three cost real money or debugging time in a production deployment. Every request was logged twice. The platform's own LoggingMiddleware logs each request with a correlation id, and uvicorn's access log logged the same request again — double the lines for the same information, which is volume and cost in a hosted log pipeline. `AgentPlatform.run()` now defaults uvicorn's access log off when its own logging middleware is active (an explicit `access_log=` from the caller still wins, and with the middleware disabled uvicorn's log is left alone, since it is then the only record). The `agentomatic run` CLI path does the same for the scaffolded `main.py`, with `AGENTOMATIC_UVICORN_ACCESS_LOG=1` to opt back in. The log had two formats in it. Lines emitted before `configure_logging` installs our sink use loguru's built-in format (` - ` separator); ours used an em dash, so the separator changed partway through startup and log-shipping regexes had to handle both. Now matches loguru's default, which also keeps the line ASCII. `Database tables created/verified` was logged (and the DDL re-run) twice. Startup can reach `store.initialize()` from three places: an explicitly configured store, one derived from `DATABASE_URL`, and a post-connection pass. Rather than untangle which call site should win, `SQLAlchemyStore.initialize()` is now idempotent — it is the invariant that actually matters, and it protects user code that calls it too. `close()` resets the flag so a disposed store can be re-initialized. 2069 passed, 47 skipped; ruff/mypy/mkdocs clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
… vs current_query
`agentomatic new NAME --dir X` scaffolded the project's 15 files straight into
X, dropping NAME entirely — so a directory the user only meant to scaffold
*inside* got the project scattered across it. `--dir` is documented as "Target
parent directory" and already behaves that way for agents
(`init foo --dir x` -> `x/foo`); projects now match (`x/NAME`).
Documented the request-field split that two separate verification passes
tripped over: the REST body takes `query`, which the framework normalises to
`current_query` in the state dict `input_to_state` receives — which is why the
class-agent example reads `data.get("current_query", "")`. Posting
`current_query` returns 422, and `/chat` uses `content` instead. Not a bug, but
the primer showed one half without the other.
Investigated and dismissed a reported "exit code 2 on success" for
`agentomatic new`: reproducing it with the console script directly gives exit 0
and the correct layout. The non-zero code came from `uv run` failing to resolve
a project when invoked from a directory outside the repo — a harness artifact,
not a CLI bug, so nothing to fix.
2069 passed, 47 skipped; ruff/mypy/mkdocs clean, `agents-guide` renders.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
The auth bypasses fixed earlier were found by reading code, which cannot show that a third does not exist. These sweeps answer that by construction. Exhaustive auth coverage (new tests/test_auth_coverage.py): enumerates every route the platform mounts and probes each one with auth enabled and no credentials. Of 130 routes, exactly 11 answer — all health probes, the Swagger/ReDoc shells, and the Studio SPA assets — and the set must equal an explicit allowlist that carries a justification per entry, so a newly added route that is accidentally public fails here instead of in production. Both the API-key and JWT middlewares are swept, since each keeps its own skip list. Verified non-vacuous: reintroducing the original "/studio" skip-prefix makes it fail and name all 13 exposed Studio routes. Public routes are also asserted not to carry the API key or control token. Ten more exception leaks, same class as the invoke/chat fix: plugin predict, custom endpoint handlers, log analysis, optimize invoke, thread fork/lineage/summary, and the A2A error decorator all interpolated raw exception text into 500 responses. A plugin raising a driver error leaked its DSN exactly as the invoke path did — reproduced, then confirmed fixed. All now route through client_safe_detail. The one 400 in tasks/routes.py is left verbatim: it wraps an explicit ValueError describing the caller's own bad input, where the message is the useful part. Scaffolded ingestors taught the vulnerable pattern: the ingestion template piped `request.source` straight into a file read, so every custom ingestor built from it inherited the arbitrary-file-read that was just fixed in the builtin. The example now resolves through resolve_within_root() and says why. Two existing tests asserted the leaky behaviour as correct (`assert "boom" in detail`, and the "/studio" skip-prefix); both now assert the fix, as the earlier `test_jwt_defaults_include_studio` did. 2083 passed, 47 skipped; ruff/mypy/mkdocs clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
… pass missed
An adversarial re-audit tried to break the earlier fixes. Traversal against the
auth skip-prefix and the ingestion confinement both held (symlink escape,
root-is-a-symlink, sibling-prefix, encoded traversal, null bytes, raw
un-normalised ASGI paths — all blocked). Error sanitisation did not: it had
only ever covered the synchronous request paths.
Async/background paths served raw exception text (high):
an agent failing with a driver error was sanitised on /invoke, /chat and
/invoke/stream, but the *same* failure submitted to /invoke/async came back
verbatim — DSN, credentials and file paths — from GET /tasks/{id},
/tasks/{id}/result, the whole task list, and the A2A task view. The raw text
was being persisted onto the task record itself (tasks/manager.py), so every
reader served it. Sanitised at the point of capture instead of at each reader,
so all of them are covered at once; per-item batch errors too.
Studio runs served raw exception text (medium): run_tracker stored `str(exc)`
on the run and streamed it over SSE, and the internal graph runtime did the
same in its Studio events. Reachable unauthenticated in the default
`agentomatic run` posture. The earlier fix had patched the resume route but
missed the tracker every run goes through.
A bug I introduced (low, but unauthenticated): `hmac.compare_digest` raises
TypeError on a non-ASCII `str`, so `?api_key=%C3%A9vil` returned 500 rather
than 401 — an unhandled exception reachable by anyone. Both the API-key and
control-token comparisons now compare bytes.
New `client_safe_message` is the string form of `client_safe_detail`, for the
places that persist an error into a plain `str` field later served over HTTP.
Errors stay actionable: type plus a correlation id for the full server log.
A third test asserting leaky behaviour as correct (`"kaboom" in rec.error`)
now asserts the fix.
Remaining, deliberately unchanged: ingestion errors echo the resolved path and
root back to the caller (server-path disclosure, informational), and the
control-plane *read* endpoints embed health_check errors — those reads are not
token-gated by design.
2092 passed, 47 skipped; ruff/mypy/mkdocs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
… configured
Found by curling a default scaffolded deployment: GET
/api/v1/{agent}/optimization-runs answered 500 Internal Server Error with an
empty body and an unhandled exception in the log. No store configured is the
*default* posture, so this was reachable on a stock `agentomatic run`.
`thread_store` is a `_LazyStoreProxy`: never `None`, with a `__bool__` that
reports whether a real store exists yet. Five routes guarded it with
`if thread_store is None:` — never true for a proxy object — so the guard fell
through and the first attribute access raised RuntimeError straight out of the
handler. Sibling routes using truthiness (`if thread_store:`) were always fine,
which is why this went unnoticed. All six guards (five here, one in
logs/router.py) now use the proxy's `__bool__`.
The routes now answer 400 with "Storage backend is not configured" (or 200 with
an empty result, where that is the established shape) instead of crashing.
Also verified in this pass, closing two caveats I had previously only been able
to disclose rather than test:
- **CI matrix parity**: the suite had only ever been run on Python 3.11. It now
passes on 3.12 and 3.13 as well — 2093 on each, matching what CI runs.
- **Container healthchecks**: previously validated by parsing the generated
compose file. Both are now *executed* against a live server: the standard
image's `curl -f` and the distroless image's shell-free
`/app/.venv/bin/python -c "urllib.request.urlopen(...)"` each exit 0 against a
healthy server and non-zero (7 and 1) against a dead port, so an unhealthy
container is actually reported unhealthy. The distroless healthcheck uses the
same interpreter path as its ENTRYPOINT, which must exist for the container to
start at all.
2093 passed, 47 skipped on 3.11/3.12/3.13; ruff/mypy/mkdocs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…ey can't recur
The last round's bug — a bare 500 on a stock deployment while 2092 unit tests
passed — was found by curling a running server by hand. That is not a repeatable
guarantee, so this applies the same technique exhaustively and then automates it.
Sweeping all 96 route/method pairs in the default (no store) posture found two
more, neither reachable from any single feature test:
- `GET /studio/agents/{name}/graph` returned a bare 500 for any agent
registered with only a `node_fn`: the LangGraph adapter *raised*
("has no graph_fn") where the graph-agent adapter returns an empty topology,
and the route caught only `TimeoutError`. The Studio UI calls this endpoint
for every agent, so the debug view broke on a perfectly valid agent. The
adapter now degrades like its sibling, and the route no longer lets any
adapter failure escape — graph introspection runs user code, so a debug view
failing to draw must never 500.
- `GET /api/v1/{agent}/threads/{id}/summary` guarded on `memory_mgr`, which
wraps the lazy store proxy and stays truthy with no store configured, so the
store's RuntimeError surfaced as a 500 for what is a configuration issue. It
now checks the store and returns 400, like its siblings.
New tests/test_no_unhandled_500s.py walks every mounted route and asserts none
5xx. 4xx is fine anywhere — missing resource, unconfigured backend, bad input;
5xx means an exception escaped a handler. It runs in the default no-store
configuration, which is what a fresh `agentomatic run` actually uses and which
no individual test happened to build. Verified non-vacuous: reverting the
`is None` guard makes it fail naming the exact route.
Sweep now reports 0 failures across 96 route/method pairs.
2097 passed, 47 skipped; ruff/mypy/mkdocs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…eaks
An independent review pass over the whole branch diff caught two regressions
this branch itself introduced, plus findings the route sweep could not see.
REGRESSION (mine): Studio state and history showed an opaque blob.
Switching the checkpointer to LangGraph's serde (so BaseMessage objects survive
a round-trip) changed what a stored checkpoint row contains. Two Studio reads go
to the store directly and were never updated, so
`GET /studio/agents/{name}/threads/{id}/state` and `/history` returned
`{__agentomatic_serde_type__, __agentomatic_serde_data__}` instead of the actual
state. `encode_for_storage`/`decode_from_storage` are now public (they are used
across modules) and both read sites decode. Verified end to end: state comes
back with real values and real HumanMessage/AIMessage objects.
REGRESSION (mine): the Studio resume guard called `agent.graph_fn()` outside any
try, so an agent whose `build_graph()` raises got the bare 500 this branch had
just removed elsewhere — and built the graph twice. Now guarded, and the built
graph is reused.
Three more raw-exception leaks the sweep missed, because each returns a
*handled* response that happens to embed `str(exc)`:
- `POST /threads/{id}/resume` — `detail=f"Error resuming execution: {exc}"`.
- `POST /chat` — a history-loading failure put `str(exc)` into the 200
response's own metadata, so it never looked like an error path at all.
- control-plane `_agent_health` — embedded in `/control/agents` and
`/control/health`, which are read routes `_authorize` does not gate.
`SQLAlchemyStore` decided the SQLite `foreign_keys` pragma from the `url`
argument even when the caller passed their own `engine=`, where that url is an
unused default — it could attach a SQLite pragma to someone else's non-SQLite
engine. Now scoped to engines the store actually owns.
Not changed — a reported finding I disagree with: a condition-skipped pipeline
step does not mark its dependents unsuccessful. `test_conditions_still_skip_in_dag_order`
asserts dependents keep running, and a separate test asserts that a *failed*
upstream does block them. Failure propagating while an optional branch does not
is a coherent, deliberate distinction, so changing it would break documented
behaviour on my own judgement. I tried the change, saw the test fail, and
reverted it.
2098 passed, 47 skipped; ruff/mypy/mkdocs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
Found by driving the real Studio UI in Chromium against a real server: every
agent reply rendered twice in the chat pane (3 messages for a single exchange).
`RunTracker.execute_with_adapter` brackets each run with its own
`run_start`/`run_complete` — those carry the real run_id, timing and output —
then forwards every event the adapter yields. `AgentGraph.astream_studio_events`
emits its own lifecycle pair too, so the client received two runs' worth of
events for one run. Confirmed on the wire: the SSE stream contained
`run_start` x2 and `run_complete` x2. The tracker owns the lifecycle, so
adapter-level duplicates are now dropped as they pass through; node events
still flow untouched.
Verified in the browser, before and after: 3 messages -> 2, one reply instead
of two, and the stream now carries exactly one of each lifecycle event.
No unit test caught this because each component was individually correct — the
duplication only exists in their composition, and only shows up in a rendered
UI. The new test drives the tracker with an adapter that emits its own
lifecycle pair and asserts exactly one of each reaches the client; reverting
the fix makes it fail.
Browser session also confirms genuine frontend/backend alignment: the UI loads,
connects ("Connected to Uiproj Platform v1.0.0 - 3 agent(s)"), lists all
agents, renders a form generated from the backend's real AgentInvokeRequest
schema, and executes a run end to end with 0 JS exceptions. Every request it
made returned 200 — including `/studio/agents/{slug}/graph`, the endpoint fixed
earlier in this branch from a bare 500. The only failed requests in the whole
session were Google Fonts, blocked by this sandbox's egress (worth knowing for
air-gapped deploys: the Studio UI fetches fonts from fonts.googleapis.com).
2099 passed, 47 skipped; ruff/mypy/mkdocs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
… build Simulating the generated container (venv + the pinned wheel from PyPI + `uvicorn main:app`) surfaced two defects that only a real deployment shows. 1. Version skew kills the container with an unreadable traceback. The Dockerfile pins `agentomatic[all]==<version>` from PyPI, but a project scaffolded from a tree that is ahead of the published release calls `AgentPlatform` with options that release does not have. The image died at import with a bare `unexpected keyword argument` and no clue what to do. The scaffolded `main.py` now reframes exactly that TypeError, naming the installed version and the fix, and chains the original so the traceback survives. Unrelated TypeErrors still propagate untouched — nothing is silently dropped, so an unsupported hardening flag can never look enabled when it is not. 2. `--profile minimal` advertises quieter logs but delivered none. Logging was configured in the lifespan, which runs at startup — after construction and `build()` have already narrated settings loading, discovery, and every mount. `log_level="WARNING"` now applies from construction onwards: the minimal-profile boot log drops from ~40 lines to the CORS warning alone, with the REST API, health, metrics, and Swagger unchanged. Both are covered by regression tests that fail without the fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
`agentomatic deploy --distroless` produced an image that could not run at
all. The build stage installed into a virtualenv on `python:3.12-slim`, but
`gcr.io/distroless/python3-debian12` is Debian 12's Python 3.11. A venv's
`bin/python` is a symlink to the *builder's* interpreter, which does not
exist in the runtime stage, so the ENTRYPOINT was a dangling symlink:
exec: "/app/.venv/bin/python": stat /app/.venv/bin/python:
no such file or directory
The container exited 127 before any application code ran. Resolving the
symlink would not have been enough either — cp312 wheels do not import under
3.11.
The build stage now uses `python:3.11-slim` to match the runtime, installs
with `pip --target=/app/deps` instead of a virtualenv (a plain directory on
PYTHONPATH works with any 3.11), and the ENTRYPOINT runs the base image's own
`/usr/bin/python3`. The compose healthcheck for distroless pointed at the same
missing venv binary and is corrected the same way.
Verified by building both images against the real base images and running
them: the distroless container now serves /health, /docs, and agent invokes
as UID 65532, with Studio off under the minimal profile.
`test_compose_distroless_healthcheck_has_no_curl` and
`test_distroless_uses_nonroot_numeric_uid` had both asserted the broken
`/app/.venv/bin/python` path as correct; they now assert the interpreter the
runtime image actually has, alongside a new test pinning the builder/runtime
Python versions together.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
The note still described the old `json.dumps(default=str)` behaviour, telling readers that custom objects, datetimes, and bytes are stored as their string representation. Checkpoints now round-trip through LangGraph's `JsonPlusSerializer`, so a reloaded thread yields real `HumanMessage` / `AIMessage` objects rather than their `repr()`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
The Studio state/history endpoints read the graph's checkpointer, and nothing
else writes checkpoints. An agent built with the native `GraphBuilder`
(`self.new_graph().compile()` — the pattern the primer recommends) has no
checkpointer, so `/state` returns `{}` and `/history` returns `[]` while the
thread's chat messages persist normally through a separate path. Nothing said
so, which reads as a broken panel rather than an opt-in feature.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…dependency
Sweeping all 315 route/method pairs of a running container turned up two
defects the suite could not see.
**A2A text was silently discarded.** `submit_a2a_task` read only
`message.content`, but the A2A protocol carries text in `message.parts`. A
spec-shaped request ran the agent on an *empty* query and returned 200 with a
meaningless result — `{"result": "Response to: "}` for a message that plainly
said "a2a hello". Every existing A2A test used the `content` shape, so the
gap was invisible. Text is now extracted from `parts` (protocol form),
`content` (string or parts), or `text`, and a message carrying no readable
text is rejected with 422 naming the accepted shapes instead of running
empty.
**The deepagent template failed with no clue what to install.** It imports
the third-party `deepagents` package, which agentomatic does not depend on.
The agent scaffolded, registered, and reported healthy, but every invoke,
chat, and optimize call returned a sanitised 500 whose only signal was
`"error_type": "ModuleNotFoundError"`. The import is now guarded with a
message naming `pip install deepagents`, and `agentomatic init --template
deepagent` says so as step 1.
Both verified in the container: the parts-shaped message now returns
"Response to: a2a hello", a text-less message returns 422, and the `content`
form still returns 200.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
The existing sweep runs the default no-store configuration, so it never reaches the handlers that only execute once a store exists — history reads, checkpoint lookups, thread summaries. A container sweep in that posture is what surfaced the last round of defects, so the same 300+ route/method surface is now covered with a MemoryStore and invocation history enabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…guration The global auth lock's own error message offers "enable_auth=True with auth_api_key" as remedy (b). Running exactly that in a container produced a platform that could not serve a single request — two separate defects, both invisible to the suite because no test built this combination. 1. **Every route returned 500, including /health and /docs.** The scaffolded `main.py` turns JWT auth on whenever `AGENTOMATIC_REQUIRE_AUTH` is set, and `JWTAuthMiddleware` refuses to construct without a `jwks_url` under the auth lock. Starlette builds the middleware stack on the *first request*, not at `build()`, so the `try/except` around `add_middleware` never saw it: the app started clean and then 500'd forever. When API-key auth is configured and no JWKS is, the JWT middleware is now skipped with a warning naming the fix — the API key is the enforcement mechanism. With neither, the build still refuses to start, so the forged-JWT hole stays closed. 2. **A valid API key was rejected 401 "no valid JWT claims found".** The zero-trust middleware was registered after the API-key middleware, which under Starlette's reverse ordering means it ran *first* — it looked for JWT claims before the key had been checked, and denied. Authentication now runs before authorization, the API-key middleware records the authenticated principal on `request.state`, and the enforcer accepts it. An API key carries no roles or scopes, so an agent policy restricting either fails closed with an explicit reason rather than silently passing. Verified in a container: valid key 200, no key 401, bad key 401, forged unsigned JWT 401, health and docs 200. `_mock_request` in test_security.py handed every mocked request a MagicMock `state`, which answers truthily to any attribute — it now controls `api_key_authenticated` explicitly like the other state fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…erification
Verified JWT auth was documented as configurable "via stack", and
`agentomatic deploy` writes `AUTH__JWKS_URL` / `AUTH__ISSUER` /
`AUTH__AUDIENCE` into the generated `.env` — but nothing ever read them.
`JWTConfig` was built only from the in-process `jwt_config=` kwarg, so a
deployed container running the scaffolded `main.py` could not switch signature
verification on at all, and `require_auth_globally` refused to boot unless an
API key was configured instead. Remedy (a) in the auth lock's own error
message was unreachable from a deployment.
`build()` now resolves a `JWTConfig` from those environment variables or the
active stack's `auth:` block (environment wins, `${VAR}` placeholders are
expanded, an unexpanded one is not mistaken for a URL), matching how the
database URL already resolves.
Verified end to end against a real JWKS endpoint in a container: an
RS256-signed token returns 200, while an expired token, a wrong-issuer token,
an `alg=none` forgery, and a missing token all return 401.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
`agentomatic init NAME --template pipeline` writes `pipelines/<name>/pipeline.yaml` alongside the pipeline's `dataset.jsonl`, `eval.py` and `Makefile`. Discovery only scanned flat `pipelines/*.yaml` and `agents/*/pipeline.yaml`, so a freshly scaffolded pipeline was never found: `agentomatic pipeline list` printed "No pipelines found" and advised creating "a pipeline.yaml in pipelines/" — which is exactly what had just been created — and the Pipelines API mounted with zero pipelines. Per-pipeline folders are now scanned in both entry shapes (project root and the `pipelines/` directory itself). Verified in a container: the scaffolded pipeline is discovered, listed by the API with its three steps, and its run endpoint validates agent references with a clear 4xx. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
…y is `trust_proxy_headers=False` promises X-Forwarded-For is ignored, and this middleware does ignore it — but uvicorn's own `--proxy-headers` (on by default) rewrites `request.client` from that header for peers in `--forwarded-allow-ips` (default `127.0.0.1`) before any middleware runs, and the original peer address is not recoverable afterwards. A caller connecting from an allowed peer address can therefore still steer the rate-limit key. Observed in a container: after exhausting the limit (100 requests, then 429 as configured), the same caller adding `X-Forwarded-For: 9.9.9.9` was served again. The code comment claimed more than the flag can deliver, so it and the deployment guide now name uvicorn's `--forwarded-allow-ips` as the control that decides whether the rewrite happens, and point out that the generated `nginx.conf` sits on exactly that trusted hop. Tests pin the key derivation in all four cases, including the rewritten-peer one, so the documented boundary stays true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
`test_openai_compat_mirrors_enable_thinking_into_chat_template` imports `langchain_openai` through the code under test. That package lives in the optional `openai` extra, which `agentomatic[all]` deliberately does not pull in — the platform ships no first-party vendor connectors — so the suite failed for anyone who installed exactly what the docs recommend. CI happens to use `uv sync --all-extras`, which hid it. The module is now stubbed when absent (and the real one used when present), matching how `test_llm_base_url.py` already handles it. Verified: 2128 pass on 3.11, 3.12, and 3.13 with only `--extra all`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
`uv build` with the currently-resolving hatchling (1.32) produces a wheel and
sdist carrying `Metadata-Version: 2.5`, and `twine check` refuses both:
InvalidDistribution: Invalid distribution metadata:
'2.5' is not a valid metadata version
The release workflow publishes through `pypa/gh-action-pypi-publish`, which
verifies metadata with twine before upload, so the next release would have
failed at the publish step — with an unbounded `requires` the outcome depended
on whichever hatchling resolved on the day. Pinned to `>=1.27,<1.30`; both
artifacts now pass `twine check`. A test asserts the upper bound stays, so
lifting it once 2.5 is accepted is a deliberate act rather than a silent one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4
piotrlaczkowski
changed the title
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.
Summary
This PR hardens the platform against three classes of vulnerabilities discovered through end-to-end testing:
Key Changes
Path Confinement (
src/agentomatic/ingestion/paths.py- new)resolve_within_root()to safely resolve paths relative to an ingestion rootsafe_output_filename()to reject path traversal in output filenames (e.g.,../../pwned.txt)cwdor respectsINGESTION_ROOT_ENVenvironment variableError Sanitization (
src/agentomatic/core/errors.py- new)client_safe_detail()andclient_safe_message()helpersSecret Redaction (
src/agentomatic/stacks/redaction.py- new)REDACTEDmarker and key-name patterns for secrets (api_key, password, token, etc.)${ENV_VAR}references (safe and useful for verification)Supporting Changes
_ensure_json_serializable()withencode_for_storage()/decode_from_storage()for safer round-tripping of non-JSON objectsinclude_in_schema=False) to prevent duplicate operationIds and warningsagentomatic newnow uses only the final path segment, not the full filesystem path (which was published in/openapi.json,/.well-known/agent.json,/studio/info)dict_to_messages()to properly convert"system"role toSystemMessageinstead ofHumanMessagetrust_proxy_headersparameter for X-Forwarded-For handlingTest Coverage
Added comprehensive regression test suites:
tests/test_security_hardening.py(739 lines): Path confinement, error sanitization, secret redactiontests/test_production_readiness.py(547 lines): OpenAPI deduplication, slug alias routingtests/test_e2e_langchain_class_agent.py(384 lines): Full LangChain integration via HTTPtests/test_template_scaffold_quality.py(298 lines): Generated code lintingtests/test_studio_bundle_alignment.py(297 lines): Backend ↔ Studio-frontend API drift detectiontests/test_auth_coverage.py(264 lines): Exhaustive auth coverage over every routetests/test_no_unhandled_500s.py(216 lines): Route crash detectiontests/conftest.py: Shared test helpersEach test corresponds to a vulnerability reproduced against a running platform, not inferred from code inspection.
Notable Implementation Details
resolve_within_root()once at entry{"error_type": "...", "correlation_id": "..."}https://claude.ai/code/session_01XJ9z9ijdHpRCmkHhTp8Ag4