Skip to content
Merged
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

### Added

- `docs/DEPLOY.md`: Coolify deployment runbook (Dockerfile build pack, the environment variables that matter in production, and the two behaviours that surprise operators — in-memory sessions, and the SSRF guard refusing a private mail server). The app now warns at startup when `AYS_TRUSTED_PROXIES` is empty, which behind a reverse proxy means every client shares one login rate-limit bucket

- `POST /api/scripts/preview` renders one Rule through the backend generator, and the SPA's duplicate generator (`previewRule`) is deleted. The preview is now the bytes a save writes, asserted as such; the duplicate had diverged five ways, including showing nothing for a Rule whose last Condition was deleted while a save wrote invalid Sieve (areyousievious-8fg.17)

- GitHub Actions CI workflow (`.github/workflows/ci.yml`): runs pytest and frontend build on every push and pull request (P1)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ Open `http://localhost:8091` and log in with your IMAP credentials.
| `AYS_SESSION_IDLE_TIMEOUT` | `1800` (30 min) | Seconds of inactivity after which a session stops being accepted — immediately and always. Its credentials are freed a little later, by the periodic sweep, which runs when a request touches the store and at most once a minute; a session nobody returns to therefore stays resident until some other request arrives, as there is no background sweeper. Also the `max_age` on the session and CSRF cookies, so the browser hint and the server's enforcement cannot drift apart. |
| `AYS_SESSION_MAX_LIFETIME` | `28800` (8 h) | Absolute cap counted from login and NOT refreshed by use. Without it the timeout was idle-only, so a client polling `/api/auth/status` kept a plaintext password resident for as long as it cared to poll. |
| `AYS_IMAP_INSECURE` | _(unset)_ | ⚠️ **Testing only.** `1` / `true` / `yes` disables outbound IMAP TLS chain + hostname verification (for self-signed mail servers). Leaving this unset is mandatory in production — without it, an on-path attacker can MITM the IMAP login and steal credentials (CWE-295). |
| `AYS_TRUSTED_PROXIES` | _(unset)_ | CSV of CIDRs that may set `X-Forwarded-For` / `X-Real-IP` (e.g. `127.0.0.1/32,10.0.0.0/8`). When unset, those headers are ignored and the rate limiter uses the direct peer — required when the app is exposed without a reverse proxy or any caller can spoof the headers to bypass throttling (CWE-348). |
| `AYS_TRUSTED_PROXIES` | _(unset)_ | CSV of CIDRs that may set `X-Forwarded-For` / `X-Real-IP` (e.g. `127.0.0.1/32,10.0.0.0/8`). When unset, those headers are ignored and the rate limiter uses the direct peer — right when the app faces clients directly, since any caller could otherwise spoof the headers to bypass throttling (CWE-348). **Behind a reverse proxy the direct peer is the proxy for every request**, so leaving it unset gives all clients one shared rate-limit bucket and five failed logins lock out everyone. The app warns at startup when it is empty; see [docs/DEPLOY.md](docs/DEPLOY.md). |
| `AYS_IMAP_TIMEOUT` | `10` | Seconds before an outbound IMAP connect or read aborts. |
| `AYS_SIEVE_CONNECT_TIMEOUT` | `10` | Seconds before an outbound ManageSieve TCP connect aborts. A blackhole mail server would otherwise pin the threadpool worker for the OS default (~2 min). |
| `AYS_SIEVE_IO_TIMEOUT` | `30` | Seconds before an outbound ManageSieve read or write aborts on a connected socket. |
Expand Down
31 changes: 31 additions & 0 deletions backend/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"""

import argparse
import logging
from pathlib import Path

from auth import SessionManager
Expand Down Expand Up @@ -94,6 +95,35 @@ async def _protocol_name_handler(_request: Request, exc: Exception):
# ── App construction ──


log = logging.getLogger("app")


def _warn_if_no_trusted_proxies(cfg: Settings) -> None:
"""Say what an empty AYS_TRUSTED_PROXIES means BEHIND A PROXY.

Unset is the right default and is not changed here: trusting
`X-Forwarded-For` from any caller is the hole the F-2 fix closed
(CWE-290/348). But `_get_client_ip` then falls back to the direct peer,
and behind a reverse proxy the direct peer is the PROXY, for every
request — so the login limiter's 5-per-5-minutes bucket is shared by
everyone on the instance and five failed logins lock all of them out.

