Make a failed deploy impossible to miss, and a wildcard CORS harmless
Build & Deploy / Build & push images (push) Successful in 4m4s
Build & Deploy / Deploy to VM (push) Failing after 2m16s
Build & Deploy / Report a failed deploy (push) Successful in 1s

Two unrelated-looking problems with the same shape: a dangerous state that
looked fine from the outside.

DEPLOY (server-26#21). The Deploy job failed on fifteen consecutive pushes
between 2026-08-18 and 08-20 and nobody noticed for two days, because the
build job was green and a red run is only visible to someone who opens Gitea.
Production served 08-18 code the whole time -- including the entire frontend
redesign, chunks 2 through 8. Three changes:

  * The health check now asserts WHICH build answered, not just that something
    did. CI bakes the commit into the image (Dockerfile ARG/ENV GIT_SHA) and
    /health reports it, so a deploy that "succeeds" while the previous
    container keeps running now fails. Liveness alone could never have caught
    this.
  * The image pull retries once after a prune. The actual failure was
    containerd unable to extract a layer -- "failed to Lchown ... no such file
    or directory" -- a corrupted entry in the snapshot store, which a prune
    clears. A second failure after pruning is a real problem (check the VM's
    disk) and still stops the deploy.
  * A notify-failure job POSTs to DEPLOY_ALERT_WEBHOOK when anything in the
    workflow fails. Unset means skip quietly, not fail.

CORS (server-26#20). allow_origins=["*"] with allow_credentials=True is not
the permissive-but-harmless setting it reads as. Starlette does not reject the
pair -- it reflects the caller's Origin back and still sends
Access-Control-Allow-Credentials: true, so the effective policy is "any
origin, WITH credentials", the opposite of what a wildcard normally means.

Rather than trust every deployment to remember CORS_ORIGINS, the pair is now
unrepresentable: a wildcard forces allow_credentials off and logs an ERROR
naming the variable to set. Correctly configured deployments that name their
origins are unaffected and keep credentialed requests.

Severity honestly: low today. c2-core is bearer-auth, and browsers do not
attach bearer tokens cross-origin the way they attach cookies. This is a
misconfiguration waiting for the day something starts trusting a cookie.

Also adds firebase_admin.auth.UserRecord and the list/update/create/delete_user
names to the conftest stub. routers/users.py annotates with UserRecord at
import time, so without it importing app.main failed at collection -- which is
why nothing had ever tested anything wired at app level, CORS included.

Tests: 5 new in test_cors_policy.py, covering the pure policy function, the
middleware actually mounted on the app (so re-hardcoding allow_credentials=True
fails here), and the presence of the build stamp.

Closes logan/server-26#20
Closes logan/server-26#21
This commit is contained in:
Logan Cusano
2026-08-23 01:26:15 -04:00
parent 33a247d306
commit 8fbfe7d6de
6 changed files with 174 additions and 7 deletions
+63 -5
View File
@@ -31,6 +31,8 @@ jobs:
with: with:
context: ./drb-c2-core context: ./drb-c2-core
push: true push: true
build-args: |
GIT_SHA=${{ gitea.sha }}
tags: | tags: |
${{ env.REGISTRY }}/c2-core:latest ${{ env.REGISTRY }}/c2-core:latest
${{ env.REGISTRY }}/c2-core:${{ gitea.sha }} ${{ env.REGISTRY }}/c2-core:${{ gitea.sha }}
@@ -91,14 +93,70 @@ jobs:
# Update compose files + mosquitto config # Update compose files + mosquitto config
git pull origin main git pull origin main
# Pull pre-built images and restart (no build on the VM) # Pull pre-built images and restart (no build on the VM).
docker compose -f docker-compose.yml -f docker-compose.prod.yml pull #
docker compose -f docker-compose.yml -f docker-compose.prod.yml up -d --remove-orphans # The retry is not defensive padding: this exact step failed fifteen
# deploys in a row (2026-08-18 to 08-20) with containerd unable to
# extract a layer -- "failed to Lchown ... no such file or directory"
# -- a corrupted entry in the snapshot store. Pruning clears the bad
# layer and the second pull succeeds. If it fails again after a
# prune that is a real problem (check the VM's disk) and should stop
# the deploy rather than be retried forever.
COMPOSE="docker compose -f docker-compose.yml -f docker-compose.prod.yml"
if ! $COMPOSE pull; then
echo "image pull failed - pruning and retrying once"
docker image prune -af
$COMPOSE pull
fi
$COMPOSE up -d --remove-orphans
docker image prune -f docker image prune -f
ENDSSH ENDSSH
- name: Health check - name: Health check
run: | run: |
sleep 20 sleep 20
curl -f https://api.${{ secrets.DRB_DOMAIN }}/health || \ BODY=$(curl -fsS https://api.${{ secrets.DRB_DOMAIN }}/health) || {
(echo "Health check failed" && exit 1) echo "Health check failed: /health did not respond"; exit 1; }
echo "$BODY"
# Liveness alone is not enough. A deploy can report success while the
# PREVIOUS container keeps serving -- that is how production ran
# 08-18 code for two days without a single red run. Assert that the
# build which answered is the commit we just pushed.
RUNNING=$(printf '%s' "$BODY" | tr ',' '\n' | grep git_sha | cut -d'"' -f4)
if [ "$RUNNING" != "${{ gitea.sha }}" ]; then
echo "Deployed build is '$RUNNING', expected '${{ gitea.sha }}'."
echo "The container was not actually replaced."
exit 1
fi
notify-failure:
name: Report a failed deploy
needs: [build, deploy]
if: failure()
runs-on: ubuntu-latest
steps:
- name: Post to Discord
# A red run in Gitea is only visible to someone who opens Gitea, and
# nobody did for two days. Same shape as an AI tier dying quietly,
# which is why both now push a message out of the box instead of
# waiting to be discovered. No webhook configured => skip quietly
# rather than fail, since not every deployment will set one.
env:
WEBHOOK: ${{ secrets.DEPLOY_ALERT_WEBHOOK }}
RUN_URL: ${{ gitea.server_url }}/${{ gitea.repository }}/actions/runs/${{ gitea.run_number }}
SHA: ${{ gitea.sha }}
run: |
if [ -z "$WEBHOOK" ]; then
echo "DEPLOY_ALERT_WEBHOOK is not set - skipping notification."
exit 0
fi
python3 - <<'PY' > /tmp/payload.json
import json, os
print(json.dumps({"content":
"**DRB deploy failed** on `%s`\n%s\nProduction is still running the previous build."
% (os.environ["SHA"][:8], os.environ["RUN_URL"])}))
PY
curl -sS -X POST -H "Content-Type: application/json" \
--data @/tmp/payload.json "$WEBHOOK" || echo "notification POST failed"
+6
View File
@@ -8,4 +8,10 @@ RUN pip install uv && uv pip install --system --no-cache-dir -r requirements.txt
COPY app/ ./app/ COPY app/ ./app/
COPY tests/ ./tests/ COPY tests/ ./tests/
# Stamped by CI so /health can prove WHICH build is running. A deploy that
# reports success while the old container keeps running is otherwise silent
# -- exactly how production served two-day-old code for two days.
ARG GIT_SHA=unknown
ENV GIT_SHA=$GIT_SHA
CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000"] CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000"]
+7
View File
@@ -134,6 +134,13 @@ class Settings(BaseSettings):
# CORS — set to your frontend origin(s) in production, e.g. ["https://app.example.com"] # CORS — set to your frontend origin(s) in production, e.g. ["https://app.example.com"]
# Defaults to "*" for local development only. # Defaults to "*" for local development only.
#
# Leaving this as "*" is not merely permissive: main.py turns OFF
# allow_credentials when it sees a wildcard, because Starlette would
# otherwise reflect each caller's origin back WITH
# Access-Control-Allow-Credentials. So a production deployment that
# forgets to set this gets a loud ERROR at startup and loses credentialed
# cross-origin requests, rather than silently accepting every origin.
cors_origins: list[str] = ["*"] cors_origins: list[str] = ["*"]
# Discord webhook URL that app/internal/ai_health.py posts to when an AI # Discord webhook URL that app/internal/ai_health.py posts to when an AI
+36 -2
View File
@@ -1,3 +1,4 @@
import os
import asyncio import asyncio
from contextlib import asynccontextmanager from contextlib import asynccontextmanager
from fastapi import FastAPI, Depends from fastapi import FastAPI, Depends
@@ -77,12 +78,33 @@ async def lifespan(app: FastAPI):
app = FastAPI(title="DRB C2 Core", lifespan=lifespan) app = FastAPI(title="DRB C2 Core", lifespan=lifespan)
# "*" plus allow_credentials=True is not the permissive-but-harmless setting it
# looks like. Starlette does not refuse the combination -- it reflects the
# caller's Origin back and still sends Access-Control-Allow-Credentials: true,
# so the effective policy becomes "any origin, with credentials", the opposite
# of what a wildcard normally means. Rather than trust every deployment to
# remember to override CORS_ORIGINS, make the dangerous pair unrepresentable.
def cors_allows_credentials(origins: list[str]) -> bool:
"""False when any entry is a wildcard. Extracted so it can be tested
without re-importing this module, which drags in every router."""
return "*" not in origins
_cors_is_wildcard = not cors_allows_credentials(settings.cors_origins)
if _cors_is_wildcard:
logger.error(
"CORS_ORIGINS is '*', so credentialed cross-origin requests are being "
"DISABLED to avoid reflecting every caller's origin back with "
"Access-Control-Allow-Credentials. Set CORS_ORIGINS to your frontend "
"origin(s) in production, e.g. [\"https://app.example.com\"]."
)
app.add_middleware( app.add_middleware(
CORSMiddleware, CORSMiddleware,
allow_origins=settings.cors_origins, allow_origins=settings.cors_origins,
allow_methods=["*"], allow_methods=["*"],
allow_headers=["*"], allow_headers=["*"],
allow_credentials=True, allow_credentials=not _cors_is_wildcard,
) )
app.include_router(nodes.router, dependencies=[Depends(require_service_or_firebase_token)]) app.include_router(nodes.router, dependencies=[Depends(require_service_or_firebase_token)])
@@ -118,9 +140,21 @@ app.include_router(media.router)
# against a future /internal/* route being added and forgotten there. # against a future /internal/* route being added and forgotten there.
# Read straight from the environment rather than through Settings: this is a
# build stamp baked in by the Dockerfile, not configuration anyone sets or
# tunes, and keeping it out of Settings avoids implying it can be changed.
_GIT_SHA = os.getenv("GIT_SHA", "unknown")
@app.get("/health") @app.get("/health")
async def health(): async def health():
return {"ok": True, "mqtt_connected": mqtt_handler.is_connected} return {
"ok": True,
"mqtt_connected": mqtt_handler.is_connected,
# CI asserts this equals the commit it just deployed. Without it a
# deploy can "succeed" while the previous container is still serving.
"git_sha": _GIT_SHA,
}
# Deliberately unauthenticated, same as /health above: the CI deploy step # Deliberately unauthenticated, same as /health above: the CI deploy step
+9
View File
@@ -40,6 +40,15 @@ except ModuleNotFoundError:
_auth.set_custom_user_claims = MagicMock() _auth.set_custom_user_claims = MagicMock()
_auth.get_user_by_email = MagicMock() _auth.get_user_by_email = MagicMock()
_auth.get_user = MagicMock() _auth.get_user = MagicMock()
# Type used in annotations at import time by routers/users.py, so it has to
# exist as a name even though nothing here ever instantiates it. Without it,
# importing app.main -- and therefore testing anything wired at app level,
# like the CORS policy -- fails at collection.
_auth.UserRecord = MagicMock()
_auth.list_users = MagicMock()
_auth.update_user = MagicMock()
_auth.create_user = MagicMock()
_auth.delete_user = MagicMock()
_firebase.auth = _auth _firebase.auth = _auth
_firebase.credentials = _credentials _firebase.credentials = _credentials
+53
View File
@@ -0,0 +1,53 @@
"""
CORS must never end up as "any origin, WITH credentials".
Starlette does not reject `allow_origins=["*"]` combined with
`allow_credentials=True`. It reflects the caller's Origin back in
Access-Control-Allow-Origin and still sends
Access-Control-Allow-Credentials: true, so the effective policy is the
opposite of what a wildcard usually means. main.py defuses that by turning
credentials off whenever it sees a wildcard; these tests hold it to that.
The policy lives in a pure function so it can be exercised directly --
reloading app.main to vary settings drags every router back through import
and is not worth the fragility.
"""
from starlette.middleware.cors import CORSMiddleware
from app.config import settings
from app.main import app, cors_allows_credentials
def test_wildcard_alone_disables_credentials():
assert cors_allows_credentials(["*"]) is False
def test_wildcard_among_real_origins_still_disables_credentials():
# A list that merely CONTAINS "*" is as permissive as ["*"] alone --
# Starlette treats any wildcard entry as allow-all.
assert cors_allows_credentials(["https://app.example.com", "*"]) is False
def test_named_origins_keep_credentials():
# Naming your origins is how you ask for credentialed requests, so a
# correctly configured deployment must not be penalised.
assert cors_allows_credentials(["https://app.example.com"]) is True
assert cors_allows_credentials([]) is True
def test_the_app_actually_mounted_that_policy():
"""Guards the wiring, not just the helper: a future edit to main.py that
hardcodes allow_credentials=True again fails here."""
opts = next(
(mw.kwargs for mw in app.user_middleware if mw.cls is CORSMiddleware), None
)
assert opts is not None, "CORSMiddleware is not mounted at all"
assert opts["allow_credentials"] is cors_allows_credentials(settings.cors_origins)
def test_health_exposes_a_build_stamp():
"""CI compares this against the commit it just deployed; a deploy that
leaves the previous container running is otherwise invisible."""
from app.main import _GIT_SHA
assert isinstance(_GIT_SHA, str) and _GIT_SHA