fix: scorecard and codeql on push to protected branch

.claude/skills/test/SKILL.md

@@ -0,0 +1,62 @@
+---
+name: test
+description: Write tests for a change following red-green-refactor TDD, testing behavior through this service's public HTTP interface rather than internals. Use during implement, before considering a ticket done.
+---
+
+Write the test first. Watch it fail for the right reason, a missing
+behavior or a real bug, not a typo or an import error, then write the
+minimum code to make it pass, then refactor with the test as a safety
+net. Skipping straight to the implementation and testing it afterward is
+not TDD, even if a test file ends up next to the code.
+
+Test through `/records`, `/records/{ave_id}`, `/records/{ave_id}/mitigation`,
+and `/search` with FastAPI's `TestClient`, the same way a real caller
+would, rather than importing and calling `_refresh_cache()` or
+`_ensure_fresh()` directly. A test that reaches into an internal function
+locks in that function's shape and breaks on refactors that don't change
+any actual behavior; a test that hits the HTTP interface survives them.
+
+Never let a test touch the real network. `main.py` makes exactly one
+outbound call, to `aveproject/ave`'s raw content; monkeypatch
+`main._refresh_cache` to populate `main._cache` with a small, fake
+fixture of one or two records instead of hitting GitHub. A test suite
+that depends on GitHub being up, or on `aveproject/ave`'s current record
+content, is not a test suite, it is a flaky integration check wearing a
+test's clothes.
+
+Reset shared state before every test: `main._cache` and `main.limiter`'s
+storage are both module-level singletons that persist across the whole
+test session unless something clears them. An autouse fixture that
+resets both before each test is not optional scaffolding, it is what
+makes the rest of the suite trustworthy; without it, test order starts
+to matter and failures stop being reproducible.
+
+One behavior per test, named as a sentence that describes it,
+`test_get_mitigation_404s_when_record_has_no_mitigation`, not
+`test_mitigation` or `test_2`. If a test's name can't describe what it
+checks in one sentence, it's checking more than one thing and should be
+split.
+
+Cover the behaviors that are actually load-bearing for this repo, not
+just the happy path:
+- Every route's 404 case, worded well enough that whatever it says is
+  actually useful to whoever hits it.
+- `/mitigation` returns the neutral `mitigation` object, and nothing
+  else, and 404s cleanly when a record has none.
+- `/search` matches across all four documented fields (`title`,
+  `description`, `attack_class`, `behavioral_fingerprint`),
+  case-insensitively, and returns an empty list rather than an error
+  for no matches.
+- The resilience pattern from `CLAUDE.md`: a refresh failure with a
+  populated cache serves the stale data (200, not 500); a refresh
+  failure with an empty cache is a real failure (500, it has to
+  surface); startup logs and continues on a failed initial fetch rather
+  than crash-looping.
+- Every route carries its `@limiter.limit(...)` decorator; a route
+  discovered without one while writing its tests is a bug in that
+  route, not a gap in the test.
+
+Run `pytest` before considering a ticket done, same bar as
+`python3 -m py_compile main.py` in `implement`'s own checklist: a red
+suite caught in code review that a local run would have caught is
+wasted review time.

.github/workflows/scorecard.yml

@@ -5,7 +5,7 @@ on:
   schedule:
     - cron: "30 1 * * 6"
   push:
-    branches: ["main", "develop"]
+    branches: ["main"]
 
 permissions: read-all
 

ARCHITECTURE.md

