Make a failed deploy impossible to miss, and a wildcard CORS harmless
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:
@@ -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"
|
||||||
|
|||||||
@@ -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"]
|
||||||
|
|||||||
@@ -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
@@ -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
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user