From 80baacf0f41e8076c699fa22cdd315f7c52d26de Mon Sep 17 00:00:00 2001 From: precy41 Date: Sun, 26 Jul 2026 11:23:21 +0000 Subject: [PATCH] feat(backend,ai,onchain): resolve #249 and #233 in one PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #249 — Shared error-code taxonomy * docs/errors.yaml: single source of truth for every error code we emit across backend and AI service. * app/backend/src/common/errors/codes.ts: TypeScript enum + meta table + codeForHttpStatus/httpStatusForCode helpers. * app/backend/src/common/errors/codes.spec.ts: parity test that walks docs/errors.yaml and asserts the TS binding matches every entry, plus a first-declared-wins lookup lock. * app/ai-service/schemas/codes.py: Python mirror of the TS binding (enum + dataclass meta + identical reverse-lookup semantics). * app/ai-service/tests/test_codes.py: parity test mirroring codes.spec.ts. Both YAML parsers are byte-for-byte equivalent in shape and produce the same set of entries for docs/errors.yaml. * app/ai-service/main.py: HTTPException, validation, body-size, and fallback exception handlers now route through ErrorCode..value / code_for_http_status(...) instead of ad-hoc literal strings. * app/backend/src/common/filters/http-exception.filter.ts: doc comment pinning the wire-format contract (code: number, the existing public envelope shape) so a future PR cannot accidentally break it. #233 — Cap allowed_tokens at MAX_ALLOWED_TOKENS * app/onchain/contracts/aid_escrow/src/lib.rs: - new pub const MAX_ALLOWED_TOKENS: u32 = 32 (re-exported for tests). - new Error::TooManyAllowedTokens = 23 variant. - set_config rejects oversized allowlists with Error::TooManyAllowedTokens BEFORE the per-token validate_token loop, so a malicious admin can neither bloat wasm storage nor burn host cost. * app/onchain/contracts/aid_escrow/tests/aid_escrow_tests.rs: - set_config_rejects_too_many_allowed_tokens (vec of length MAX + 1, asserts TooManyAllowedTokens) - set_config_passes_cap_check_at_max_boundary (vec of length MAX made of dummy AidEscrow writes to lmts, asserts InvalidToken rather than TooManyAllowedTokens to prove the cap is exclusive). Both tests reference aid_escrow::MAX_ALLOWED_TOKENS directly so future tweaks to the cap cannot silently drift out of spec. Both contracts public wire formats are unchanged. Closes #249 Closes #233 --- app/ai-service/main.py | 17 +- app/ai-service/schemas/codes.py | 184 ++++++++++++ app/ai-service/tests/test_codes.py | 278 ++++++++++++++++++ app/backend/src/common/errors/codes.spec.ts | 220 ++++++++++++++ app/backend/src/common/errors/codes.ts | 194 ++++++++++++ .../common/filters/http-exception.filter.ts | 8 + app/onchain/contracts/aid_escrow/src/lib.rs | 24 ++ .../aid_escrow/tests/aid_escrow_tests.rs | 67 +++++ docs/errors.yaml | 92 ++++++ 9 files changed, 1079 insertions(+), 5 deletions(-) create mode 100644 app/ai-service/schemas/codes.py create mode 100644 app/ai-service/tests/test_codes.py create mode 100644 app/backend/src/common/errors/codes.spec.ts create mode 100644 app/backend/src/common/errors/codes.ts create mode 100644 docs/errors.yaml diff --git a/app/ai-service/main.py b/app/ai-service/main.py index e20f8bd5..fab0de8a 100644 --- a/app/ai-service/main.py +++ b/app/ai-service/main.py @@ -17,6 +17,7 @@ from fastapi.responses import JSONResponse, RedirectResponse, Response from exceptions import AIServiceError from schemas.errors import ErrorDetail, ErrorEnvelope +from schemas.codes import ErrorCode, code_for_http_status # Issue #249 import time import metrics import email.utils @@ -348,7 +349,7 @@ async def _send_413(self, send, observed: int, reason: str, limit: Optional[int] envelope = ErrorEnvelope( error=ErrorDetail( - code="PAYLOAD_TOO_LARGE", + code=ErrorCode.PAYLOAD_TOO_LARGE.value, message=msg, ) ).model_dump() @@ -376,7 +377,7 @@ async def _send_400_mismatch(self, send, declared: int, observed: int): ) envelope = ErrorEnvelope( error=ErrorDetail( - code="CODE_BODY_LENGTH_MISMATCH", + code=ErrorCode.CODE_BODY_LENGTH_MISMATCH.value, message=msg, ) ).model_dump() @@ -1036,7 +1037,11 @@ async def http_exception_handler(request, exc: HTTPException): return JSONResponse( status_code=exc.status_code, content=ErrorEnvelope( - error=ErrorDetail(code=f"HTTP_{exc.status_code}", message=str(exc.detail)) + # Issue #249 — use the shared taxonomy. ``code_for_http_status`` + # returns the canonical string ID from docs/errors.yaml. For + # unknown statuses it falls back to ``HTTP_{n}`` so legacy + # behaviour is preserved. + error=ErrorDetail(code=code_for_http_status(exc.status_code), message=str(exc.detail)) ).model_dump(), ) @@ -1053,7 +1058,8 @@ async def validation_exception_handler(request, exc: RequestValidationError): status_code=422, content=ErrorEnvelope( error=ErrorDetail( - code="VALIDATION_ERROR", + # Issue #249 — use the shared taxonomy. + code=ErrorCode.VALIDATION_ERROR.value, message="Request validation failed", details=exc.errors(), ) @@ -1078,7 +1084,8 @@ async def general_exception_handler(request, exc: Exception): return JSONResponse( status_code=500, content=ErrorEnvelope( - error=ErrorDetail(code="INTERNAL_SERVER_ERROR", message="Internal server error") + # Issue #249 — use the shared taxonomy. + error=ErrorDetail(code=ErrorCode.INTERNAL_SERVER_ERROR.value, message="Internal server error") ).model_dump(), ) diff --git a/app/ai-service/schemas/codes.py b/app/ai-service/schemas/codes.py new file mode 100644 index 00000000..eb572bc6 --- /dev/null +++ b/app/ai-service/schemas/codes.py @@ -0,0 +1,184 @@ +""" +app/ai-service/schemas/codes.py + +Issue #249 — Shared error-code taxonomy consumed by both backend +(NestJS, ``app/backend``) and AI service (FastAPI, ``app/ai-service``). + +This file is the Python binding for ``docs/errors.yaml``, the single +source of truth. The TypeScript binding lives in +``app/backend/src/common/errors/codes.ts``. Parity between the two +bindings is checked automatically by: + + * ``app/backend/src/common/errors/codes.spec.ts`` (backend unit test) + * ``app/ai-service/tests/test_codes.py`` (AI service unit test) + +Both tests load ``docs/errors.yaml``, walk every entry, and assert that: + + 1. The string ``code`` from YAML matches ``ErrorCode..value`` here. + 2. The numeric ``http_status`` from YAML matches ``ErrorCodeMeta.http_status``. + 3. The ``description`` matches the meta table. + +If you change this file you MUST update ``docs/errors.yaml`` AND +``app/backend/src/common/errors/codes.ts`` in the same change. The parity +tests will fail otherwise, which is the whole point of the issue. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from enum import Enum +from typing import Optional + + +class ErrorCode(str, Enum): + """Stable string codes emitted by the API surface. + + String values are what the wire format publishes. Mirroring + ``ErrorCode`` in ``app/backend/src/common/errors/codes.ts`` exactly; + parity is enforced by ``tests/test_codes.py``. + """ + + CODE_BODY_LENGTH_MISMATCH = "CODE_BODY_LENGTH_MISMATCH" + HTTP_400 = "HTTP_400" + HTTP_401 = "HTTP_401" + HTTP_403 = "HTTP_403" + HTTP_404 = "HTTP_404" + HTTP_409 = "HTTP_409" + HTTP_413 = "HTTP_413" + HTTP_422 = "HTTP_422" + HTTP_500 = "HTTP_500" + HTTP_502 = "HTTP_502" + HTTP_503 = "HTTP_503" + INTERNAL_SERVER_ERROR = "INTERNAL_SERVER_ERROR" + PAYLOAD_TOO_LARGE = "PAYLOAD_TOO_LARGE" + VALIDATION_ERROR = "VALIDATION_ERROR" + AI_SERVICE_ERROR = "AI_SERVICE_ERROR" + AI_TIMEOUT = "AI_TIMEOUT" + + +@dataclass(frozen=True) +class ErrorCodeMeta: + """Per-code metadata derived from ``docs/errors.yaml``. + + Indexed by ``ErrorCode`` so consumers can look up HTTP status and + description in O(1). Every entry here MUST be a 1:1 with + ``ErrorCode`` and ``docs/errors.yaml``; ``tests/test_codes.py`` fails + the build if they are not. + """ + + code: str + http_status: int + description: str + + +# Metadata table — keys here MUST exist in ``ErrorCode``; the parity +# test enforces this. +ERROR_CODE_META: dict[ErrorCode, ErrorCodeMeta] = { + ErrorCode.HTTP_400: ErrorCodeMeta( + code="HTTP_400", + http_status=400, + description="Bad request — the request was malformed or contained invalid parameters.", + ), + ErrorCode.HTTP_401: ErrorCodeMeta( + code="HTTP_401", + http_status=401, + description="Unauthorized — authentication is required to access this resource.", + ), + ErrorCode.HTTP_403: ErrorCodeMeta( + code="HTTP_403", + http_status=403, + description="Forbidden — the caller is authenticated but lacks permission.", + ), + ErrorCode.HTTP_404: ErrorCodeMeta( + code="HTTP_404", + http_status=404, + description="Not found — the requested resource does not exist.", + ), + ErrorCode.HTTP_409: ErrorCodeMeta( + code="HTTP_409", + http_status=409, + description="Conflict — the request conflicts with the current resource state.", + ), + ErrorCode.HTTP_413: ErrorCodeMeta( + code="HTTP_413", + http_status=413, + description="Payload too large — the request body exceeds the configured limit.", + ), + ErrorCode.HTTP_422: ErrorCodeMeta( + code="HTTP_422", + http_status=422, + description="Unprocessable entity — request payload failed schema validation.", + ), + ErrorCode.HTTP_500: ErrorCodeMeta( + code="HTTP_500", + http_status=500, + description="Internal server error — an unexpected exception escaped the handler.", + ), + ErrorCode.HTTP_502: ErrorCodeMeta( + code="HTTP_502", + http_status=502, + description="Bad gateway — an upstream/downstream dependency failed.", + ), + ErrorCode.HTTP_503: ErrorCodeMeta( + code="HTTP_503", + http_status=503, + description="Service unavailable — temporary degradation; retry with backoff.", + ), + ErrorCode.VALIDATION_ERROR: ErrorCodeMeta( + code="VALIDATION_ERROR", + http_status=422, + description="Validation failed — request payload does not match the expected schema.", + ), + ErrorCode.AI_SERVICE_ERROR: ErrorCodeMeta( + code="AI_SERVICE_ERROR", + http_status=502, + description="AI service error — the upstream LLM/OCR provider failed to respond.", + ), + ErrorCode.AI_TIMEOUT: ErrorCodeMeta( + code="AI_TIMEOUT", + http_status=502, + description="AI service timeout — the upstream LLM exceeded its time budget.", + ), + ErrorCode.PAYLOAD_TOO_LARGE: ErrorCodeMeta( + code="PAYLOAD_TOO_LARGE", + http_status=413, + description="Payload too large — the request body exceeded the configured size limit.", + ), + ErrorCode.CODE_BODY_LENGTH_MISMATCH: ErrorCodeMeta( + code="CODE_BODY_LENGTH_MISMATCH", + http_status=400, + description="Body length mismatch — streamed bytes exceeded the declared Content-Length.", + ), + ErrorCode.INTERNAL_SERVER_ERROR: ErrorCodeMeta( + code="INTERNAL_SERVER_ERROR", + http_status=500, + description="Internal server error — generic catch-all for unhandled exceptions.", + ), +} + + +def code_for_http_status(status: int) -> str: + """Reverse lookup: HTTP status → stable string code. + + Mirrors ``codeForHttpStatus`` in + ``app/backend/src/common/errors/codes.ts``. If a status maps to + multiple codes the FIRST one declared in ``ERROR_CODE_META`` wins + (preserves insertion order); ``tests/test_codes.py`` asserts this + matches the lookup in the TS module. + """ + for meta in ERROR_CODE_META.values(): + if meta.http_status == status: + return meta.code + return f"HTTP_{status}" + + +def http_status_for_code(code: str) -> Optional[int]: + """Reverse lookup: stable string code → HTTP status. + + Mirrors ``httpStatusForCode`` in + ``app/backend/src/common/errors/codes.ts``. + """ + for code_value, meta in ERROR_CODE_META.items(): + if code_value.value == code or meta.code == code: + return meta.http_status + return None diff --git a/app/ai-service/tests/test_codes.py b/app/ai-service/tests/test_codes.py new file mode 100644 index 00000000..5bde7bce --- /dev/null +++ b/app/ai-service/tests/test_codes.py @@ -0,0 +1,278 @@ +""" +app/ai-service/tests/test_codes.py + +Issue #249 parity test for the AI service: verifies that the Python +binding in ``schemas/codes.py`` is consistent with ``docs/errors.yaml`` +(the single source of truth) AND with the TypeScript binding in +``app/backend/src/common/errors/codes.ts``. + +Mirrors ``app/backend/src/common/errors/codes.spec.ts`` so any drift +breaks BOTH builds simultaneously rather than silently regressing in +only one repo. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path +from typing import List + +import pytest + +_HERE = Path(__file__).resolve().parent +_REPO_ROOT = _HERE.parents[2] # tests/ -> ai-service/ -> app/ -> repo root + +# --------------------------------------------------------------------------- +# pytest discovery — when pytest is invoked from any cwd, ai-service/ +# must be on sys.path so `from schemas.codes import ...` resolves. +# Existing tests like tests/test_error_envelope.py rely on the project's +# pytest config pinning cwd to app/ai-service, but we add the directory +# defensively below so the parity test also runs under alternative +# runners (pytest --rootdir repo, IDE test runners, etc.). +# MUST come BEFORE the `from schemas.codes import ...` line below. +# --------------------------------------------------------------------------- +_AI_SERVICE_ROOT = _HERE.parents[1] +if str(_AI_SERVICE_ROOT) not in sys.path: + sys.path.insert(0, str(_AI_SERVICE_ROOT)) + +from schemas.codes import ( # noqa: E402 (intentional sys.path tweak above) + ERROR_CODE_META, + ErrorCode, + code_for_http_status, + http_status_for_code, +) + +YAML_PATH = _REPO_ROOT / "docs" / "errors.yaml" + + +class _YamlEntry(dict): + """Tiny dotted-access shim so tests read ``entry.code`` cleanly.""" + + def __getattr__(self, key): + try: + return self[key] + except KeyError as e: + raise AttributeError(key) from e + + +def _coerce_value(key: str, raw: str): + """Cast a YAML scalar to the right Python type. + + Keeps the parser small: known numeric shape (``httpStatus``) is int, + everything else stays str. Adding more types means extending the + switch. + """ + if key == "httpStatus": + return int(raw) + return raw + + +def _load_yaml() -> List[_YamlEntry]: + """Minimal line-based YAML reader for docs/errors.yaml. + + The schema is intentionally trivial (a flat list of ``- key: value`` + blocks under ``codes:``) so we don't pull in PyYAML just for parity + testing. Both the TS and Python parity tests share this exact + parser shape so any divergence catches the eye. + + NOTE — when changing this parser, KEEP IT IN LOCK-STEP with + ``loadYaml()`` in ``app/backend/src/common/errors/codes.spec.ts``. + The two parsers MUST interpret the same lines the same way or the + parity suite will pass or fail asymmetrically across the two repos. + + KNOWN LIMITATIONS (intentional, see also the TS parser): + * ``#``-prefixed comments and trailing comments are stripped. + * Multi-line block scalars (``|`` / ``>``) are NOT supported. + Every description in docs/errors.yaml is a single line ending + in a period. If a future description contains a literal ``:`` + the regex below will truncate at the first ``:``; the parity + test will catch such a regression because both parsers use + the identical regex. + """ + if not YAML_PATH.exists(): + pytest.fail( + f"Shared error-code taxonomy not found at {YAML_PATH}. " + "Issue #249 requires docs/errors.yaml as the single source of truth." + ) + raw = YAML_PATH.read_text(encoding="utf-8").splitlines() + + codes: List[_YamlEntry] = [] + in_codes = False + current: _YamlEntry | None = None + for line in raw: + # Strip leading + trailing whitespace AND drop comments so + # ` - code: HTTP_500` (list marker indented two spaces) is + # detected by the simple `startswith("- ")` test below, and so + # `# ---- HTTP status codes ---` separators are skipped cleanly. + hash_idx = line.find("#") + line_no_comment = line[:hash_idx] if hash_idx >= 0 else line + stripped = line_no_comment.strip() + if stripped == "": + continue + if stripped.startswith("version:"): + continue + if stripped == "codes:": + in_codes = True + continue + if not in_codes: + continue + if stripped.startswith("- "): + # Start of a new entry: the rest of the line is a `key: value` + # pair whose KEY is the entry's first property. We DO NOT + # assume the first line is always `- code: …` — a future YAML + # could start with `- name: …` or similar. The remainder is + # parsed with the same key:value regex used for indented lines. + if current is not None: + codes.append(current) + tail = stripped[2:] # e.g. ``code: HTTP_400`` + m_entry = re.match(r"^([a-zA-Z_]+):\s*(.*)$", tail) + if m_entry: + ek, ev = m_entry.group(1), m_entry.group(2).strip().strip('"').strip("'") + current = _YamlEntry({ek: _coerce_value(ek, ev)}) + else: + # Malformed entry line; drop `current` so subsequent + # key/value lines don't attach to garbage. The previous + # entry (if any) was already appended above. + current = None + continue + if current is None: + continue + m = re.match(r"^([a-zA-Z_]+):\s*(.*)$", stripped) + if not m: + continue + key, value = m.group(1), m.group(2).strip().strip('"').strip("'") + current[key] = _coerce_value(key, value) + if current is not None: + codes.append(current) + return codes + + +@pytest.fixture(scope="module") +def yaml_codes() -> List[_YamlEntry]: + return _load_yaml() + + +# --------------------------------------------------------------------------- +# pytest discovery `sys.path` tweak lives at the TOP of this module +# (above the `from schemas.codes import ...` line) so the import works +# even when pytest is invoked from the repo root, not from app/ai-service. +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def yaml_codes() -> List[_YamlEntry]: + return _load_yaml() + + +def test_yaml_loaded(yaml_codes): + assert len(yaml_codes) > 0, "docs/errors.yaml must declare at least one code" + + +def test_every_yaml_entry_in_python_module(yaml_codes): + """Every YAML entry has a matching ErrorCode + meta entry in Python.""" + for entry in yaml_codes: + code = entry["code"] + try: + enum_value = ErrorCode(code) + except ValueError as e: + pytest.fail( + f"docs/errors.yaml declares code {code!r} which is missing " + f"from ErrorCode in app/ai-service/schemas/codes.py: {e}" + ) + + meta = ERROR_CODE_META[enum_value] + assert meta.code == code + assert meta.http_status == entry["httpStatus"], ( + f"docs/errors.yaml lists {code} with httpStatus=" + f"{entry['httpStatus']!r} but the Python meta has {meta.http_status!r}" + ) + assert meta.description == entry["description"], ( + f"docs/errors.yaml description for {code} is out of sync with " + f"schemas/codes.py ERROR_CODE_META" + ) + + +def test_every_python_member_in_yaml(yaml_codes): + """No orphan enum members — every Python enum value is documented in YAML.""" + yaml_codes_set = {e["code"] for e in yaml_codes} + for value in ErrorCode: + assert value.value in yaml_codes_set, ( + f"ErrorCode.{value.name} = {value.value!r} is in " + f"app/ai-service/schemas/codes.py but not in docs/errors.yaml" + ) + + +def test_every_meta_key_in_enum(): + """No orphan meta entries — every meta key is a valid ErrorCode.""" + enum_values = set(ErrorCode) + for key in ERROR_CODE_META.keys(): + assert key in enum_values, ( + f"ERROR_CODE_META contains key {key!r} which is missing " + f"from ErrorCode enum" + ) + + +@pytest.mark.parametrize( + "http_status,expected_code", + [ + (400, "HTTP_400"), + (401, "HTTP_401"), + (404, "HTTP_404"), + (422, "HTTP_422"), + (500, "HTTP_500"), + (502, "HTTP_502"), + (503, "HTTP_503"), + ], +) +def test_code_for_http_status_known(http_status, expected_code): + assert code_for_http_status(http_status) == expected_code + + +def test_code_for_http_status_unknown_falls_back(): + assert code_for_http_status(418) == "HTTP_418" + + +@pytest.mark.parametrize( + "code,expected_status", + [ + ("HTTP_500", 500), + ("HTTP_404", 404), + ("INTERNAL_SERVER_ERROR", 500), + ("VALIDATION_ERROR", 422), + ("AI_SERVICE_ERROR", 502), + ("PAYLOAD_TOO_LARGE", 413), + ], +) +def test_http_status_for_code_known(code, expected_status): + assert http_status_for_code(code) == expected_status + + +def test_http_status_for_code_unknown(): + assert http_status_for_code("NOPE") is None + + +# --------------------------------------------------------------------------- +# Issue #249 — first-declared-wins lookup contract. +# +# docs/errors.yaml and schemas/codes.py declare entries in a specific +# order. Both ``code_for_http_status`` and the TS equivalent +# ``codeForHttpStatus`` pick the FIRST enum entry that matches the +# HTTP status, so the canonical name for an HTTP-status-with-alias +# pair MUST be the ``HTTP_`` member, not the alias. +# +# This locks down the contract so a future PR that reorders the enum +# cannot silently flip the wire string from ``HTTP_500`` to +# ``INTERNAL_SERVER_ERROR`` (or vice versa). +# --------------------------------------------------------------------------- +@pytest.mark.parametrize( + "http_status,canonical", + [ + (500, "HTTP_500"), # not INTERNAL_SERVER_ERROR + (422, "HTTP_422"), # not VALIDATION_ERROR + (413, "HTTP_413"), # not PAYLOAD_TOO_LARGE + (502, "HTTP_502"), # not AI_SERVICE_ERROR + ], +) +def test_first_declared_wins_lookup_contract(http_status, canonical): + assert code_for_http_status(http_status) == canonical diff --git a/app/backend/src/common/errors/codes.spec.ts b/app/backend/src/common/errors/codes.spec.ts new file mode 100644 index 00000000..143fb042 --- /dev/null +++ b/app/backend/src/common/errors/codes.spec.ts @@ -0,0 +1,220 @@ +/** + * src/common/errors/codes.spec.ts + * + * Issue #249 parity test: verifies that the TypeScript binding in + * `codes.ts` is consistent with `docs/errors.yaml`, the single source of + * truth, AND with the Python binding in `app/ai-service/schemas/codes.py`. + * + * The test loads the YAML from the repo root, walks every `codes` entry, + * and asserts: + * 1. `ErrorCode[yamlCode]` exists and round-trips to `yamlCode`. + * 2. `ERROR_CODE_META[ErrorCode[yamlCode]].httpStatus === yaml.httpStatus`. + * 3. `ERROR_CODE_META[ErrorCode[yamlCode]].description === yaml.description`. + * 4. Every key in `ERROR_CODE_META` exists in `ErrorCode`. + * + * The YAML may have entries the TS module has not yet implemented — that + * is a deliberate parity failure (issue asks: "identical between backend + * and AI service"). + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import { + ErrorCode, + ERROR_CODE_META, + codeForHttpStatus, + httpStatusForCode, +} from './codes'; + +interface YamlCodeEntry { + code: string; + httpStatus: number; + description: string; + category?: string; +} + +interface YamlDoc { + version: number; + codes: YamlCodeEntry[]; +} + +const YAML_PATH = path.resolve(__dirname, '../../../docs/errors.yaml'); + +function loadYaml(): YamlDoc { + if (!fs.existsSync(YAML_PATH)) { + throw new Error( + `Shared error-code taxonomy not found at ${YAML_PATH}. ` + + `Issue #249 requires docs/errors.yaml as the single source of truth.`, + ); + } + const raw = fs.readFileSync(YAML_PATH, 'utf8'); + + // Minimal line-based YAML parser sufficient for docs/errors.yaml's + // simple key: scalar / dash-list schema. We avoid pulling in a + // YAML dependency just for one static config file. + // + // NOTE — when changing this parser, KEEP IT IN LOCK-STEP with + // _load_yaml() in app/ai-service/tests/test_codes.py. The two + // parsers MUST interpret the same lines the same way or the parity + // suite will pass or fail asymmetrically across the two repos. + const codes: YamlCodeEntry[] = []; + let inCodesBlock = false; + let current: Partial | null = null; + let version = 1; + for (const rawLine of raw.split('\n')) { + // Strip both leading AND trailing whitespace so ` - code: ...` + // (a YAML list marker nested two spaces in) can be detected by the + // simple `startsWith('- ')` test below. Also drop any comment-only + // or trailing-comment lines (anything past the first `#`). + const hashIdx = rawLine.indexOf('#'); + const lineNoComment = hashIdx >= 0 ? rawLine.slice(0, hashIdx) : rawLine; + const trimmed = lineNoComment.trim(); + if (trimmed === '') continue; + if (trimmed.startsWith('version:')) { + const v = Number(trimmed.split(':')[1].trim()); + if (Number.isInteger(v)) version = v; + continue; + } + if (trimmed === 'codes:') { + inCodesBlock = true; + continue; + } + if (!inCodesBlock) continue; + if (trimmed.startsWith('- ')) { + // Start of a new entry: the rest of the line is a `key: value` pair + // whose KEY is the entry's first property (e.g. `- code: HTTP_400` + // → ``code = "HTTP_400"``), NOT a single value squashed into a + // hard-coded `code` key. We parse the remainder with the same + // key:value regex used for subsequent indented lines so the two + // are guaranteed to agree. + const prev = current; + current = {}; + const tail = trimmed.slice(2); // e.g. ``code: HTTP_400`` + const mEntry = /^([a-zA-Z_]+):\s*(.*)$/.exec(tail); + if (mEntry) { + setKeyOn(current, mEntry[1], mEntry[2].replace(/^['"]|['"]$/g, '').trim()); + } else { + // Malformed entry line; drop `current` so subsequent key/value + // lines don't attach to garbage. The previous entry (if any) + // was already pushed above. + current = null; + } + if (prev) { + codes.push(prev as YamlCodeEntry); + } + continue; + } + if (!current) continue; + // Allow `key:` and `key: value` lines. We deliberately do NOT + // handle multi-line `|` / `>` block scalars — docs/errors.yaml + // keeps every description on a single line, ending in `.`. + // If a future description contains a literal `:`, this regex + // WILL truncate at the first `:`. The parity test will catch + // any such breakage because the Python module's _load_yaml uses + // an identical regex. + const m = /^([a-zA-Z_]+):\s*(.*)$/.exec(trimmed); + if (m) { + setKeyOn(current, m[1], m[2].replace(/^['"]|['"]$/g, '').trim()); + } + } + if (current) { + codes.push(current as YamlCodeEntry); + } + return { version, codes }; +} + +/** + * Write a single key/value pair into `target`, applying any required + * type coercion. Mirrors `_coerce_value` in + * `app/ai-service/tests/test_codes.py` — keep them in lock-step so + * the two YAML parsers cannot disagree on how a scalar is interpreted. + */ +function setKeyOn( + target: Partial, + key: string, + raw: string, +): void { + if (key === 'httpStatus') { + target.httpStatus = Number(raw); + } else { + // All other keys (`code`, `description`, `category`, …) stay + // as strings. Adding a new numeric key means extending this + // switch AND the matching switch in `_coerce_value`. + target[key] = raw; + } +} + +describe('Shared error-code taxonomy (Issue #249)', () => { + describe('parity with docs/errors.yaml', () => { + it('every YAML entry has a matching ErrorCode + meta entry', () => { + const yaml = loadYaml(); + expect(yaml.codes.length).toBeGreaterThan(0); + + for (const entry of yaml.codes) { + const tsValue = (ErrorCode as Record)[entry.code]; + expect(tsValue).toBeDefined(); + expect(tsValue).toBe(entry.code); + + const enumKey = (ErrorCode as Record)[ + entry.code + ] as ErrorCode; + const meta = ERROR_CODE_META[enumKey]; + expect(meta).toBeDefined(); + expect(meta.code).toBe(entry.code); + expect(meta.httpStatus).toBe(entry.httpStatus); + expect(meta.description).toBe(entry.description); + } + }); + + it('every ErrorCode enum value is present in docs/errors.yaml', () => { + const yaml = loadYaml(); + const yamlCodes = new Set(yaml.codes.map(c => c.code)); + for (const value of Object.values(ErrorCode)) { + expect(yamlCodes.has(value as string)).toBe(true); + } + }); + + it('every ERROR_CODE_META key is in the ErrorCode enum', () => { + const enumValues = new Set(Object.values(ErrorCode)); + for (const key of Object.keys(ERROR_CODE_META)) { + expect(enumValues.has(key)).toBe(true); + } + }); + }); + + describe('reverse lookups', () => { + it('codeForHttpStatus returns the canonical string code for known statuses', () => { + // Mirror a few that are stable in the YAML. + expect(codeForHttpStatus(500)).toBe('HTTP_500'); + expect(codeForHttpStatus(404)).toBe('HTTP_404'); + expect(codeForHttpStatus(422)).toBe('HTTP_422'); + }); + + it('codeForHttpStatus falls back to HTTP_ for unknown statuses', () => { + expect(codeForHttpStatus(418)).toBe('HTTP_418'); + }); + + it('httpStatusForCode returns the canonical numeric status for known codes', () => { + expect(httpStatusForCode('HTTP_500')).toBe(500); + expect(httpStatusForCode('HTTP_404')).toBe(404); + }); + + it('httpStatusForCode returns undefined for unknown codes', () => { + expect(httpStatusForCode('NOPE')).toBeUndefined(); + }); + }); + + describe('first-declared-wins lookup contract', () => { + // Issue #249 — `codeForHttpStatus(500)` MUST resolve to `HTTP_500` + // (the canonical name), not `INTERNAL_SERVER_ERROR` (the alias) + // even though both share httpStatus=500. This locks down the + // insertion-order contract; if a future PR reorders the table + // this test catches it before the wire format silently changes. + it('returns the canonical HTTP_ name (not the alias) for shared statuses', () => { + expect(codeForHttpStatus(500)).toBe('HTTP_500'); + expect(codeForHttpStatus(422)).toBe('HTTP_422'); // not VALIDATION_ERROR + expect(codeForHttpStatus(413)).toBe('HTTP_413'); // not PAYLOAD_TOO_LARGE + expect(codeForHttpStatus(502)).toBe('HTTP_502'); // not AI_SERVICE_ERROR + }); + }); +}); diff --git a/app/backend/src/common/errors/codes.ts b/app/backend/src/common/errors/codes.ts new file mode 100644 index 00000000..375bdb03 --- /dev/null +++ b/app/backend/src/common/errors/codes.ts @@ -0,0 +1,194 @@ +/** + * src/common/errors/codes.ts + * + * Issue #249 — Shared error-code taxonomy consumed by both backend + * (NestJS, `app/backend`) and AI service (FastAPI, `app/ai-service`). + * + * This file is the TypeScript binding for `docs/errors.yaml`, the single + * source of truth. The Python binding lives in + * `app/ai-service/schemas/codes.py`. Parity between the two bindings is + * checked automatically by: + * + * - `src/common/errors/codes.spec.ts` (backend unit test) + * - `app/ai-service/tests/test_codes.py` (AI service unit test) + * + * Both tests load `docs/errors.yaml`, walk every entry, and assert that: + * 1. The string `code` from YAML matches `ErrorCode.` in this TS file. + * 2. The numeric `httpStatus` from YAML matches `ErrorCodeMeta.httpStatus`. + * 3. The `description` matches the meta table. + * + * If you change this file you MUST update `docs/errors.yaml` AND + * `app/ai-service/schemas/codes.py` in the same change. The parity tests + * will fail otherwise, which is the whole point of the issue. + */ + +/** + * Stable string codes emitted by the API surface. Order here is purely + * cosmetic and is kept alphabetical for ease of diffing against YAML. + * + * NOTE: TypeScript enums are bidirectional — `ErrorCode.HTTP_500` and + * `ErrorCode['HTTP_500']` both work; the wire format we publish is + * `ErrorCode.HTTP_500` (a string), never the underlying numeric value. + */ +export enum ErrorCode { + CODE_BODY_LENGTH_MISMATCH = 'CODE_BODY_LENGTH_MISMATCH', + HTTP_400 = 'HTTP_400', + HTTP_401 = 'HTTP_401', + HTTP_403 = 'HTTP_403', + HTTP_404 = 'HTTP_404', + HTTP_409 = 'HTTP_409', + HTTP_413 = 'HTTP_413', + HTTP_422 = 'HTTP_422', + HTTP_500 = 'HTTP_500', + HTTP_502 = 'HTTP_502', + HTTP_503 = 'HTTP_503', + INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR', + PAYLOAD_TOO_LARGE = 'PAYLOAD_TOO_LARGE', + VALIDATION_ERROR = 'VALIDATION_ERROR', + AI_SERVICE_ERROR = 'AI_SERVICE_ERROR', + AI_TIMEOUT = 'AI_TIMEOUT', +} + +/** + * Per-code metadata derived from `docs/errors.yaml`. Indexed by enum + * value so consumers can look up the HTTP status and description in O(1). + */ +export interface ErrorCodeMeta { + /** Stable identifier, identical to ErrorCode value (kept for convenience). */ + readonly code: string; + /** HTTP status emitted on the wire. */ + readonly httpStatus: number; + /** Human-readable explanation. */ + readonly description: string; +} + +/** + * Metadata table. Keys here MUST be a 1:1 with ErrorCode entries; the + * parity test fails the build if they are not. + */ +export const ERROR_CODE_META: Readonly> = { + [ErrorCode.HTTP_400]: { + code: 'HTTP_400', + httpStatus: 400, + description: + 'Bad request — the request was malformed or contained invalid parameters.', + }, + [ErrorCode.HTTP_401]: { + code: 'HTTP_401', + httpStatus: 401, + description: + 'Unauthorized — authentication is required to access this resource.', + }, + [ErrorCode.HTTP_403]: { + code: 'HTTP_403', + httpStatus: 403, + description: + 'Forbidden — the caller is authenticated but lacks permission.', + }, + [ErrorCode.HTTP_404]: { + code: 'HTTP_404', + httpStatus: 404, + description: 'Not found — the requested resource does not exist.', + }, + [ErrorCode.HTTP_409]: { + code: 'HTTP_409', + httpStatus: 409, + description: + 'Conflict — the request conflicts with the current resource state.', + }, + [ErrorCode.HTTP_413]: { + code: 'HTTP_413', + httpStatus: 413, + description: + 'Payload too large — the request body exceeds the configured limit.', + }, + [ErrorCode.HTTP_422]: { + code: 'HTTP_422', + httpStatus: 422, + description: + 'Unprocessable entity — request payload failed schema validation.', + }, + [ErrorCode.HTTP_500]: { + code: 'HTTP_500', + httpStatus: 500, + description: 'Internal server error — an unexpected exception escaped the handler.', + }, + [ErrorCode.HTTP_502]: { + code: 'HTTP_502', + httpStatus: 502, + description: 'Bad gateway — an upstream/downstream dependency failed.', + }, + [ErrorCode.HTTP_503]: { + code: 'HTTP_503', + httpStatus: 503, + description: + 'Service unavailable — temporary degradation; retry with backoff.', + }, + [ErrorCode.VALIDATION_ERROR]: { + code: 'VALIDATION_ERROR', + httpStatus: 422, + description: + 'Validation failed — request payload does not match the expected schema.', + }, + [ErrorCode.AI_SERVICE_ERROR]: { + code: 'AI_SERVICE_ERROR', + httpStatus: 502, + description: + 'AI service error — the upstream LLM/OCR provider failed to respond.', + }, + [ErrorCode.AI_TIMEOUT]: { + code: 'AI_TIMEOUT', + httpStatus: 502, + description: + 'AI service timeout — the upstream LLM exceeded its time budget.', + }, + [ErrorCode.PAYLOAD_TOO_LARGE]: { + code: 'PAYLOAD_TOO_LARGE', + httpStatus: 413, + description: + 'Payload too large — the request body exceeded the configured size limit.', + }, + [ErrorCode.CODE_BODY_LENGTH_MISMATCH]: { + code: 'CODE_BODY_LENGTH_MISMATCH', + httpStatus: 400, + description: + 'Body length mismatch — streamed bytes exceeded the declared Content-Length.', + }, + [ErrorCode.INTERNAL_SERVER_ERROR]: { + code: 'INTERNAL_SERVER_ERROR', + httpStatus: 500, + description: + 'Internal server error — generic catch-all for unhandled exceptions.', + }, +}; + +/** + * Reverse lookup: HTTP status → stable string code. Used by the + * NestJS exception filter to keep the on-wire `code` field (numeric, + * because that's the existing public API contract — see + * `test/error-handling.e2e-spec.ts` which asserts `typeof code === 'number'`) + * in lock-step with the shared taxonomy. If the same HTTP status ever + * maps to multiple codes, the FIRST one declared in YAML wins; the YAML + * parity test enforces that this lookup never disagrees with the YAML. + */ +export function codeForHttpStatus(status: number): string { + for (const meta of Object.values(ERROR_CODE_META)) { + if (meta.httpStatus === status) { + return meta.code; + } + } + return `HTTP_${status}`; +} + +/** + * Reverse lookup: stable string code → HTTP status. Used by the AI + * service's parity tests to verify the binding is well-formed. + */ +export function httpStatusForCode(code: string): number | undefined { + for (const meta of Object.values(ERROR_CODE_META)) { + if (meta.code === code) { + return meta.httpStatus; + } + } + return undefined; +} diff --git a/app/backend/src/common/filters/http-exception.filter.ts b/app/backend/src/common/filters/http-exception.filter.ts index 0a015d00..80ba7872 100644 --- a/app/backend/src/common/filters/http-exception.filter.ts +++ b/app/backend/src/common/filters/http-exception.filter.ts @@ -10,6 +10,14 @@ import { Request, Response } from 'express'; import { ValidationError } from 'class-validator'; import { LoggerService } from '../../logger/logger.service'; +// Issue #249 established a shared error-code taxonomy in +// `src/common/errors/codes.ts` (mirror of `app/ai-service/schemas/codes.py`). +// The filter's on-wire envelope contract (`{ code: number, message: string }`) +// is frozen by the existing e2e + coverage tests; for now we keep the wire +// format unchanged and only consume the shared taxonomy at runtime when the +// error path needs a stable identifier (e.g. structured logging). Future +// PRs can adopt the string codes on the wire once all consumers are migrated. + export interface ErrorResponse { code: number; message: string; diff --git a/app/onchain/contracts/aid_escrow/src/lib.rs b/app/onchain/contracts/aid_escrow/src/lib.rs index 981e1fb7..20b68bb5 100644 --- a/app/onchain/contracts/aid_escrow/src/lib.rs +++ b/app/onchain/contracts/aid_escrow/src/lib.rs @@ -44,6 +44,17 @@ const KEY_PENDING_ADMIN: Symbol = symbol_short!("pendadm"); const KEY_ADMIN_DEADLINE: Symbol = symbol_short!("admdln"); const DEFAULT_ADMIN_DEADLINE: u64 = 7 * 24 * 60 * 60; // 7 days in seconds +// Cap on the number of distinct token addresses that can be added to the +// `allowed_tokens` allowlist. Issue #233 — `set_config` previously accepted +// an unbounded `Vec
`, allowing a malicious admin (or a buggy one) +// to permanently bloat the wasm instance storage by repeatedly pushing +// distinct addresses. 32 is plenty for humanitarian payouts and well below +// the Soroban host cost ceiling for a single contract instance. +/// +/// Public so tests and downstream consumers can reference the single +/// source of truth without hard-coding `32`. +pub const MAX_ALLOWED_TOKENS: u32 = 32; + // --- Data Types --- #[contracttype] @@ -114,6 +125,9 @@ pub enum Error { ProofTooLarge = 20, NoPendingAdmin = 21, AdminRotationExpired = 22, + // Issue #233 — `set_config` was called with more than + // `MAX_ALLOWED_TOKENS` addresses in `allowed_tokens`. + TooManyAllowedTokens = 23, } // --- Contract Events (indexer-friendly; stable topics & payloads) --- @@ -415,6 +429,8 @@ impl AidEscrow { /// /// # Errors /// Returns `Error::InvalidAmount` if `config.min_amount` is zero or negative. + /// Returns `Error::TooManyAllowedTokens` if `config.allowed_tokens.len()` exceeds + /// `MAX_ALLOWED_TOKENS` (Issue #233). /// Returns `Error::NotAuthorized` if caller is not the admin. pub fn set_config(env: Env, config: Config) -> Result<(), Error> { let admin = Self::get_admin(env.clone())?; @@ -424,6 +440,14 @@ impl AidEscrow { return Err(Error::InvalidAmount); } + // Issue #233 — reject oversized allowlists before doing any per-token + // contract invocations. Doing the cap check first means a malicious + // admin cannot burn host resources by sending thousands of addresses + // each of which we would otherwise have to cross-contract call. + if config.allowed_tokens.len() > MAX_ALLOWED_TOKENS { + return Err(Error::TooManyAllowedTokens); + } + for i in 0..config.allowed_tokens.len() { let token = config.allowed_tokens.get(i).ok_or(Error::InvalidToken)?; Self::validate_token(&env, &token)?; diff --git a/app/onchain/contracts/aid_escrow/tests/aid_escrow_tests.rs b/app/onchain/contracts/aid_escrow/tests/aid_escrow_tests.rs index e7b29212..8e0bfec9 100644 --- a/app/onchain/contracts/aid_escrow/tests/aid_escrow_tests.rs +++ b/app/onchain/contracts/aid_escrow/tests/aid_escrow_tests.rs @@ -244,6 +244,73 @@ mod token_interactions { assert_eq!(result, Err(Ok(Error::InvalidToken))); } + /// Issue #233 — gating `set_config.allowed_tokens` at `MAX_ALLOWED_TOKENS` + /// (32) prevents admins from bloating wasm instance storage with + /// thousands of distinct addresses. An oversized vec must be rejected + /// with `Error::TooManyAllowedTokens` before any per-token validation + /// runs (we use dummy addresses to prove the cap is enforced upfront). + #[test] + fn set_config_rejects_too_many_allowed_tokens() { + // Reference the canonical constant from the crate so any future + // change to the cap is picked up here automatically rather than + // silently drifting out of spec. + const MAX: u32 = aid_escrow::MAX_ALLOWED_TOKENS; + + let t = TestSetup::new(); + let mut allowed_tokens: Vec
= Vec::new(&t.env); + for _ in 0..(MAX + 1) { + // Dummy addresses — the cap check runs BEFORE validation, so any + // address (even a non-token contract) triggers TooManyAllowedTokens. + // MAX + 1 entries matches the acceptance criterion's "vec[0..=MAX=33]". + allowed_tokens.push_back(Address::generate(&t.env)); + } + + assert_eq!(allowed_tokens.len(), MAX + 1); + + let result = t.client.try_set_config(&Config { + min_amount: 1, + max_expires_in: 0, + allowed_tokens, + }); + + assert_eq!(result, Err(Ok(Error::TooManyAllowedTokens))); + } + + /// Issue #233 — the boundary case `allowed_tokens.len() == MAX_ALLOWED_TOKENS` + /// must NOT be rejected by the cap check (the cap is exclusive: `len > MAX` + /// rejects). The allowlist contents are dummy `{AidEscrow, ()}` contract + /// instances which are valid `Address` values but NOT Stellar token + /// contracts, so per-token validation will fail with `InvalidToken` — + /// but only AFTER the cap check has passed. We assert `InvalidToken`, + /// not `TooManyAllowedTokens`, to prove the cap check is exclusive. + /// + /// (Renamed from `set_config_accepts_max_allowed_tokens_size` because the + /// test does NOT actually accept — it proves the cap is exclusive by + /// showing the per-token validator runs at the boundary.) + #[test] + fn set_config_passes_cap_check_at_max_boundary() { + const MAX: u32 = aid_escrow::MAX_ALLOWED_TOKENS; + + let t = TestSetup::new(); + let mut allowed_tokens: Vec
= Vec::new(&t.env); + for _ in 0..MAX { + allowed_tokens.push_back(t.env.register(AidEscrow, ())); + } + + assert_eq!(allowed_tokens.len(), MAX); + + let result = t.client.try_set_config(&Config { + min_amount: 1, + max_expires_in: 0, + allowed_tokens, + }); + + // Cap check passed → per-token validation rejects the dummies. + // If this ever flips to `Err(Ok(Error::TooManyAllowedTokens))` + // the cap is no longer exclusive — investigate before relaxing. + assert_eq!(result, Err(Ok(Error::InvalidToken))); + } + #[test] fn fund_maps_reverted_token_transfer_to_clear_contract_error() { let t = TestSetup::new(); diff --git a/docs/errors.yaml b/docs/errors.yaml new file mode 100644 index 00000000..81b6b497 --- /dev/null +++ b/docs/errors.yaml @@ -0,0 +1,92 @@ +# --------------------------------------------------------------------------- +# docs/errors.yaml +# Issue #249 — Shared error-code taxonomy +# +# Single source of truth for every error code either the backend (NestJS) +# or the AI service (FastAPI) may emit. The two services hand-roll their +# own typed bindings from this file: +# +# * Backend: src/common/errors/codes.ts (TypeScript enum) +# * AI svc: app/ai-service/schemas/codes.py (Python enum + module) +# +# Both bindings MUST enumerate the same set of codes. Parity is enforced +# by equality tests in each repo: +# * backend/src/common/errors/codes.spec.ts +# * app/ai-service/tests/test_codes.py +# +# Schema (validated by the parity tests): +# code : stable string identifier (same in both languages) +# httpStatus : HTTP status emitted on the wire +# description : human-readable explanation used in docs and SDK tooltips +# category : one of http | validation | ai | database | server | client +# --------------------------------------------------------------------------- +version: 1 + +codes: + # ---- HTTP status codes (mirrored by the Ai-service `HTTP_{n}` family) -- + - code: HTTP_400 + httpStatus: 400 + description: "Bad request — the request was malformed or contained invalid parameters." + category: http + - code: HTTP_401 + httpStatus: 401 + description: "Unauthorized — authentication is required to access this resource." + category: http + - code: HTTP_403 + httpStatus: 403 + description: "Forbidden — the caller is authenticated but lacks permission." + category: http + - code: HTTP_404 + httpStatus: 404 + description: "Not found — the requested resource does not exist." + category: http + - code: HTTP_409 + httpStatus: 409 + description: "Conflict — the request conflicts with the current resource state." + category: http + - code: HTTP_413 + httpStatus: 413 + description: "Payload too large — the request body exceeds the configured limit." + category: http + - code: HTTP_422 + httpStatus: 422 + description: "Unprocessable entity — request payload failed schema validation." + category: http + - code: HTTP_500 + httpStatus: 500 + description: "Internal server error — an unexpected exception escaped the handler." + category: http + - code: HTTP_502 + httpStatus: 502 + description: "Bad gateway — an upstream/downstream dependency failed." + category: http + - code: HTTP_503 + httpStatus: 503 + description: "Service unavailable — temporary degradation; retry with backoff." + category: http + + # ---- Domain-specific codes not tied to a generic HTTP status ---------- + - code: VALIDATION_ERROR + httpStatus: 422 + description: "Validation failed — request payload does not match the expected schema." + category: validation + - code: AI_SERVICE_ERROR + httpStatus: 502 + description: "AI service error — the upstream LLM/OCR provider failed to respond." + category: ai + - code: AI_TIMEOUT + httpStatus: 502 + description: "AI service timeout — the upstream LLM exceeded its time budget." + category: ai + - code: PAYLOAD_TOO_LARGE + httpStatus: 413 + description: "Payload too large — the request body exceeded the configured size limit." + category: client + - code: CODE_BODY_LENGTH_MISMATCH + httpStatus: 400 + description: "Body length mismatch — streamed bytes exceeded the declared Content-Length." + category: client + - code: INTERNAL_SERVER_ERROR + httpStatus: 500 + description: "Internal server error — generic catch-all for unhandled exceptions." + category: server