fix: reject idempotency key payload conflicts

This commit is contained in:
ik
2026-09-11 07:44:07 +07:00
parent 42bdec6c2f
commit 5b2db1cf48
4 changed files with 11 additions and 1 deletions
@@ -14,6 +14,7 @@ depends_on = None
def upgrade() -> None: def upgrade() -> None:
op.add_column("submission_attempt", sa.Column("idempotency_key", sa.String(128), nullable=True)) op.add_column("submission_attempt", sa.Column("idempotency_key", sa.String(128), nullable=True))
op.add_column("submission_attempt", sa.Column("catch_report_id", sa.Uuid(), nullable=True)) op.add_column("submission_attempt", sa.Column("catch_report_id", sa.Uuid(), nullable=True))
op.add_column("submission_attempt", sa.Column("payload_hash", sa.String(64), nullable=True))
op.create_foreign_key("fk_submission_attempt_report", "submission_attempt", "catch_report", ["catch_report_id"], ["id"]) op.create_foreign_key("fk_submission_attempt_report", "submission_attempt", "catch_report", ["catch_report_id"], ["id"])
op.create_index("ix_submission_attempt_idempotency_key", "submission_attempt", ["idempotency_key"], unique=True) op.create_index("ix_submission_attempt_idempotency_key", "submission_attempt", ["idempotency_key"], unique=True)
op.add_column("import_record_event", sa.Column("changes", sa.JSON(), nullable=True)) op.add_column("import_record_event", sa.Column("changes", sa.JSON(), nullable=True))
@@ -25,4 +26,5 @@ def downgrade() -> None:
op.drop_index("ix_submission_attempt_idempotency_key", table_name="submission_attempt") op.drop_index("ix_submission_attempt_idempotency_key", table_name="submission_attempt")
op.drop_constraint("fk_submission_attempt_report", "submission_attempt", type_="foreignkey") op.drop_constraint("fk_submission_attempt_report", "submission_attempt", type_="foreignkey")
op.drop_column("submission_attempt", "catch_report_id") op.drop_column("submission_attempt", "catch_report_id")
op.drop_column("submission_attempt", "payload_hash")
op.drop_column("submission_attempt", "idempotency_key") op.drop_column("submission_attempt", "idempotency_key")
+5 -1
View File
@@ -5,6 +5,7 @@ from datetime import datetime, timedelta, timezone
from ipaddress import IPv4Address, IPv6Address, IPv4Network, IPv6Network from ipaddress import IPv4Address, IPv6Address, IPv4Network, IPv6Network
import hashlib import hashlib
import hmac import hmac
import json
import logging import logging
import secrets import secrets
import time as time_module import time as time_module
@@ -460,6 +461,7 @@ def create_catch_report(
) -> CatchReportAccepted: ) -> CatchReportAccepted:
if payload.website: if payload.website:
raise HTTPException(status_code=400, detail="invalid submission") raise HTTPException(status_code=400, detail="invalid submission")
payload_hash = hashlib.sha256(json.dumps(payload.model_dump(mode="json"), sort_keys=True, separators=(",", ":")).encode()).hexdigest()
# A05: Server-side idempotency — check BEFORE rate limit to avoid polluting table # A05: Server-side idempotency — check BEFORE rate limit to avoid polluting table
if idempotency_key: if idempotency_key:
key_hash = hmac.new(settings.rate_limit_secret.encode(), idempotency_key.encode(), hashlib.sha256).hexdigest() key_hash = hmac.new(settings.rate_limit_secret.encode(), idempotency_key.encode(), hashlib.sha256).hexdigest()
@@ -476,6 +478,8 @@ def create_catch_report(
# Return 200 with idempotent flag — client can retry safely # Return 200 with idempotent flag — client can retry safely
logger.info("idempotent hit", extra={"idempotency_key": idempotency_key[:8]}) logger.info("idempotent hit", extra={"idempotency_key": idempotency_key[:8]})
report = existing.catch_report report = existing.catch_report
if existing.payload_hash and not hmac.compare_digest(existing.payload_hash, payload_hash):
raise HTTPException(status_code=409, detail="Idempotency-Key was already used with different payload")
if report is None: if report is None:
raise HTTPException(status_code=409, detail="idempotency record is incomplete; retry with a new key") raise HTTPException(status_code=409, detail="idempotency record is incomplete; retry with a new key")
# Re-derive the one-time upload token from the idempotency key; # Re-derive the one-time upload token from the idempotency key;
@@ -509,7 +513,7 @@ def create_catch_report(
# Store idempotency key if provided # Store idempotency key if provided
if idempotency_key: if idempotency_key:
key_hash = hmac.new(settings.rate_limit_secret.encode(), idempotency_key.encode(), hashlib.sha256).hexdigest() key_hash = hmac.new(settings.rate_limit_secret.encode(), idempotency_key.encode(), hashlib.sha256).hexdigest()
db.add(SubmissionAttempt(client_hash="", idempotency_key=key_hash, catch_report_id=report.id, created_at=datetime.now(timezone.utc))) db.add(SubmissionAttempt(client_hash="", idempotency_key=key_hash, catch_report_id=report.id, payload_hash=payload_hash, created_at=datetime.now(timezone.utc)))
db.commit() db.commit()
logger.info("idempotency key stored", extra={"idempotency_key": idempotency_key[:8]}) logger.info("idempotency key stored", extra={"idempotency_key": idempotency_key[:8]})
return CatchReportAccepted(id=report.id, moderation_status=report.moderation_status.value, screenshot_upload_token=upload_token, idempotent=False) return CatchReportAccepted(id=report.id, moderation_status=report.moderation_status.value, screenshot_upload_token=upload_token, idempotent=False)
+1
View File
@@ -138,6 +138,7 @@ class SubmissionAttempt(Base):
client_hash: Mapped[str] = mapped_column(String(64), index=True) client_hash: Mapped[str] = mapped_column(String(64), index=True)
idempotency_key: Mapped[str | None] = mapped_column(String(128), index=True) idempotency_key: Mapped[str | None] = mapped_column(String(128), index=True)
catch_report_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("catch_report.id"), nullable=True) catch_report_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("catch_report.id"), nullable=True)
payload_hash: Mapped[str | None] = mapped_column(String(64), nullable=True)
created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), index=True) created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), index=True)
catch_report: Mapped[CatchReport | None] = relationship() catch_report: Mapped[CatchReport | None] = relationship()
+3
View File
@@ -441,3 +441,6 @@ def test_catch_report_idempotency_key_prevents_duplicates(monkeypatch) -> None:
assert second.json()["idempotent"] is True assert second.json()["idempotent"] is True
assert second.json()["id"] == report_id assert second.json()["id"] == report_id
assert second.json()["screenshot_upload_token"] == first.json()["screenshot_upload_token"] assert second.json()["screenshot_upload_token"] == first.json()["screenshot_upload_token"]
changed = dict(payload, weight_g=7800)
conflict = client.post("/api/v1/catch-reports", json=changed, headers=headers)
assert conflict.status_code == 409