Nothing about that is visible from outside. The app starts, serves, and
throttles; the symptom arrives minutes later as "nobody can log in". A
deploy that puts this behind Traefik, nginx or Caddy needs the CIDR set,
and this is where it gets told. See docs/DEPLOY.md.
"""
if cfg.trusted_proxies:
return
log.warning(
"AYS_TRUSTED_PROXIES is empty, so X-Forwarded-For is ignored and the "
"login rate limit is keyed on the direct peer. Correct when this app "
"faces clients directly; behind a reverse proxy it means EVERY client "
"shares one rate limit bucket and five failed logins lock out all "
"users. See docs/DEPLOY.md."
)


def create_app(config: Settings | None = None) -> FastAPI:
"""Build an app from an explicit configuration.

Expand All @@ -103,6 +133,7 @@ def create_app(config: Settings | None = None) -> FastAPI:
this, instead of mutating os.environ and reloading the module.
"""
cfg = config or settings()
_warn_if_no_trusted_proxies(cfg)

app = FastAPI(
title="AreYouSievious",
Expand Down
69 changes: 69 additions & 0 deletions backend/tests/test_docs_gating.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@

from __future__ import annotations

from pathlib import Path

import httpx
import pytest
from app import create_app
from config import Settings
from routers import static as static_router_mod

DOC_PATHS = ["/docs", "/redoc", "/openapi.json"]

Expand Down Expand Up @@ -58,3 +61,69 @@ async def test_default_settings_block_docs():
"""The default — what a deploy gets with no AYS_ENV set at all."""
app = create_app(Settings())
assert await _status(app, "/openapi.json") == 404


# ── The shape a deploy actually runs in (docs/DEPLOY.md) ──


@pytest.mark.parametrize("path", DOC_PATHS)
@pytest.mark.asyncio
async def test_the_schema_is_still_withheld_when_the_spa_is_served(
path: str, tmp_path: Path
) -> None:
"""Every test above builds an app with NO static directory, and asserts a
404. A DEPLOYED app serves the SPA, and its catch-all
`GET /{full_path:path}` answers anything unmatched — so in production
these paths come back 200 with the SPA shell, not 404.

Found by running the container rather than the suite: `AYS_ENV=prod` and
`curl /openapi.json` gave `200 text/html`, which read like a gate failure
and was not one. The gate holds; the STATUS CODE just cannot be what says
so once a catch-all exists.

So this asserts the property the 404 was standing in for: whatever comes
back, it is not the schema. A status-code assertion that stops being true
the moment the app is deployed is a test whose green means less than it
appears.
"""
static_dir = tmp_path / "static"
static_dir.mkdir()
(static_dir / "index.html").write_text("<!doctype html><title>spa</title>")

static_router_mod.configure(static_dir)
try:
app = create_app(Settings(env="prod"))
transport = httpx.ASGITransport(app=app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
r = await client.get(path)
finally:
static_router_mod.configure(None)

assert r.status_code == 200, "the SPA catch-all answers, which is the point"
assert "text/html" in r.headers["content-type"]
assert "spa" in r.text
for leaked in ("openapi", "swagger", "redoc", '"paths"'):
assert leaked not in r.text.lower(), f"{path} leaked {leaked!r}"


@pytest.mark.asyncio
async def test_dev_still_serves_the_real_schema_past_the_catch_all(tmp_path: Path) -> None:
"""The other half: with a static dir configured, `dev` must still reach
the REAL schema rather than being swallowed by the catch-all. Route order
is what decides that, and route order is easy to change by accident."""
static_dir = tmp_path / "static"
static_dir.mkdir()
(static_dir / "index.html").write_text("<!doctype html><title>spa</title>")

static_router_mod.configure(static_dir)
try:
app = create_app(Settings(env="dev"))
transport = httpx.ASGITransport(app=app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
r = await client.get("/openapi.json")
finally:
static_router_mod.configure(None)

assert r.status_code == 200
assert "application/json" in r.headers["content-type"]
assert "paths" in r.json()
76 changes: 76 additions & 0 deletions backend/tests/test_proxy_trust_warning.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
"""
The startup warning for an unset AYS_TRUSTED_PROXIES (Coolify deploy).