@@ -52,11 +52,22 @@ when and why this was added.
 ## ADR: hosting platform
 
 Considered Vercel (rejected: serverless-only Python, no persistent
-in-memory state, incompatible with this service's cache design). Chose
-Render as primary (zero card on file, real cost is a slower cold start
-after idle) over Cloud Run (technically better fit for the persistent-
-container model, but requires a card on file even though usage should
-stay within the free tier). See `README.md` deployment section for the
-current live choice; check there before assuming this ADR reflects where
-it's actually running today, ADRs record reasoning at decision time, they
-don't self-update.
+in-memory state, incompatible with this service's cache design).
+Originally chose Render as primary (zero card on file, real cost was a
+slower cold start after idle) over Cloud Run (technically better fit
+for the persistent-container model, but requires a card on file even
+though usage should stay within the free tier).
+
+Revisited for the first production deployment and switched to Cloud
+Run: the card-on-file requirement was accepted deliberately, with a $1
+budget alert as the concrete mitigation (a service expected to cost $0
+means any alert firing at all is the signal, not a soft limit), in
+exchange for the persistent-container model actually matching this
+service's in-memory cache design instead of working around Render's
+sleep-and-wake cycle. `--min-instances 0` keeps it free at idle the
+same way Render's sleep did, just without the cold-start latency on
+wake.
+
+See `README.md`'s deployment section for the current live choice; check
+there before assuming this ADR reflects where it's actually running
+today, ADRs record reasoning at decision time, they don't self-update.

CHANGELOG.md

@@ -3,6 +3,31 @@
 All notable changes to this project are documented here. Format loosely
 follows [Keep a Changelog](https://keepachangelog.com/).
 
+## [Unreleased]
+
+### Changed
+
+- Hosting switched from the planned Render deployment to Google Cloud
+  Run for the first production deployment (`--min-instances 0`,
+  `--max-instances 3`, `us-central1`), accepting the card-on-file
+  tradeoff in exchange for a persistent-container model that matches
+  this service's in-memory cache design. A $1 budget alert is the
+  concrete mitigation. See `ARCHITECTURE.md`'s ADR section for the full
+  reasoning.
+
+### Fixed
+
+- `Dockerfile` only copied `main.py`, never `constants.py`, so the
+  built image crashed on startup with `ModuleNotFoundError` before
+  `uvicorn` ever bound to the port. Caught by the first real deploy
+  attempt; `python3 -m py_compile main.py` never catches this since it
+  runs against the full checked-out repo, not the trimmed set of files
+  the Dockerfile actually copies into the image.
+- The fix for the above (`COPY main.py constants.py .`) built fine
+  locally under BuildKit but failed on Cloud Build's classic builder,
+  which enforces the Dockerfile spec strictly: a multi-source `COPY`'s
+  destination must end with `/`.
+
 ## [1.0.0] - initial release
 
 ### Added

Dockerfile

@@ -2,7 +2,7 @@ FROM python:3.14-slim
 WORKDIR /app
 COPY requirements.txt .
 RUN pip install --no-cache-dir -r requirements.txt
-COPY main.py .
+COPY main.py constants.py ./
 ENV PORT=8080
 EXPOSE 8080
 CMD exec uvicorn main:app --host 0.0.0.0 --port ${PORT}

README.md

@@ -45,9 +45,9 @@ minutes thereafter. This repo holds no copy of the record data itself;
 
 ## Deployment
 
-Deployed on Render. See `ARCHITECTURE.md`'s ADR section for why Render was
-chosen over Cloud Run and Vercel, and what would have to change to revisit
-that.
+Deployed on Google Cloud Run (`--min-instances 0`, scales to zero at
+idle). See `ARCHITECTURE.md`'s ADR section for the full reasoning,
+including why Render was the original choice and what changed.
 
 ## Contributing
 

requirements-dev.txt

@@ -0,0 +1,2 @@
+-r requirements.txt
+pytest==8.3.3

tests/conftest.py

@@ -0,0 +1,49 @@
+import time
+
+import pytest
+from fastapi.testclient import TestClient
+
+import main
+
+FAKE_RECORDS = [
+    {
+        "ave_id": "AVE-0001",
+        "title": "Prompt injection via tool output",
+        "description": "An agent trusts unsanitized tool output as instructions.",
+        "attack_class": "instruction-injection",
+        "behavioral_fingerprint": "unexpected-tool-call-sequence",
+        "mitigation": {
+            "summary": "Treat tool output as untrusted data, not instructions."
+        },
+    },
+    {
+        "ave_id": "AVE-0002",
+        "title": "Excessive agency",
+        "description": "An agent takes an irreversible action without confirmation.",
+        "attack_class": "privilege-escalation",
+        "behavioral_fingerprint": "unconfirmed destructive action",
+        "mitigation": None,
+    },
+]
+
+
+@pytest.fixture(autouse=True)
+def isolated_service_state(monkeypatch):
+    """Every test starts from known cache and rate-limit state, and no test
+    ever reaches the real network."""
+
+    async def fake_refresh_cache() -> None:
+        main._cache["records"] = FAKE_RECORDS
+        main._cache["fetched_at"] = time.time()
+
+    monkeypatch.setattr(main, "_refresh_cache", fake_refresh_cache)
+    main._cache["records"] = []
+    main._cache["fetched_at"] = 0.0
+    main.limiter.reset()
+    yield
+
+
+@pytest.fixture
+def client():
+    with TestClient(main.app) as test_client:
+        yield test_client

tests/test_main.py

@@ -0,0 +1,134 @@
+import time
+
+from fastapi.testclient import TestClient
+
+import main
+from conftest import FAKE_RECORDS
+
+
+def test_index_reports_service_metadata(client):
+    response = client.get("/")
+
+    assert response.status_code == 200
+    body = response.json()
+    assert body["name"] == "AVE Reference API"
+    assert body["record_count"] == len(FAKE_RECORDS)
+    assert "cache_age_seconds" in body
+
+
+def test_list_records_returns_all_cached_records(client):
+    response = client.get("/records")
+
+    assert response.status_code == 200
+    assert len(response.json()) == len(FAKE_RECORDS)
+
+
+def test_get_record_returns_matching_record(client):
+    response = client.get("/records/AVE-0001")
+
+    assert response.status_code == 200
+    assert response.json()["ave_id"] == "AVE-0001"
+
+
+def test_get_record_404s_for_unknown_id(client):
+    response = client.get("/records/does-not-exist")
+
+    assert response.status_code == 404
+    assert "does-not-exist" in response.json()["detail"]
+
+
+def test_get_mitigation_returns_only_the_mitigation_object(client):
+    response = client.get("/records/AVE-0001/mitigation")
+
+    assert response.status_code == 200
+    assert response.json() == {
+        "summary": "Treat tool output as untrusted data, not instructions."
+    }
+
+
+def test_get_mitigation_404s_when_record_has_no_mitigation(client):
+    response = client.get("/records/AVE-0002/mitigation")
+
+    assert response.status_code == 404
+
+
+def test_get_mitigation_404s_for_unknown_id(client):
+    response = client.get("/records/does-not-exist/mitigation")
+
+    assert response.status_code == 404
+
+
+def test_search_matches_title_case_insensitively(client):
+    response = client.get("/search", params={"q": "PROMPT"})
+
+    assert response.status_code == 200
+    assert [r["ave_id"] for r in response.json()] == ["AVE-0001"]
+
+
+def test_search_matches_across_attack_class_and_behavioral_fingerprint(client):
+    response = client.get("/search", params={"q": "unconfirmed"})
+
+    assert [r["ave_id"] for r in response.json()] == ["AVE-0002"]
+
+
+def test_search_returns_empty_list_for_no_match(client):
+    response = client.get("/search", params={"q": "nothing-matches-this"})
+
+    assert response.status_code == 200
+    assert response.json() == []
+
+
+def test_every_route_is_rate_limited(client):
+    for _ in range(60):
+        response = client.get("/records")
+        assert response.status_code == 200
+
+    response = client.get("/records")
+
+    assert response.status_code == 429
+
+
+def test_startup_does_not_crash_when_initial_fetch_fails(monkeypatch):
+    async def failing_refresh() -> None:
+        raise RuntimeError("simulated network failure")
+
+    monkeypatch.setattr(main, "_refresh_cache", failing_refresh)
+
+    with TestClient(main.app) as test_client:
+        response = test_client.get("/")
+
+    assert response.status_code == 200
+    assert response.json()["record_count"] == 0
+
+
+def test_refresh_failure_serves_stale_cache_instead_of_failing(client, monkeypatch):
+    # `client` already has a populated, fresh cache via the autouse fixture;
+    # push it past REFRESH_INTERVAL_SECONDS so the next request tries a
+    # refresh, then make that refresh fail.
+    main._cache["fetched_at"] = time.time() - main.REFRESH_INTERVAL_SECONDS - 1
+
+    async def failing_refresh() -> None:
+        raise RuntimeError("simulated transient network failure")
+
+    monkeypatch.setattr(main, "_refresh_cache", failing_refresh)
+
+    response = client.get("/records")
+
+    assert response.status_code == 200
+    assert len(response.json()) == len(FAKE_RECORDS)
+
+
+def test_refresh_failure_is_a_real_failure_when_cache_was_never_populated(
+    monkeypatch,
+):
+    async def failing_refresh() -> None:
+        raise RuntimeError("simulated network failure, never succeeded")
+
+    monkeypatch.setattr(main, "_refresh_cache", failing_refresh)
+    main._cache["records"] = []
+    main._cache["fetched_at"] = 0.0
+
+    with TestClient(main.app, raise_server_exceptions=False) as test_client:
+        response = test_client.get("/records")
+
+    assert response.status_code == 500