6 Commits

Author SHA1 Message Date
75a1449354 Chore: Bump manifests for v1.1.1 release 2026-05-09 13:55:44 -04:00
68881b30f1 Ops: Add script to test lockout with spoofed X-Forwarded-For headers 2026-05-09 13:54:49 -04:00
9021f4816a Fix: Prefer X-Real-IP over XFF[0] in get_client_ip to close spoof bypass
XFF[0] is attacker-controllable; a crafted X-Forwarded-For header could
attribute login failures to a victim IP, triggering their lockout while
the attacker accumulates none. ingress-nginx sets X-Real-IP via its
realip module using an authoritative CIDR allowlist and overwrites any
client-supplied value, making it spoof-resistant. Fallback to XFF[0]
is retained for defence in depth but now emits a warning if reached.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 17:52:05 +00:00
35d21dafa4 Fix: Strip whitespace from S3_PUBLIC_BASE_URL before building CDN URLs
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 00:35:22 +00:00
34d8c3848b Ops: Bump manifests for v1.1.0 release 2026-05-08 20:25:32 -04:00
aaacfae653 Feat: Serve images directly from Cloudflare R2 CDN
API responses now include file_url and thumbnail_url fields. When
S3_PUBLIC_BASE_URL is configured, these point to the CDN domain;
when unset, they fall back to the existing API proxy paths so local
dev requires no additional setup. UI updated to use response URL
fields directly instead of constructing proxy URLs client-side.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 00:17:22 +00:00
24 changed files with 762 additions and 37 deletions

View File

@@ -11,6 +11,10 @@ S3_REGION=us-east-1
# Angular SPA — injected at build or runtime
API_BASE_URL=http://localhost:8000
# CDN base URL for serving images (e.g. https://cdn.example.com).
# Leave empty in local dev to use API proxy fallback.
S3_PUBLIC_BASE_URL=
# Upload size limit in bytes (default 50 MiB)
MAX_UPLOAD_BYTES=52428800

View File

@@ -1,3 +1 @@
{
"feature_directory": "specs/013-k8s-manifests"
}
{"feature_directory": "specs/014-r2-cdn-serving"}

View File

@@ -1,5 +1,5 @@
<!-- SPECKIT START -->
For additional context about technologies to be used, project structure,
shell commands, and other important information, read the current plan at
`specs/013-k8s-manifests/plan.md`.
`specs/014-r2-cdn-serving/plan.md`.
<!-- SPECKIT END -->

View File

@@ -14,20 +14,30 @@ def get_client_ip(
request: Request,
trusted_networks: list[IPv4Network | IPv6Network],
) -> str:
"""Return the resolved client IP, honouring X-Forwarded-For when the
TCP peer is a trusted upstream proxy. Falls back to the TCP peer address
when no trusted networks are configured or the peer is not in the list."""
"""Return the resolved client IP.
Prefers X-Real-IP over X-Forwarded-For when the TCP peer is a trusted
proxy. ingress-nginx sets X-Real-IP via its realip module using an
authoritative CIDR allowlist; it overwrites any client-supplied value, so
it cannot be spoofed via XFF injection. XFF[0] is the fallback for paths
that lack nginx (none currently exist, but kept for defence in depth).
"""
peer = request.client.host if request.client else "unknown"
if trusted_networks and peer != "unknown":
try:
peer_addr = ipaddress.ip_address(peer)
if any(peer_addr in net for net in trusted_networks):
xff = request.headers.get("X-Forwarded-For", "").split(",")[0].strip()
if xff:
return xff
real_ip = request.headers.get("X-Real-IP", "").strip()
if real_ip:
return real_ip
# XFF[0] fallback — warn because this path should not be
# reached in production (nginx always sets X-Real-IP).
xff = request.headers.get("X-Forwarded-For", "").split(",")[0].strip()
if xff:
logger.warning(
"X-Real-IP absent from trusted peer %s; falling back to XFF[0]", peer
)
return xff
except ValueError:
pass
return peer

View File

@@ -14,6 +14,7 @@ class Settings(BaseSettings):
s3_secret_access_key: str
s3_region: str = "us-east-1"
api_base_url: str = "http://localhost:8000"
s3_public_base_url: str | None = None
max_upload_bytes: int = 52_428_800 # 50 MiB
jwt_secret_key: str
jwt_expiry_seconds: int = 86400

View File