`_get_client_ip` falls back to the direct peer when no proxy is trusted, which
is correct and deliberate — trusting `X-Forwarded-For` from anyone is the F-2
hole (CWE-290/348). But behind a reverse proxy the direct peer is the PROXY,
for every request, so the login limiter's 5-attempts-per-5-minutes bucket is
shared by every user of the instance. Five failed logins lock out everybody.

Nothing about that is visible: the app starts, serves, and throttles. It only
shows up as "nobody can log in" some minutes after a stranger typed a password
wrong. So the app says so at startup instead.

The DEFAULT IS NOT CHANGED. Defaulting to "trust the private ranges" would
make a proxied deploy work with no configuration and hand anything on the same
network the ability to spoof past the throttle — reversing the fix that put
this gate here.

Run from the backend/ directory:
cd backend && python -m pytest tests/test_proxy_trust_warning.py -v
"""

from __future__ import annotations

import ipaddress
import logging

import pytest
from app import create_app
from config import Settings


def test_an_empty_trusted_proxy_list_warns_at_startup(caplog) -> None:
"""The whole point: a deploy behind a proxy with no configuration gets
told, in the log Coolify shows, what it is about to do."""
with caplog.at_level(logging.WARNING):
create_app(Settings())

warnings = [r for r in caplog.records if r.levelno >= logging.WARNING]
assert len(warnings) == 1, f"expected exactly one warning, got {warnings}"
message = warnings[0].getMessage()
assert "AYS_TRUSTED_PROXIES" in message, "name the variable to set"
assert "rate limit" in message.lower(), "name the consequence, not just the setting"


def test_a_configured_trusted_proxy_list_is_silent(caplog) -> None:
"""A warning that fires when the thing is configured correctly is a
warning people learn to scroll past."""
with caplog.at_level(logging.WARNING):
create_app(Settings(trusted_proxies=(ipaddress.ip_network("10.0.0.0/8"),)))

assert [r for r in caplog.records if r.levelno >= logging.WARNING] == []


@pytest.mark.parametrize("env", ["prod", "dev", ""])
def test_the_warning_does_not_depend_on_env(caplog, env: str) -> None:
"""`AYS_ENV=dev` gates the docs, not this. A dev deploy behind a proxy
has exactly the same shared-bucket problem."""
with caplog.at_level(logging.WARNING):
create_app(Settings(env=env))
assert [r for r in caplog.records if r.levelno >= logging.WARNING]


def test_it_is_a_log_record_and_not_a_print(capsys, caplog) -> None:
"""`print` is what the static-dir warning in `main()` uses, and it goes to
stdout unstructured. This one has to survive whatever the platform does
with its logs, so it is a WARNING on a named logger — which is also why
the rest of the suite can build a thousand apps without it becoming noise
anyone has to filter.
"""
with caplog.at_level(logging.WARNING):
create_app(Settings())

assert "AYS_TRUSTED_PROXIES" not in capsys.readouterr().out
(record,) = [r for r in caplog.records if r.levelno >= logging.WARNING]
assert record.name.startswith("app"), f"unexpected logger {record.name!r}"
8 changes: 8 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
# LOCAL DEVELOPMENT ONLY.
#
# This publishes to 127.0.0.1, which a platform's reverse proxy (Coolify's
# Traefik, for one) cannot reach — pointing a deploy at this file gets a
# container nothing can talk to. Deployments build the Dockerfile directly;
# see docs/DEPLOY.md, which also lists the variables that matter in
# production and are deliberately absent here.

services:
areyousievious:
build: .
Expand Down
1 change: 1 addition & 0 deletions docs/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Project documentation: architecture notes, decision records, agent-skill configu
| File | Description |
|------|-------------|
| `ARCHITECTURE.md` | Long-form architecture write-up. **Partly stale** — tracked by `bd:areyousievious-8au` (dead file references, a `/api/test` endpoint that was never built, and a rule shape carrying an `id` that ADR-0001 removed) |
| `DEPLOY.md` | The deployment runbook: Coolify via the repo's Dockerfile, the environment variables that matter in production, and the two behaviours that surprise operators (in-memory sessions, and the SSRF guard refusing a private mail server). Anything asserted there is checked against the running app before it is written down |

## Subdirectories
| Directory | Purpose |
Expand Down
Loading
Loading