Repository navigation
Security hardening from an external audit - #60
Merged
Merged
Conversation
josedaniel-dev
force-pushed
the
fix/security-hardening
branch
from
October 16, 2026 07:59
1ae0f47 to
6859bdd
Compare
Deployment: compose binds backend and dashboard to loopback (OBSERVER_BIND), no longer publishes PostgreSQL and has no default database password; the backend warns at start when no API key is set. The dashboard dev/preview proxy lends the API key only to loopback requests and listens on 127.0.0.1. Generic ingestion gets limits: body size middleware, span counts, JSON size and depth, string lengths. The rate limiter keys on the API key only when it is the configured one, otherwise on the IP. The MCP server allow-lists attributes and metadata and withholds dataset notes and phrases. Internal hand-off notes and operational details are removed from the docs; docs/security.md added.
josedaniel-dev
force-pushed
the
fix/security-hardening
branch
from
October 16, 2026 16:56
6859bdd to
9b28def
Compare
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
1 participant
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.
Addresses the findings of an external review of
mainat ea5671c. I checked each against the code before changing it; all were real, as described.Deployment (High)
postgres/postgres. Now: backend and dashboard on${OBSERVER_BIND:-127.0.0.1}, PostgreSQL not published, no defaultPOSTGRES_PASSWORD(compose refuses to start without it),OBSERVATORY_API_KEYpassed through. The backend logs a warning at start when no key is set. A test parses both compose files and fails on a public bind, a published database port or a literal password.OBSERVER_DASHBOARD_HOST). Caveat indocs/security.md: a reverse proxy on the same host makes every request look local, so authenticate in front. The container dashboard (nginx) still does not add the key.Ingestion limits (Medium) — body-size middleware (16 MiB, also for streamed bodies), spans per trace/batch, traces per import, string lengths, JSON field size (256 KiB) and depth (12) on the generic routes; all configurable (
docs/security.md). Defaults are generous so ordinary SDK payloads (50 KB prompts) still pass; tested.Rate limiting (Medium) — the limiter trusted any
Authorizationvalue, so rotating fake tokens bought a fresh budget. It now uses the API key identity only when it is the configured key, else the client IP.MCP (Medium) — attributes and trace metadata are now an allow-list of structural keys (scalars, single line, 200 chars) instead of passing everything; dataset notes, descriptions and expected phrases appear only as counts.
OBSERVER_MCP_CONTENT=1opts out. Span/trace names are still free text (shortened, control characters stripped).Published internals (Medium) — removed
docs/handoff-codex-2026-10-08.md(and its reference in AGENTS.md), the Manitos module path and PR/restart details fromdocs/manitos-integration.mdand the roadmap, and a service-name command from the setup guide. Deleting files does not remove them from history; the audit found no secret that would justify rewriting it.Tests: 9 new/updated regression tests that fail without the fixes (limits, body middleware, rate-limit identity, compose, MCP allow-list and withheld text), proxy-auth unit tests. Backend 219 passed, dashboard 39, ruff/tsc/build clean.
Not covered / for you: the audit did not run a dependency-vulnerability scan and neither did I; I could not run docker/podman here, so the compose change is checked by parsing, not by
up; andOBSERVER_BIND, the API key and a front proxy remain your deployment choices.Merge order: independent of #58 and #59, but all three touch
docs/roadmap.md(and #58/#59i18n-en.ts), so the later ones will need a rebase.