@@ -27,7 +27,16 @@ def _error(detail: str, code: str, status: int):
raise HTTPException(status_code=status, detail={"detail": detail, "code": code})
def _image_to_dict(image: Image, *, duplicate: bool | None = None) -> dict[str, Any]:
def _image_to_dict(
image: Image, *, cdn_base: str | None = None, duplicate: bool | None = None
) -> dict[str, Any]:
_base = cdn_base.strip().rstrip("/") if cdn_base else None
file_url = f"{_base}/{image.storage_key}" if _base else f"/api/v1/images/{image.id}/file"
thumbnail_url = (
(f"{_base}/{image.thumbnail_key}" if _base else f"/api/v1/images/{image.id}/thumbnail")
if image.thumbnail_key
else None
)
data: dict[str, Any] = {
"id": str(image.id),
"hash": image.hash,
@@ -38,6 +47,8 @@ def _image_to_dict(image: Image, *, duplicate: bool | None = None) -> dict[str,
"height": image.height,
"storage_key": image.storage_key,
"thumbnail_key": image.thumbnail_key,
"file_url": file_url,
"thumbnail_url": thumbnail_url,
"created_at": image.created_at.isoformat(),
"tags": image.tags,
}
@@ -133,10 +144,13 @@ async def upload_image(
hash_hex = compute_sha256(data)
image_repo = ImageRepository(db)
_cdn_base = settings.s3_public_base_url
existing = await image_repo.get_by_hash(hash_hex)
if existing:
return Response(
content=__import__("json").dumps(_image_to_dict(existing, duplicate=True)),
content=__import__("json").dumps(
_image_to_dict(existing, cdn_base=_cdn_base, duplicate=True)
),
status_code=200,
media_type="application/json",
)
@@ -183,7 +197,7 @@ async def upload_image(
await tag_repo.attach_tags(image, tag_names)
image = await image_repo.reload_with_tags(image.id)
return _image_to_dict(image, duplicate=False)
return _image_to_dict(image, cdn_base=_cdn_base, duplicate=False)
@router.get("/images")
@@ -192,13 +206,15 @@ async def list_images(
limit: int = 50,
offset: int = 0,
db: AsyncSession = Depends(get_db),
settings=Depends(get_settings),
):
limit = min(limit, 100)
_cdn_base = settings.s3_public_base_url
tag_names = [t.strip() for t in tags.split(",") if t.strip()] if tags else None
image_repo = ImageRepository(db)
images, total = await image_repo.list_images(tag_names=tag_names, limit=limit, offset=offset)
return {
"items": [_image_to_dict(img) for img in images],
"items": [_image_to_dict(img, cdn_base=_cdn_base) for img in images],
"total": total,
"limit": limit,
"offset": offset,
@@ -209,7 +225,9 @@ async def list_images(
async def get_image(
image_id: uuid.UUID,
db: AsyncSession = Depends(get_db),
settings=Depends(get_settings),
):
_cdn_base = settings.s3_public_base_url
image_repo = ImageRepository(db)
image = await image_repo.get_by_id(image_id)
if not image:
@@ -217,7 +235,7 @@ async def get_image(
status_code=404,
detail={"detail": "Image not found", "code": "image_not_found"},
)
return _image_to_dict(image)
return _image_to_dict(image, cdn_base=_cdn_base)
@router.get("/images/{image_id}/file")
@@ -288,7 +306,9 @@ async def update_image_tags(
body: dict,
db: AsyncSession = Depends(get_db),
_: Identity = Depends(require_auth),
settings=Depends(get_settings),
):
_cdn_base = settings.s3_public_base_url
image_repo = ImageRepository(db)
image = await image_repo.get_by_id(image_id)
if not image:
@@ -309,7 +329,7 @@ async def update_image_tags(
await tag_repo.replace_tags_on_image(image, tag_names)
image = await image_repo.reload_with_tags(image.id)
return _image_to_dict(image)
return _image_to_dict(image, cdn_base=_cdn_base)
@router.delete("/images/{image_id}", status_code=204)

View File

@@ -132,6 +132,10 @@ async def test_upload_returns_thumbnail_key(authed_client):
assert "thumbnail_key" in body
assert body["thumbnail_key"] is not None
assert body["thumbnail_key"].endswith("-thumb")
assert "file_url" in body
assert body["file_url"].startswith("/api/v1/images/")
assert "thumbnail_url" in body
assert body["thumbnail_url"].startswith("/api/v1/images/")
@pytest.mark.asyncio
@@ -172,3 +176,6 @@ async def test_upload_succeeds_when_thumbnail_fails(authed_client):
assert response.status_code in (200, 201)
body = response.json()
assert body["thumbnail_key"] is None
assert "file_url" in body
assert body["file_url"].startswith("/api/v1/images/")
assert body["thumbnail_url"] is None

View File

@@ -80,10 +80,17 @@ def test_get_client_ip_no_trusted_networks_returns_peer():
assert get_client_ip(req, []) == "203.0.113.1"
def test_get_client_ip_trusted_peer_uses_xff():
req = make_request("10.0.0.1", {"X-Forwarded-For": "203.0.113.5"})
def test_get_client_ip_trusted_peer_uses_real_ip():
req = make_request("10.0.0.1", {"X-Real-IP": "203.0.113.9"})
nets = [ipaddress.ip_network("10.0.0.0/8")]
assert get_client_ip(req, nets) == "203.0.113.5"
assert get_client_ip(req, nets) == "203.0.113.9"
def test_get_client_ip_real_ip_wins_over_xff():
# Regression: spoofed XFF must not override nginx-set X-Real-IP.
req = make_request("10.0.0.1", {"X-Real-IP": "203.0.113.9", "X-Forwarded-For": "1.2.3.4"})
nets = [ipaddress.ip_network("10.0.0.0/8")]
assert get_client_ip(req, nets) == "203.0.113.9"
def test_get_client_ip_untrusted_peer_ignores_xff():
@@ -92,7 +99,7 @@ def test_get_client_ip_untrusted_peer_ignores_xff():
assert get_client_ip(req, nets) == "8.8.8.8"
def test_get_client_ip_trusted_peer_falls_back_to_real_ip():
req = make_request("10.0.0.1", {"X-Real-IP": "203.0.113.9"})
def test_get_client_ip_trusted_peer_falls_back_to_xff_when_no_real_ip():
req = make_request("10.0.0.1", {"X-Forwarded-For": "203.0.113.5"})
nets = [ipaddress.ip_network("10.0.0.0/8")]
assert get_client_ip(req, nets) == "203.0.113.9"
assert get_client_ip(req, nets) == "203.0.113.5"

View File

@@ -0,0 +1,65 @@
import uuid
from unittest.mock import MagicMock
import pytest
from app.routers.images import _image_to_dict
def _make_image(*, thumbnail_key=None):
img = MagicMock()
img.id = uuid.UUID("00000000-0000-0000-0000-000000000001")
img.hash = "abc123"
img.filename = "test.jpg"
img.mime_type = "image/jpeg"
img.size_bytes = 1024
img.width = 100
img.height = 100
img.storage_key = "abc123storagekey"
img.thumbnail_key = thumbnail_key
img.created_at.isoformat.return_value = "2026-05-09T00:00:00"
img.tags = []
return img
def test_cdn_configured_with_thumbnail():
img = _make_image(thumbnail_key="abc123storagekey-thumb")
result = _image_to_dict(img, cdn_base="https://cdn.example.com")
assert result["file_url"] == "https://cdn.example.com/abc123storagekey"
assert result["thumbnail_url"] == "https://cdn.example.com/abc123storagekey-thumb"
def test_cdn_configured_no_thumbnail():
img = _make_image(thumbnail_key=None)
result = _image_to_dict(img, cdn_base="https://cdn.example.com")
assert result["file_url"] == "https://cdn.example.com/abc123storagekey"
assert result["thumbnail_url"] is None
def test_no_cdn_with_thumbnail():
img = _make_image(thumbnail_key="abc123storagekey-thumb")
result = _image_to_dict(img, cdn_base=None)
assert result["file_url"] == "/api/v1/images/00000000-0000-0000-0000-000000000001/file"
assert result["thumbnail_url"] == "/api/v1/images/00000000-0000-0000-0000-000000000001/thumbnail"
def test_no_cdn_no_thumbnail():
img = _make_image(thumbnail_key=None)
result = _image_to_dict(img, cdn_base=None)
assert result["file_url"] == "/api/v1/images/00000000-0000-0000-0000-000000000001/file"
assert result["thumbnail_url"] is None
def test_cdn_trailing_slash_normalised():
img = _make_image(thumbnail_key="abc123storagekey-thumb")
result = _image_to_dict(img, cdn_base="https://cdn.example.com/")
assert result["file_url"] == "https://cdn.example.com/abc123storagekey"
assert result["thumbnail_url"] == "https://cdn.example.com/abc123storagekey-thumb"
assert "//" not in result["file_url"].replace("https://", "")
def test_cdn_trailing_whitespace_normalised():
img = _make_image(thumbnail_key="abc123storagekey-thumb")
result = _image_to_dict(img, cdn_base="https://cdn.example.com ")
assert result["file_url"] == "https://cdn.example.com/abc123storagekey"
assert result["thumbnail_url"] == "https://cdn.example.com/abc123storagekey-thumb"

View File

@@ -15,7 +15,7 @@ spec:
spec:
initContainers:
- name: migrate
image: git.juggalol.com/juggalol/reactbin-api:v1.0.1
image: git.juggalol.com/juggalol/reactbin-api:v1.1.1
command: ["alembic", "upgrade", "head"]
workingDir: /app
envFrom:
@@ -26,7 +26,7 @@ spec:
runAsUser: 1001
containers:
- name: api
image: git.juggalol.com/juggalol/reactbin-api:v1.0.1
image: git.juggalol.com/juggalol/reactbin-api:v1.1.1
ports:
- containerPort: 8000
envFrom:

View File

@@ -15,7 +15,7 @@ spec:
spec:
containers:
- name: ui
image: git.juggalol.com/juggalol/reactbin-ui:v1.0.1
image: git.juggalol.com/juggalol/reactbin-ui:v1.1.1
ports:
- containerPort: 8080
livenessProbe:

67
scripts/test_lockout.sh Normal file
View File

@@ -0,0 +1,67 @@
#!/usr/bin/env bash
#
# Test reactbin's login rate limiter and demonstrate the XFF injection bypass.
#
# Phase 1: Send 6 bad login attempts in quick succession.
# Attempts 1-5 should return 401 (invalid credentials).
# Attempt 6 should return 429 (rate limited) — the limiter blocks after
# max_failures=5 within the window.
#
# Phase 2: Send a 7th bad attempt with a spoofed X-Forwarded-For header
# pointing at a different IP. If the lockout keys correctly on the trusted
# client IP, this should still return 429 (same client, still locked).
# If reactbin trusts client-supplied XFF blindly, this would return 401
# instead — the spoof would make the request look like a different client
# that hasn't accumulated failures.
#
# Interpretation:
# - 429 on attempt 7 → lockout is correctly identifying the client
# - 401 on attempt 7 → XFF injection succeeded; server treated us as a
# new client because we set a fake XFF
#
# Note: this script is ONLY useful when run against the public origin path
# where XFF spoofing is potentially possible. It does not exercise the
# Cloudflare-proxied path because Cloudflare strips/replaces XFF before
# forwarding to origin.
set -u
URL="${URL:-https://reactbin.juggalol.com/api/v1/auth/token}"
SPOOFED_IP="${SPOOFED_IP:-198.51.100.99}" # TEST-NET-2, never routed
USERNAME="${USERNAME:-not-a-real-user}"
PASSWORD="${PASSWORD:-not-a-real-password}"
# JSON body for a bad login. Username/password chosen to be obviously fake;
# adjust if your auth provider has its own validation that would 400 instead
# of 401 on these values.
BODY=$(printf '{"username":"%s","password":"%s"}' "$USERNAME" "$PASSWORD")
echo "Target: $URL"
echo "Body: $BODY"
echo
echo "=== Phase 1: 6 bad logins from real client IP ==="
for i in 1 2 3 4 5 6; do
code=$(curl -sS -o /dev/null -w '%{http_code}' \
-X POST \
-H 'Content-Type: application/json' \
--data "$BODY" \
"$URL")
echo "Attempt $i: HTTP $code"
done
echo
echo "=== Phase 2: 7th attempt with spoofed X-Forwarded-For ==="
echo "Setting X-Forwarded-For: $SPOOFED_IP"
code=$(curl -sS -o /dev/null -w '%{http_code}' \
-X POST \
-H 'Content-Type: application/json' \
-H "X-Forwarded-For: $SPOOFED_IP" \
--data "$BODY" \
"$URL")
echo "Attempt 7: HTTP $code"
echo
echo "Interpretation:"
echo " Attempt 7 = 429 → lockout correctly tracks real client; XFF spoof ineffective"
echo " Attempt 7 = 401 → XFF spoof succeeded; server believed the fake client IP"

View File

@@ -0,0 +1,34 @@
# Specification Quality Checklist: CDN Image Serving
**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2026-05-08
**Feature**: [spec.md](../spec.md)
## Content Quality
- [X] No implementation details (languages, frameworks, APIs)
- [X] Focused on user value and business needs
- [X] Written for non-technical stakeholders
- [X] All mandatory sections completed
## Requirement Completeness
- [X] No [NEEDS CLARIFICATION] markers remain
- [X] Requirements are testable and unambiguous
- [X] Success criteria are measurable
- [X] Success criteria are technology-agnostic (no implementation details)
- [X] All acceptance scenarios are defined
- [X] Edge cases are identified
- [X] Scope is clearly bounded
- [X] Dependencies and assumptions identified
## Feature Readiness
- [X] All functional requirements have clear acceptance criteria
- [X] User scenarios cover primary flows
- [X] Feature meets measurable outcomes defined in Success Criteria
- [X] No implementation details leak into specification
## Notes
- All items pass. Ready for `/speckit-plan`.

View File

@@ -0,0 +1,54 @@
# Contract: Image Metadata Response
**Version**: 2.0 (adds `file_url`, `thumbnail_url`)
**Endpoints affected**: `GET /api/v1/images`, `GET /api/v1/images/{id}`, `POST /api/v1/images`, `PATCH /api/v1/images/{id}/tags`
## Response Schema
```json
{
"id": "550e8400-e29b-41d4-a716-446655440000",
"hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
"filename": "reaction.gif",
"mime_type": "image/gif",
"size_bytes": 204800,
"width": 480,
"height": 270,
"storage_key": "e3b0c44298fc1c149afbf4c8996fb924",
"thumbnail_key": "e3b0c44298fc1c149afbf4c8996fb924.thumb",
"file_url": "https://cdn.reactbin.juggalol.com/e3b0c44298fc1c149afbf4c8996fb924",
"thumbnail_url": "https://cdn.reactbin.juggalol.com/e3b0c44298fc1c149afbf4c8996fb924.thumb",
"created_at": "2026-05-08T12:00:00.000000",
"tags": ["funny", "reaction"]
}
```
## Field Descriptions
| Field | Type | Nullable | Notes |
|-------|------|----------|-------|
| `id` | string (UUID) | No | Stable image identifier |
| `hash` | string (hex) | No | SHA-256 of file content; deduplication key |
| `filename` | string | No | Original upload filename |
| `mime_type` | string | No | One of: `image/jpeg`, `image/png`, `image/gif`, `image/webp` |
| `size_bytes` | integer | No | File size in bytes |
| `width` | integer | No | Image width in pixels |
| `height` | integer | No | Image height in pixels |
| `storage_key` | string | No | Object storage key (retained for backward compat) |
| `thumbnail_key` | string | Yes | Thumbnail object storage key; null if generation failed |
| `file_url` | string | No | Full URL to fetch the image file — CDN URL in production, API proxy path in local dev |
| `thumbnail_url` | string | Yes | Full URL to fetch the thumbnail — CDN URL in production, API proxy path in local dev; null if no thumbnail |
| `created_at` | string (ISO 8601) | No | Upload timestamp |
| `tags` | string[] | No | Lowercase normalised tag list |
| `duplicate` | boolean | Yes | Present only on upload responses; true if hash matched an existing image |
## URL Behaviour
| Configuration | `file_url` example | `thumbnail_url` example |
|---------------|--------------------|------------------------|
| `S3_PUBLIC_BASE_URL` set | `https://cdn.reactbin.juggalol.com/{storage_key}` | `https://cdn.reactbin.juggalol.com/{thumbnail_key}` |
| `S3_PUBLIC_BASE_URL` not set | `/api/v1/images/{id}/file` | `/api/v1/images/{id}/thumbnail` |
## UI Contract
The UI MUST use `file_url` and `thumbnail_url` from the response to render images. The UI MUST NOT construct image URLs from `id`, `storage_key`, or `thumbnail_key` directly. The UI MUST treat `thumbnail_url: null` as "no thumbnail available" and fall back to `file_url` for display.

View File

@@ -0,0 +1,137 @@
# Implementation Plan: CDN Image Serving
**Branch**: `014-r2-cdn-serving` | **Date**: 2026-05-08 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `specs/014-r2-cdn-serving/spec.md`
## Summary
Extend the image metadata API response to include `file_url` and `thumbnail_url` fields. When `S3_PUBLIC_BASE_URL` is configured, these fields contain CDN URLs pointing directly to Cloudflare R2. When unconfigured, they fall back to the existing API proxy paths so local development requires no setup changes. The UI is updated to use these response fields instead of constructing proxy URLs client-side. Proxy endpoints are retained unchanged.
## Technical Context
**Language/Version**: Python 3.12 (API), TypeScript strict mode (UI)
**Primary Dependencies**: FastAPI, SQLAlchemy 2.x async, Angular (latest stable), pydantic-settings
**Storage**: PostgreSQL (image metadata), S3-compatible object storage (R2 in production, MinIO in dev)
**Testing**: pytest (unit + integration), Angular component tests
**Target Platform**: Linux (k3s), local Docker Compose
**Project Type**: Web service (API) + SPA (UI)
**Performance Goals**: No additional latency on API responses; image load latency reduced by eliminating API proxy hop in production
**Constraints**: No breaking changes to existing API response fields; proxy endpoints must remain functional
**Scale/Scope**: Single-owner app; ~100 existing images migrated to R2 prior to this feature
## Constitution Check
| Principle | Status | Notes |
|-----------|--------|-------|
| §2.1 Strict separation of concerns | PASS | URL construction stays in router layer; storage backend unchanged |
| §2.3 Storage abstraction | PASS | No changes to `StorageBackend` interface or `S3StorageBackend` |
| §2.6 No speculative abstraction | PASS | No new interfaces introduced; URL logic is a simple helper |
| §3.1 API versioning (`/api/v1/`) | PASS | Adding fields to response is non-breaking per §3.1 |
| §3.2 OpenAPI as contract | PASS | New fields documented in contracts/image-response.md |
| §5.1 Tests alongside implementation | REQUIRED | Unit tests for URL construction; integration tests for response fields |
| §7.2 Environment configuration | PASS | `S3_PUBLIC_BASE_URL` via env var; no hardcoded URLs |
No constitution violations. All gates pass.
## Project Structure
### Documentation (this feature)
```text
specs/014-r2-cdn-serving/
├── plan.md # This file
├── research.md # Technical decisions
├── contracts/
│ └── image-response.md # Updated image response schema
├── quickstart.md # Integration test scenarios
└── tasks.md # Phase 2 output (speckit-tasks)
```
### Source Code Changes
```text
api/
├── app/
│ ├── config.py # Add: s3_public_base_url: str | None = None
│ └── routers/
│ └── images.py # Update: _image_to_dict gains cdn_base param;
│ # add file_url + thumbnail_url to response;
│ # pass cdn_base from get_settings() at endpoint level
├── tests/
│ ├── unit/
│ │ └── test_url_construction.py # New: pure unit tests for URL logic
│ └── integration/
│ └── test_images.py # Update: assert file_url + thumbnail_url present in responses
ui/src/app/
├── services/
│ └── image.service.ts # Update: add file_url/thumbnail_url to ImageRecord;
│ # remove getFileUrl()/getThumbnailUrl() methods
├── library/
│ └── library.component.ts # Update: use img.thumbnail_url instead of getThumbnailUrl(img.id)
└── detail/
└── detail.component.ts # Update: use img.file_url instead of getFileUrl(img.id)
.env.example # Add: S3_PUBLIC_BASE_URL= (empty = local dev proxy fallback)
```
## Key Implementation Details
### URL construction logic (`api/app/routers/images.py`)
`_image_to_dict` gains a `cdn_base: str | None` parameter:
```python
def _image_to_dict(image: Image, *, cdn_base: str | None = None, duplicate: bool | None = None):
base = cdn_base.rstrip("/") if cdn_base else None
file_url = f"{base}/{image.storage_key}" if base else f"/api/v1/images/{image.id}/file"
thumbnail_url = (
(f"{base}/{image.thumbnail_key}" if base else f"/api/v1/images/{image.id}/thumbnail")
if image.thumbnail_key else None
)
return {
..., # existing fields unchanged
"file_url": file_url,
"thumbnail_url": thumbnail_url,
}
```
Each endpoint calls `get_settings()` once and passes `settings.s3_public_base_url` as `cdn_base`.
### Config addition (`api/app/config.py`)
```python
s3_public_base_url: str | None = None
```
No validator needed — `None` is the valid "not configured" state.
### UI changes (`ui/src/app/services/image.service.ts`)
`ImageRecord` gains two new fields:
```typescript
file_url: string;
thumbnail_url: string | null;
```
`getFileUrl(id)` and `getThumbnailUrl(id)` methods are removed. Components use `image.file_url` and `image.thumbnail_url` directly.
## Phase Breakdown
### Phase 1: API — config + URL construction (US1 foundation)
- Add `s3_public_base_url` to config
- Update `_image_to_dict` with `cdn_base` parameter
- Update all call sites to pass `cdn_base` from settings
- Unit tests for URL construction (both CDN and fallback paths)
- Integration tests verifying `file_url`/`thumbnail_url` in all image responses
### Phase 2: UI — consume response URLs (US1 + US2)
- Update `ImageRecord` interface
- Remove `getFileUrl`/`getThumbnailUrl` methods from service
- Update library component
- Update detail component
- Update service tests
### Phase 3: Config + docs
- Add `S3_PUBLIC_BASE_URL` to `.env.example`
- Manual end-to-end verification (local dev + production)

View File

@@ -0,0 +1,66 @@
# Quickstart: CDN Image Serving
## Local development (no CDN)
No configuration change required. `S3_PUBLIC_BASE_URL` is unset by default.
```bash
docker compose up
```
Upload an image and inspect the API response:
```bash
curl -s http://localhost:8000/api/v1/images | jq '.items[0] | {file_url, thumbnail_url}'
```
Expected (local dev — relative proxy paths):
```json
{
"file_url": "/api/v1/images/550e8400-.../file",
"thumbnail_url": "/api/v1/images/550e8400-.../thumbnail"
}
```
The UI loads images via these relative paths, which hit the API proxy as before.
---
## Production (CDN configured)
Add `S3_PUBLIC_BASE_URL` to the Vault secret bundle at `reactbin/api/config`:
```
S3_PUBLIC_BASE_URL = https://cdn.reactbin.juggalol.com
```
Force VSO sync and restart:
```bash
kubectl annotate vaultstaticsecret api-secret -n reactbin \
secrets.hashicorp.com/force-sync=$(date +%s) --overwrite
kubectl rollout restart deployment/api -n reactbin
```
Upload a test image and inspect the response:
```bash
curl -s https://reactbin.juggalol.com/api/v1/images | jq '.items[0] | {file_url, thumbnail_url}'
```
Expected (production — CDN URLs):
```json
{
"file_url": "https://cdn.reactbin.juggalol.com/e3b0c44...",
"thumbnail_url": "https://cdn.reactbin.juggalol.com/e3b0c44....thumb"
}
```
Open the browser network panel on the library page and confirm image requests go to `cdn.reactbin.juggalol.com`, not `/api/`.
---
## Verifying existing images after migration
All existing images were migrated to R2 with the same object keys before this feature was deployed. Once `S3_PUBLIC_BASE_URL` is configured, the API will return CDN URLs for all images immediately — no per-image migration step is needed.

View File

@@ -0,0 +1,51 @@
# Research: CDN Image Serving
## Decision 1: Where does URL construction logic live?
**Decision**: In the image router's `_image_to_dict` helper, not in the `StorageBackend`.
**Rationale**: The `StorageBackend` interface is responsible for put/get/delete of object bytes. Adding URL construction there conflates two concerns — storage operations and HTTP URL generation — and would require the storage abstraction to know about CDN configuration. The router already has access to application settings via `get_settings()` and knows the image ID and storage key, making it the natural place to construct URLs.
**Alternatives considered**: Adding a `get_url(key)` method to `StorageBackend` — rejected because it leaks HTTP/CDN concerns into the storage abstraction, violating §2.3.
---
## Decision 2: Fallback URL format in local development
**Decision**: Relative paths (`/api/v1/images/{id}/file`, `/api/v1/images/{id}/thumbnail`) when `S3_PUBLIC_BASE_URL` is not set.
**Rationale**: Relative paths work regardless of the host the app is running on, require no additional configuration, and match how the UI currently constructs these URLs via `getFileUrl(id)` and `getThumbnailUrl(id)`. An absolute fallback would require `API_BASE_URL` to be set in local dev, adding unnecessary setup friction.
**Alternatives considered**: Absolute URL fallback using `API_BASE_URL` — rejected because it adds a mandatory config dependency where none exists today.
---
## Decision 3: Trailing slash normalisation
**Decision**: Strip trailing slash from `S3_PUBLIC_BASE_URL` at construction time using `rstrip('/')` in the config validator or at point of use.
**Rationale**: Prevents double-slash URLs (`https://cdn.example.com//key`) if the operator includes a trailing slash in the configured value. Simple, defensive, zero-cost.
---
## Decision 4: Proxy endpoints retained or removed?
**Decision**: Retained, fully functional, unchanged.
**Rationale**: Spec FR-005 explicitly requires them. They serve as the local dev fallback and a safety net if the CDN is temporarily unavailable or misconfigured. Removing them would break local development immediately.
---
## Decision 5: `storage_key` and `thumbnail_key` in API response
**Decision**: Keep both fields in the response alongside the new `file_url` and `thumbnail_url`.
**Rationale**: Removing them is a breaking API change. The UI currently reads `thumbnail_key` to decide whether a thumbnail exists. After this change the UI will use `thumbnail_url` (null when no thumbnail), but the keys remain in the response for backward compatibility with any tooling.
---
## Decision 6: Settings access in `_image_to_dict`
**Decision**: Pre-compute the CDN base URL string once per request at the endpoint level and pass it into `_image_to_dict` as a parameter, rather than calling `get_settings()` inside the helper.
**Rationale**: Keeps `_image_to_dict` a pure function (easier to test), avoids calling `get_settings()` inside a helper that is called in a loop (image list endpoint), and makes the dependency explicit.

View File

@@ -0,0 +1,93 @@
# Feature Specification: CDN Image Serving
**Feature Branch**: `014-r2-cdn-serving`
**Created**: 2026-05-08
**Status**: Draft
**Input**: User description: "R2 CDN image serving with local dev fallback to API proxy"
## Overview
Images and thumbnails are currently served by proxying bytes through the API. This feature changes image delivery so that clients receive direct URLs pointing to a CDN edge network, eliminating the API as a middleman for image content. In local development, where no CDN is available, the API proxy endpoints remain as a fallback so the developer experience is unchanged.
---
## User Scenarios & Testing *(mandatory)*
### User Story 1 - Images Load Directly from CDN (Priority: P1)
When a visitor views the image library or opens an image detail page, images and thumbnails are fetched directly from the CDN rather than through the application server. The page loads faster because image bytes no longer pass through the API.
**Why this priority**: Core value of the feature. Reduces API load and improves image load speed for all users.
**Independent Test**: Upload an image, open the library page, and inspect the network requests. Image and thumbnail requests should go directly to the CDN domain, not to `/api/`. The API response for the image list should include direct CDN URLs for each image and thumbnail.
**Acceptance Scenarios**:
1. **Given** a published image, **When** the visitor loads the image library, **Then** each thumbnail `src` URL points to the CDN domain and loads without passing through the API
2. **Given** a published image, **When** the visitor opens the detail page, **Then** the full image `src` URL points to the CDN domain
3. **Given** the API returns image metadata, **When** the response is inspected, **Then** it includes a `file_url` and `thumbnail_url` field containing full CDN URLs
---
### User Story 2 - Local Development Works Without CDN (Priority: P2)
In local development, where no CDN is configured, images continue to load via the existing API proxy endpoints. No additional setup is required to run the application locally.
**Why this priority**: Developer experience must not regress. The proxy endpoints must remain functional and be used automatically when no CDN is configured.
**Independent Test**: Run the application locally without setting a public base URL. Upload an image. Verify the library and detail pages load images correctly via the API proxy endpoints, with no errors or broken images.
**Acceptance Scenarios**:
1. **Given** no CDN base URL is configured, **When** the API returns image metadata, **Then** `file_url` and `thumbnail_url` point to the API proxy paths (e.g. `/api/v1/images/{id}/file`)
2. **Given** no CDN base URL is configured, **When** a visitor views the library, **Then** thumbnails load via the API proxy with no broken images
3. **Given** a CDN base URL is configured, **When** the application starts, **Then** all image URLs use the CDN domain instead of the proxy paths
---
### Edge Cases
- What happens when the CDN base URL is set but the object does not exist in CDN storage? The browser receives a 404 from the CDN — the API does not re-proxy the content.
- What happens if an image has no thumbnail (thumbnail generation failed)? The `thumbnail_url` field is absent or null; the UI falls back to the full image URL as it does today.
- What happens if the CDN base URL has a trailing slash? The system normalises the URL to avoid double slashes in constructed paths.
---
## Requirements *(mandatory)*
### Functional Requirements
- **FR-001**: The API MUST include a `file_url` field in all image metadata responses, containing the full URL from which the image file can be fetched
- **FR-002**: The API MUST include a `thumbnail_url` field in all image metadata responses when a thumbnail exists, containing the full URL from which the thumbnail can be fetched
- **FR-003**: When a CDN base URL is configured, `file_url` and `thumbnail_url` MUST point to the CDN domain
- **FR-004**: When no CDN base URL is configured, `file_url` and `thumbnail_url` MUST point to the existing API proxy endpoints so local development continues to work without additional setup
- **FR-005**: The existing API proxy endpoints (`/images/{id}/file`, `/images/{id}/thumbnail`) MUST remain functional regardless of whether a CDN base URL is configured
- **FR-006**: The UI MUST use `file_url` and `thumbnail_url` from the API response to render images, rather than constructing proxy URLs client-side
- **FR-007**: The CDN base URL MUST be configurable via environment variable; no value is required in local development
- **FR-008**: A trailing slash in the configured CDN base URL MUST NOT result in double slashes in constructed image URLs
- **FR-009**: When `thumbnail_url` is null, the UI MUST fall back to `file_url` for thumbnail display rather than rendering a broken image
### Key Entities
- **Image metadata response**: Extended to include `file_url` and `thumbnail_url` fields alongside existing fields (`id`, `filename`, `tags`, `width`, `height`, `mime_type`, etc.)
---
## Success Criteria *(mandatory)*
### Measurable Outcomes
- **SC-001**: In production, zero image or thumbnail requests pass through the API server — all are served directly by the CDN
- **SC-002**: Local development requires no additional configuration beyond what is already required — `docker compose up` continues to work with images loading correctly
- **SC-003**: All existing image-related API integration tests continue to pass after the change
- **SC-004**: Image metadata responses include `file_url` and `thumbnail_url` fields for 100% of images that have been successfully stored
---
## Assumptions
- The CDN storage bucket and public domain are already configured and operational before this feature is deployed — this feature only changes how URLs are constructed and served, not how objects are stored
- Object keys in CDN storage are identical to those used in the existing storage backend — no key remapping is needed
- The CDN serves objects publicly without authentication — no signed URL generation is required
- The existing API proxy endpoints are retained as functional fallbacks; the UI stops calling them in production but they are not removed
- Local development uses the existing MinIO-backed proxy and does not require a locally running CDN

View File

@@ -0,0 +1,116 @@
# Tasks: CDN Image Serving
**Input**: Design documents from `specs/014-r2-cdn-serving/`
**Prerequisites**: plan.md ✅, spec.md ✅, research.md ✅, contracts/image-response.md ✅, quickstart.md ✅
**Tests**: Unit tests for URL construction logic; integration tests asserting `file_url` and `thumbnail_url` in all image responses. Tests accompany each implementation task per §5.1.
**Organization**: Phase 1 adds the config value (foundational — blocks everything). Phase 2 implements US1 (CDN URL serving in API + UI consumption). Phase 3 verifies US2 (local dev fallback). Polish runs the full suite and manual end-to-end check.
## Format: `[ID] [P?] [Story] Description`
- **[P]**: Can run in parallel (different files, no dependencies)
- **[Story]**: Which user story this task belongs to
---
## Phase 1: Foundational (Config)
**Goal**: Add `s3_public_base_url` to config and `.env.example`. All US1 and US2 tasks depend on this.
**⚠️ CRITICAL**: No user story work can begin until this phase is complete.
- [X] T001 Add `s3_public_base_url: str | None = None` to the `Settings` class in `api/app/config.py` (after `api_base_url`); add `S3_PUBLIC_BASE_URL=` with comment "# CDN base URL for serving images (e.g. https://cdn.example.com). Leave empty in local dev to use API proxy fallback." to `.env.example` after the `API_BASE_URL` line
**Checkpoint**: Config in place — user story work can begin.
---
## Phase 2: User Story 1 — Images Load Directly from CDN (Priority: P1) 🎯 MVP
**Goal**: API returns `file_url` and `thumbnail_url` in all image responses; UI uses those fields to render images rather than constructing proxy URLs client-side.
**Independent Test**: With `S3_PUBLIC_BASE_URL=https://cdn.reactbin.juggalol.com` set, call `GET /api/v1/images` and confirm each item has `file_url` starting with `https://cdn.reactbin.juggalol.com/` and `thumbnail_url` starting with `https://cdn.reactbin.juggalol.com/` (or null). Open the library page in a browser and confirm image requests go to the CDN domain in the network panel.
- [X] T002 [US1] Write unit tests in `api/tests/unit/test_url_construction.py` covering four cases: (1) CDN base set, image has thumbnail — `file_url` and `thumbnail_url` are CDN URLs; (2) CDN base set, image has no thumbnail — `thumbnail_url` is None; (3) CDN base not set, image has thumbnail — `file_url` is `/api/v1/images/{id}/file` and `thumbnail_url` is `/api/v1/images/{id}/thumbnail`; (4) CDN base not set, no thumbnail — `thumbnail_url` is None. Test the trailing-slash normalisation case (CDN base with trailing slash produces no double-slash). Import and call `_image_to_dict` directly with a mock `Image` object.
- [X] T003 [US1] Update `_image_to_dict` in `api/app/routers/images.py`: add `cdn_base: str | None = None` keyword parameter; compute `_base = cdn_base.rstrip("/") if cdn_base else None`; set `file_url = f"{_base}/{image.storage_key}" if _base else f"/api/v1/images/{image.id}/file"`; set `thumbnail_url = (f"{_base}/{image.thumbnail_key}" if _base else f"/api/v1/images/{image.id}/thumbnail") if image.thumbnail_key else None`; add `"file_url": file_url` and `"thumbnail_url": thumbnail_url` to the returned dict. Run `make test-unit` and confirm T002 tests pass.
- [X] T004 [US1] Update every `_image_to_dict(...)` call site in `api/app/routers/images.py`: at the top of each endpoint function that calls `_image_to_dict`, add `_cdn_base = get_settings().s3_public_base_url` (import `get_settings` is already present); pass `cdn_base=_cdn_base` to every `_image_to_dict` call in that endpoint. Affected endpoints: `upload_image`, `list_images`, `get_image`, `patch_image_tags`. Confirm `get_settings()` is called once per endpoint, not once per image in a loop (for `list_images`, call it before the list comprehension).
- [X] T005 [US1] Update integration tests: in `api/tests/integration/test_upload.py`, add assertions after existing response checks that `"file_url"` is present in the response body and starts with `/api/v1/images/` (since no CDN is configured in test env); add the same assertion for `"thumbnail_url"` in `test_upload_returns_thumbnail_key`; add assertion that `thumbnail_url` is None in the test that expects `thumbnail_key` to be None. Run `make test-integration` and confirm all pass.
- [X] T006 [P] [US1] Update `ui/src/app/services/image.service.ts`: add `file_url: string` and `thumbnail_url: string | null` to the `ImageRecord` interface; remove the `getFileUrl(id: string): string` method; remove the `getThumbnailUrl(id: string): string` method.
- [X] T007 [P] [US1] Update `ui/src/app/library/library.component.ts`: replace `[src]="imageService.getThumbnailUrl(img.id)"` (line 77) with `[src]="img.thumbnail_url ?? img.file_url"` — fall back to `file_url` when thumbnail is absent (FR-009); update `ui/src/app/library/library.component.spec.ts` to add `file_url` and `thumbnail_url` to any mock `ImageRecord` objects and remove any references to `getThumbnailUrl()`.
- [X] T008 [P] [US1] Update `ui/src/app/detail/detail.component.ts`: replace `[src]="imageService.getFileUrl(image.id)"` (line 52) with `[src]="image.file_url"`; update `ui/src/app/detail/detail.component.spec.ts` to add `file_url` and `thumbnail_url` to any mock `ImageRecord` objects and remove any references to `getFileUrl()`.
- [X] T009 [US1] Update `ui/src/app/services/image.service.spec.ts`: add `file_url` and `thumbnail_url` fields to any mock `ImageRecord` objects used in tests; remove any test cases that test `getFileUrl()` or `getThumbnailUrl()` (these methods no longer exist). Run UI tests and confirm they pass.
**Checkpoint**: US1 complete. API returns CDN URLs when configured; UI uses response fields to render images.
---
## Phase 3: User Story 2 — Local Development Works Without CDN (Priority: P2)
**Goal**: Confirm that with no `S3_PUBLIC_BASE_URL` configured, `file_url` and `thumbnail_url` fall back to API proxy paths and images load correctly in local dev.
**Independent Test**: Run `make test-unit && make test-integration` with no `S3_PUBLIC_BASE_URL` set (the default). Confirm all tests pass and that `file_url` values in integration test responses begin with `/api/v1/images/`.
- [X] T010 [US2] Verify US2: run `make test-unit` and confirm the url-construction unit tests for the "no CDN base" case (T002 cases 3 and 4) pass; run `make test-integration` and confirm the updated upload tests (T005) pass — they already assert relative proxy paths since the test environment has no `S3_PUBLIC_BASE_URL`. Confirm `docker compose up` starts cleanly and images load in the browser via the proxy paths with no console errors.
**Checkpoint**: US2 verified. Local development requires no additional configuration.
---
## Phase 4: Polish & Cross-Cutting Concerns
- [X] T011 [P] Run `ruff check api/app/routers/images.py api/app/config.py` and fix any lint issues; run `ruff format --check` and format if needed.
- [X] T012 Run end-to-end verification per `specs/014-r2-cdn-serving/quickstart.md`: in production with `S3_PUBLIC_BASE_URL` set, call `GET /api/v1/images` and confirm `file_url` and `thumbnail_url` begin with `https://cdn.reactbin.juggalol.com/`; open the library page in a browser and confirm image requests in the network panel go to `cdn.reactbin.juggalol.com`, not `/api/`.
---
## Dependencies & Execution Order
- T001 must complete before any other task
- T002 before T003 (tests before implementation — unit test first)
- T003 before T004 (update helper before call sites)
- T004 before T005 (implementation before integration tests)
- T006, T007, T008 can run in parallel after T001 (different files)
- T009 after T006 (spec depends on updated interface)
- T010 after T003T009 (verification requires full implementation)
- T011 after T003T004 (lint the changed files)
- T012 last (manual end-to-end)
### Execution Order Summary
```
Step 1: T001 (foundational: config)
Step 2: T002 (US1: unit tests first)
Step 3: T003 (US1: implement _image_to_dict)
Step 4: T004 ∥ T006 ∥ T007 ∥ T008 (US1: call sites + UI in parallel)
Step 5: T005 ∥ T009 (US1: integration tests + service spec)
Step 6: T010 (US2: verify local dev fallback)
Step 7: T011 (polish: lint)
Step 8: T012 (polish: manual end-to-end)
```
---
## Implementation Strategy
### MVP (US1 only — CDN URLs in API + UI)
1. T001 — config
2. T002T005 — API implementation and tests
3. T006T009 — UI updates
4. **STOP and VALIDATE**: `make test-unit && make test-integration`, check browser network panel
### Incremental Delivery
1. T001T005 (API only) → deploy → verify CDN URLs appear in API responses
2. T006T009 (UI) → deploy → verify browser fetches images from CDN
3. T010 (local dev verification) → confirm fallback intact
4. T011T012 (polish + end-to-end) → ship

View File

@@ -10,7 +10,8 @@ import { routes } from '../app.routes';
const MOCK_IMAGE = {
id: 'img-1', hash: 'abc', filename: 'test.jpg', mime_type: 'image/jpeg',
size_bytes: 100, width: 10, height: 10, storage_key: 'abc',
thumbnail_key: null, created_at: '2026-01-01T00:00:00Z', tags: ['cat', 'funny'],
thumbnail_key: null, file_url: '/api/v1/images/img-1/file', thumbnail_url: null,
created_at: '2026-01-01T00:00:00Z', tags: ['cat', 'funny'],
};
describe('DetailComponent', () => {

View File

@@ -49,7 +49,7 @@ const PLACEHOLDER_SVG = `data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/s
<img
class="full-image"
[src]="imageService.getFileUrl(image.id)"
[src]="image.file_url"
[alt]="image.filename"
(error)="onImgError($event)"
/>

View File

@@ -19,7 +19,7 @@ function makeActivatedRoute(queryParams: Record<string, string> = {}) {
const EMPTY_PAGE = { items: [], total: 0, limit: 50, offset: 0 };
const ONE_IMAGE = {
items: [{ id: '1', filename: 'a.jpg', tags: ['cat'], hash: '', mime_type: 'image/jpeg', size_bytes: 1, width: 1, height: 1, storage_key: '', thumbnail_key: null, created_at: '' }],
items: [{ id: '1', filename: 'a.jpg', tags: ['cat'], hash: '', mime_type: 'image/jpeg', size_bytes: 1, width: 1, height: 1, storage_key: '', thumbnail_key: null, file_url: '/api/v1/images/1/file', thumbnail_url: null, created_at: '' }],
total: 1, limit: 50, offset: 0,
};

View File

@@ -74,7 +74,7 @@ const PLACEHOLDER_SVG = `data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/s
(keydown.enter)="router.navigate(['/images', img.id])"
>
<img
[src]="imageService.getThumbnailUrl(img.id)"
[src]="img.thumbnail_url ?? img.file_url"
[alt]="img.filename"
loading="lazy"
(error)="onImgError($event)"

View File

@@ -12,6 +12,8 @@ export interface ImageRecord {
height: number;
storage_key: string;
thumbnail_key: string | null;
file_url: string;
thumbnail_url: string | null;
created_at: string;
tags: string[];
duplicate?: boolean;
@@ -51,14 +53,6 @@ export class ImageService {
return this.http.get<ImageRecord>(`${this.base}/images/${id}`);
}
getFileUrl(id: string): string {
return `${this.base}/images/${id}/file`;
}
getThumbnailUrl(id: string): string {
return `${this.base}/images/${id}/thumbnail`;
}
updateTags(id: string, tags: string[]): Observable<ImageRecord> {
return this.http.patch<ImageRecord>(`${this.base}/images/${id}/tags`, { tags });
}