feat(flags): serve /api/flags, and keep the public proxy off it
The endpoint and its exposure control ship together on purpose. The moment /api/flags exists, app/api/[...path] forwards it — and the response names every unreleased feature the codebase knows about, along with whether it is on. Publishing that is the opposite of shipping dark. Denied on an exact first-segment match, not a prefix, so /api/flagship does not go down with /api/flags. Next reads the endpoint server-side over the Docker network, which never transits the public proxy. jest.setup.js now guards its browser globals. It runs for every suite, including the one that declares @jest-environment node to exercise the route handler — NextRequest needs Fetch API globals jsdom lacks, and there is no window there to define matchMedia on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
This commit is contained in:
1 parent
7424cef7c6
commit
c30ad1db07
5 files changed
+93
No files matched your search
@@ -35,6 +35,7 @@ from .data_loader import (
|
|||||||
search_schools_typesense,
|
search_schools_typesense,
|
||||||
)
|
)
|
||||||
from .data_loader import get_data_info as get_db_info
|
from .data_loader import get_data_info as get_db_info
|
||||||
|
from . import flags
|
||||||
from .places import build_place_registry
|
from .places import build_place_registry
|
||||||
from .schemas import METRIC_DEFINITIONS, RANKING_COLUMNS, SCHOOL_COLUMNS
|
from .schemas import METRIC_DEFINITIONS, RANKING_COLUMNS, SCHOOL_COLUMNS
|
||||||
from .utils import clean_for_json, convert_to_native
|
from .utils import clean_for_json, convert_to_native
|
||||||
@@ -459,6 +460,7 @@ def validate_postcode(postcode: Optional[str]) -> Optional[str]:
|
|||||||
async def lifespan(app: FastAPI):
|
async def lifespan(app: FastAPI):
|
||||||
"""Application lifespan - startup and shutdown events."""
|
"""Application lifespan - startup and shutdown events."""
|
||||||
global _sitemaps
|
global _sitemaps
|
||||||
|
flags.init()
|
||||||
print("Loading school data from marts...")
|
print("Loading school data from marts...")
|
||||||
df = load_school_data()
|
df = load_school_data()
|
||||||
if df.empty:
|
if df.empty:
|
||||||
@@ -1263,6 +1265,18 @@ async def get_place(request: Request, kind: str, slug: str,
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@app.get("/api/flags")
|
||||||
|
@limiter.limit(f"{settings.rate_limit_per_minute}/minute")
|
||||||
|
async def get_feature_flags(request: Request):
|
||||||
|
"""Every declared flag and its current value.
|
||||||
|
|
||||||
|
Internal only. The Next proxy denies this path, because the response names
|
||||||
|
every unreleased feature the codebase knows about — which is exactly what
|
||||||
|
shipping dark is meant to keep quiet.
|
||||||
|
"""
|
||||||
|
return flags.all_flags()
|
||||||
|
|
||||||
|
|
||||||
@app.get("/api/data-info")
|
@app.get("/api/data-info")
|
||||||
@limiter.limit(f"{settings.rate_limit_per_minute}/minute")
|
@limiter.limit(f"{settings.rate_limit_per_minute}/minute")
|
||||||
async def get_data_info(request: Request):
|
async def get_data_info(request: Request):
|
||||||
|
|||||||
@@ -72,3 +72,24 @@ def test_a_flag_older_than_the_limit_fails_this_test():
|
|||||||
"Remove the flag and the code branches it guards, or move its `added` "
|
"Remove the flag and the code branches it guards, or move its `added` "
|
||||||
"date deliberately."
|
"date deliberately."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _client():
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from backend import app as app_module
|
||||||
|
return TestClient(app_module.app, raise_server_exceptions=False)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_flags_endpoint_lists_every_declared_flag(monkeypatch):
|
||||||
|
monkeypatch.setattr(flags, "_client", None)
|
||||||
|
body = _client().get("/api/flags").json()
|
||||||
|
assert set(body) == set(flags.REGISTRY)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_flags_endpoint_answers_false_when_unleash_is_unreachable(monkeypatch):
|
||||||
|
# The endpoint must still answer. A frontend that cannot read flags renders
|
||||||
|
# everything dark, which is right; one that gets a 500 renders nothing.
|
||||||
|
monkeypatch.setattr(flags, "_client", None)
|
||||||
|
res = _client().get("/api/flags")
|
||||||
|
assert res.status_code == 200
|
||||||
|
assert all(v is False for v in res.json().values())
|
||||||
@@ -0,0 +1,31 @@
|
|||||||
|
/**
|
||||||
|
* The /api/* proxy is public. Anything it forwards is on the internet.
|
||||||
|
*
|
||||||
|
* @jest-environment node
|
||||||
|
*/
|
||||||
|
// The docblock above is load-bearing. jest.config.js sets jsdom globally, and
|
||||||
|
// NextRequest/NextResponse need the Web Fetch API globals that only the node
|
||||||
|
// environment provides — under jsdom this suite fails on import, not on an
|
||||||
|
// assertion.
|
||||||
|
import { NextRequest } from 'next/server';
|
||||||
|
import { GET } from '@/app/api/[...path]/route';
|
||||||
|
|
||||||
|
function request(path: string) {
|
||||||
|
return new NextRequest(`http://localhost:3000/api/${path}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('public API proxy', () => {
|
||||||
|
it('refuses to forward internal-only paths', async () => {
|
||||||
|
// /api/flags names every unreleased feature and its state. Forwarding it
|
||||||
|
// publishes the thing shipping dark exists to keep quiet.
|
||||||
|
const res = await GET(request('flags'), { params: Promise.resolve({ path: ['flags'] }) });
|
||||||
|
expect(res.status).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not deny a path that merely starts with the same letters', async () => {
|
||||||
|
// A prefix match would take /api/flagship down with /api/flags.
|
||||||
|
const res = await GET(
|
||||||
|
request('flagship'), { params: Promise.resolve({ path: ['flagship'] }) });
|
||||||
|
expect(res.status).not.toBe(404);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -26,8 +26,27 @@ function backendBase(): string {
|
|||||||
const STRIPPED_RESPONSE_HEADERS = ['content-encoding', 'content-length', 'transfer-encoding', 'connection'];
|
const STRIPPED_RESPONSE_HEADERS = ['content-encoding', 'content-length', 'transfer-encoding', 'connection'];
|
||||||
const METHODS_WITH_BODY = new Set(['POST', 'PUT', 'PATCH', 'DELETE']);
|
const METHODS_WITH_BODY = new Set(['POST', 'PUT', 'PATCH', 'DELETE']);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* API paths this public proxy must not forward.
|
||||||
|
*
|
||||||
|
* Matched on the first segment, exactly — a prefix match would take
|
||||||
|
* /api/flagship down with /api/flags.
|
||||||
|
*
|
||||||
|
* `flags` is here because GET /api/flags names every unreleased feature the
|
||||||
|
* codebase knows about, along with whether it is on. Publishing that defeats
|
||||||
|
* the point of shipping dark. Next reads it server-side via FASTAPI_URL, on
|
||||||
|
* the Docker network, which never transits this route.
|
||||||
|
*
|
||||||
|
* Anything else internal-only belongs here too.
|
||||||
|
*/
|
||||||
|
const INTERNAL_ONLY_SEGMENTS = new Set(['flags']);
|
||||||
|
|
||||||
async function handler(req: NextRequest, ctx: { params: Promise<{ path: string[] }> }) {
|
async function handler(req: NextRequest, ctx: { params: Promise<{ path: string[] }> }) {
|
||||||
const { path } = await ctx.params;
|
const { path } = await ctx.params;
|
||||||
|
if (INTERNAL_ONLY_SEGMENTS.has(path[0])) {
|
||||||
|
return NextResponse.json({ detail: 'Not Found' }, { status: 404 });
|
||||||
|
}
|
||||||
|
|
||||||
const target = `${backendBase()}/${path.join('/')}${req.nextUrl.search}`;
|
const target = `${backendBase()}/${path.join('/')}${req.nextUrl.search}`;
|
||||||
|
|
||||||
const headers = new Headers(req.headers);
|
const headers = new Headers(req.headers);
|
||||||
|
|||||||
@@ -12,6 +12,12 @@ jest.mock('next/navigation', () => ({
|
|||||||
useSearchParams: () => new URLSearchParams(),
|
useSearchParams: () => new URLSearchParams(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
// Everything below this line is browser furniture, and this file runs for
|
||||||
|
// every suite — including the ones that declare `@jest-environment node` to
|
||||||
|
// test route handlers, where NextRequest needs Fetch API globals jsdom does
|
||||||
|
// not provide. There is no `window` there, so guard rather than assume one.
|
||||||
|
if (typeof window !== 'undefined') {
|
||||||
|
|
||||||
// Mock window.matchMedia
|
// Mock window.matchMedia
|
||||||
Object.defineProperty(window, 'matchMedia', {
|
Object.defineProperty(window, 'matchMedia', {
|
||||||
writable: true,
|
writable: true,
|
||||||
@@ -52,3 +58,5 @@ const localStorageMock = {
|
|||||||
clear: jest.fn(),
|
clear: jest.fn(),
|
||||||
};
|
};
|
||||||
global.localStorage = localStorageMock;
|
global.localStorage = localStorageMock;
|
||||||
|
|
||||||
|
} // end: browser-only globals
|
||||||
Reference in new issue
Block a user