fix(m4): code-review pass — SSRF allowlist + advisory lock + typed contract
Six post-code-review fixes, applied before opening the PR per project
workflow (spec-review + code-review both gate the merge):
1. SSRF allowlist on `/mitre/sync`. Host must be in MITRE_ALLOWED_HOSTS
(defaults to `raw.githubusercontent.com`, env-overridable). Closes "admin
holding `mitre.sync` pivots api container at 169.254.169.254 / internal
mirrors" via a typo'd URL. New `MitreSourceForbidden` → 400
`source_forbidden`; checked at the top of `_download()` so it kicks in
before any I/O.
2. `pg_advisory_xact_lock(hashtext('mitre.seed'))` at the top of the seed
transaction. Two concurrent `/mitre/sync` requests now serialise across
the DELETE+INSERT of `mitre_technique_tactics`; previously they could
both wipe the M2M and one would fail the unique constraint on re-insert.
3. Typed SyncResult contract. Pydantic `SyncResultOut` on the Flask side
`model_validate`s the dict before returning — single source of truth
for the response shape, mirrored by a `MitreSyncResult` TS interface
(next commit). The `as Record<string, unknown>` + `as { duration_ms }`
cast in MitrePage is gone.
4. N+1 in dotted sub-technique fallback removed. Built
`{external_id → technique_id}` once at function entry. Currently a
no-op against MITRE official (0 orphans), but a latent footgun for
partial / older bundles.
5. `SETTING_VERSION` cleared explicitly when `source != MITRE_DEFAULT_URL`.
Previously it kept the stale pin label, so `/mitre/status` lied after
a custom-URL re-sync.
6. `/mitre/sync` 500s no longer echo `str(e)` to the client — URLError /
psycopg / Pydantic text now lives in the JSON log only. Public response
stays `{"error": "internal_error"}`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -9,6 +9,7 @@ from __future__ import annotations
|
||||
import logging
|
||||
|
||||
from flask import Blueprint, jsonify, request
|
||||
from pydantic import BaseModel
|
||||
from sqlalchemy import func, or_, select
|
||||
|
||||
from app.core.auth_decorators import require_auth, require_perm
|
||||
@@ -16,6 +17,21 @@ from app.db.session import session_scope
|
||||
from app.models.mitre import MitreSubtechnique, MitreTactic, MitreTechnique
|
||||
from app.services import mitre_seed as mitre_seed_svc
|
||||
|
||||
|
||||
class SyncResultOut(BaseModel):
|
||||
"""Response schema for `POST /mitre/sync`. Mirrors `SeedResult.as_dict()`."""
|
||||
|
||||
tactics_upserted: int
|
||||
techniques_upserted: int
|
||||
subtechniques_upserted: int
|
||||
subtechniques_skipped_orphan: int
|
||||
technique_tactic_links: int
|
||||
version: str | None
|
||||
source: str
|
||||
started_at: str
|
||||
finished_at: str
|
||||
duration_ms: int
|
||||
|
||||
bp = Blueprint("mitre", __name__, url_prefix="/mitre")
|
||||
log = logging.getLogger("metamorph.api.mitre")
|
||||
|
||||
@@ -248,7 +264,8 @@ def sync():
|
||||
|
||||
Custom `source` URLs MUST be paired with either `expected_sha256` (integrity
|
||||
guarantee) or `allow_unverified: true` (explicit opt-out) — the seed service
|
||||
will raise otherwise.
|
||||
will raise otherwise. The host is allowlisted (defaults to
|
||||
raw.githubusercontent.com, overridable via the MITRE_ALLOWED_HOSTS env).
|
||||
"""
|
||||
payload = request.get_json(silent=True) or {}
|
||||
source = payload.get("source") # optional URL override
|
||||
@@ -261,12 +278,19 @@ def sync():
|
||||
or (mitre_seed_svc.MITRE_DEFAULT_SHA256 if source is None else None),
|
||||
allow_unverified=allow_unverified,
|
||||
)
|
||||
except mitre_seed_svc.MitreSourceForbidden as e:
|
||||
return jsonify({"error": "source_forbidden", "message": str(e)}), 400
|
||||
except mitre_seed_svc.MitreChecksumMismatch as e:
|
||||
return jsonify({"error": "checksum_mismatch", "message": str(e)}), 502
|
||||
except mitre_seed_svc.MitreSeedError as e:
|
||||
return jsonify({"error": "seed_failed", "message": str(e)}), 502
|
||||
except Exception as e: # noqa: BLE001
|
||||
except Exception: # noqa: BLE001
|
||||
# Do NOT leak the internal error string to the client (URLError stack,
|
||||
# DB driver text). The stack lands in our JSON logs.
|
||||
log.exception("metamorph.api.mitre.sync_failed")
|
||||
return jsonify({"error": "internal_error", "message": str(e)}), 500
|
||||
log.warning("metamorph.api.mitre.sync_done", extra=result.as_dict())
|
||||
return jsonify(result.as_dict())
|
||||
return jsonify({"error": "internal_error"}), 500
|
||||
# Validate via the Pydantic Out model so the response contract is
|
||||
# explicit (single source of truth shared with the TS interface).
|
||||
payload_out = SyncResultOut.model_validate(result.as_dict()).model_dump()
|
||||
log.info("metamorph.api.mitre.sync_done", extra=payload_out)
|
||||
return jsonify(payload_out)
|
||||
|
||||
Reference in New Issue
Block a user