Application Service — Rust / Actix-web

BA Review — pdf-engine

Date: 2026-03-19 Commit: 4e2570c Incremental from: 3e48f66 Reviewer: BA Agent
01 — Summary
Compliance Status
COMPLIANT
0 missing criteria • 0 HIGH deviations • 28 of 29 criteria fully met. One PARTIAL (AC-022 — no Redpanda integration test, no CI workflow file). Three tracked deviations: 1×MEDIUM, 2×LOW, all pre-existing.
29
Total
28
Met
1
Partial
0
Missing
0
N/A
Spec note: No formal spec.md found at the expected path. Review performed against tasks/todo.md (SEC-FIX-2 design decisions), progress.md implementation notes, CLAUDE.md service constraints, and ODS Platform global rules. Incremental: commits 7d59f13 (SEC-FIX-2) and 4e2570c (progress doc update).
02 — Acceptance Criteria
Met
Partial
Missing
N/A
ID Criterion Status Evidence
AC-001 Every business table has tenant_id column Met migrations/001_create_templates.sql:8, migrations/003_create_jobs.sql:5 — tenant_id UUID NOT NULL on both tables.
AC-002 RLS enabled with session variable enforcement on all tables Met migrations/002,004_enable_rls_*.sql — ENABLE ROW LEVEL SECURITY. SEC-02: migrations/006_fix_rls_insert_policies.sql — WITH CHECK enforces tenant_id = current_setting(…) on INSERT.
AC-003 tenant_id from JWT claims, never from request body Met src/api/extractors.rs — tenant_id parsed from claims.tenant_id. All handlers use AuthenticatedUser.tenant_id. No body/multipart field accepted.
AC-004 Tenant data isolation — tenant A cannot access tenant B data Met template_repo.rs, job_repo.rs — WHERE includes AND tenant_id = $2. Tests: template_repo_tenant_isolation, rls_enforced_at_db_level.
AC-005 JWT Bearer auth required on all protected endpoints Met src/api/extractors.rs — returns PdfError::Unauthorized on missing/invalid token. All handlers use AuthenticatedUser extractor. SEC-03: WARN log with peer, path, failure reason.
AC-006 JWT RS256 as primary; HS256 requires explicit opt-in Met src/api/extractors.rs:35-56 — RS256 enforces JWT_RSA_PUBLIC_KEY + JWT_ISSUER + JWT_AUDIENCE. HS256 blocked unless JWT_ALLOW_HS256=true. Test: from_env_refuses_hs256_without_explicit_opt_in.
AC-007 CloudEvents v1.0 format for all events Met src/events/producer.rs — CloudEvent struct: specversion='1.0', id, source, type, subject, time, datacontenttype, tenantid, correlationid, data. SEC-07: with_correlation_id() constructor added.
AC-008 Events published to topic events.pdf-engine Met src/main.rs — REDPANDA_TOPIC defaults to 'events.pdf-engine'. All producers wired through main.rs.
AC-009 Event types follow ods.{service}.{entity}.{action} Met src/events/producer.rs — format!("ods.pdf-engine.{event_type}"). 12 event types: template.created/updated/deleted, job.created/completed/failed, split/merge/rotate.completed/failed.
AC-010 Events emitted for all state changes Met template_service.rs, job_service.rs, api/split.rs, api/merge.rs, api/rotate.rs — all 12 state changes covered. SEC-07: X-Correlation-Id propagated to all events.
AC-011 Soft delete only — no hard deletes Met src/domain/template.rs, job.rs — soft_delete() sets deleted_at. All list/find queries filter WHERE deleted_at IS NULL.
AC-012 All timestamps in UTC Met DateTime<Utc> throughout domain. Migrations use TIMESTAMPTZ. CloudEvent.time uses Utc::now().
AC-013 Parameterized SQL — no string interpolation Met template_repo.rs, job_repo.rs — all queries use $1..$N sqlx bind parameters. Processing operations perform no SQL.
AC-014 Dockerfile: multi-stage build, non-root user, healthcheck Met Dockerfile — rust builder → debian:bookworm-slim runtime. groupadd/useradd app, USER app. HEALTHCHECK via curl /health.
AC-015 No secrets in source code — config via env vars Met src/main.rs — all sensitive config via env vars. SEC-05: dead deps reqwest, validator removed from Cargo.toml.
AC-016 Health check at GET /health Met src/main.rs — 200 JSON {status: ok, service: pdf-engine, version: CARGO_PKG_VERSION}.
AC-017 Readiness check at GET /ready Met src/main.rs — PostgreSQL probe (SELECT 1). 200 on healthy, 503 on DB failure.
AC-018 Template CRUD REST API Met POST /v1/templates (201), GET /v1/templates, GET /v1/templates/{id}, PATCH /v1/templates/{id}, DELETE /v1/templates/{id} (204) — src/api/templates.rs.
AC-019 Job lifecycle REST API with state machine Met src/domain/job.rs — Pending→Processing→Completed/Failed. src/service/job_service.rs — TransitionAction enum. PATCH /v1/jobs/{id} action: start/complete/fail.
AC-020 Domain layer isolated from repository/API layers Met src/domain/ imports only chrono, serde, uuid, crate::error. Clean layers: domain → service → api. Processing imports only lopdf + error.
AC-021 Integration tests with real PostgreSQL via testcontainers Met tests/integration.rs — 13 tests using AsyncRunner + testcontainers_modules::postgres. Covers CRUD, tenant isolation, RLS, job lifecycle, events.
AC-022 CI pipeline: lint + test + build + deploy gates Partial Local gates (cargo fmt, clippy, test --all) in CLAUDE.md. No .github/workflows/ CI file. No Redpanda integration test — events via InMemoryProducer only. testcontainers-modules kafka feature present but unused.
AC-023 PDF Split: POST /v1/split multipart upload Met src/api/split.rs + src/processing/split.rs — SplitSpec (All|Ranges), 50 MB PDF cap. SEC-07/08: X-Correlation-Id in events, 64 KB MAX_SPEC_SIZE. Base64 response.
AC-024 No unwrap() in production code paths Met All production paths use PdfResult<T> / ? operator. SEC-06: Mutex::lock().unwrap() → .expect("… mutex poisoned"). .unwrap() in #[cfg(test)] only.
AC-025 JWT issuer and audience claims enforced Met src/api/extractors.rs:41-56 — set_issuer() + set_audience() + set_required_spec_claims(["sub","iss","aud"]). 4 tests: missing_iss, wrong_issuer, wrong_audience, correct_accepted.
AC-026 Domain-layer input validation: field size caps Met MAX_HTML_BYTES=512 KB, MAX_SCHEMA_BYTES=64 KB, MAX_PDF_SIZE=50 MB, MAX_MERGE_FILES=20. SEC-08: MAX_SPEC_SIZE=64 KB on split+rotate spec field.
AC-027 PDF Merge: POST /v1/merge returning merged PDF Met src/api/merge.rs + src/processing/merge.rs — multiple 'files' fields, 50 MB/file cap, 20 files max, 200 MB total, lopdf deep-copy remap. MergeResponse. SEC-07: correlation-id.
AC-028 PDF Rotate: POST /v1/rotate returning rotated PDF as base64 Met src/api/rotate.rs + src/processing/rotate.rs — RotateSpec{angle, pages}, angle validated to {90,180,270}, additive /Rotate key update. RotateResponse. SEC-07/08.
AC-029 RBAC authorization design decision documented Met docs/adr/001-defer-rbac-enforcement.md — SEC-04. Roles extracted from JWT (extractors.rs:163) but RBAC enforcement deferred pending OID role definitions. Documented and intentional.
29 criteria reviewed — incremental from commit 3e48f66 28 met · 1 partial · 0 missing Coverage 97%
03 — Deviations
Medium
No Redpanda / Kafka integration test. All event publishing tests use InMemoryProducer. RedpandaProducer (src/events/producer.rs) is untested end-to-end. The testcontainers-modules kafka feature is in dev-dependencies (intent confirmed) but no test written. All new event types (split/merge/rotate.completed/failed) are also covered by InMemoryProducer only.
Pre-existing — unchanged since initial review. Ref: Global CLAUDE.md TDD rules — integration tests require real Redpanda (testcontainers). Architecture: Event-First — every state change emits to Redpanda.
Low
Merge total-size check enforced after full in-memory buffering. The 200 MB aggregate cap is checked inside merge_pdfs() after the handler has already buffered all file bytes. A caller sending 20 files × 50 MB = 1 GB will cause the full 1 GB to be held in memory before the check rejects the request. Per-file streaming cap (50 MB) exists but total is not enforced at the streaming boundary.
Pre-existing — unchanged. Ref: Global CLAUDE.md — "No Laziness — Find root causes" / service constraint: validate at system boundaries.
Low
RBAC enforcement intentionally deferred (ADR-001). No endpoint-level authorization checks. Roles extracted from JWT claims (extractors.rs:163, roles: Vec<String>) but not evaluated. Documented architectural decision in docs/adr/001-defer-rbac-enforcement.md (SEC-04) pending OID role definitions — not an oversight.
Intentional — documented in ADR-001. Ref: CLAUDE.md — JWT from OID (middleware extracts tenant_id). Global: M2M calls via scoped tokens. ADR records deferral rationale and future implementation path.
04 — SEC-FIX-2 Changes (commit 7d59f13)

All 8 security fixes confirmed implemented and tested in this review cycle.

SEC-01
DB error message sanitization
src/error.rs — Conflict and Internal errors return generic messages. Internal DB details no longer leaked in HTTP responses.
SEC-02
RLS INSERT policies enforce tenant_id
migrations/006_fix_rls_insert_policies.sql — drops old WITH CHECK (true), replaces with WITH CHECK (tenant_id = current_setting(…)) on both tables.
SEC-03
Auth failures logged at WARN
src/api/extractors.rs — WARN log includes peer address, request path, and failure reason (missing_authorization_header, invalid_bearer_format, invalid_token, etc.).
SEC-04
RBAC deferral documented (ADR-001)
docs/adr/001-defer-rbac-enforcement.md written. Roles extracted from JWT but enforcement deferred pending OID role definitions.
SEC-05
Dead dependencies removed
Cargo.toml — reqwest and validator removed. Reduces attack surface and compile times.
SEC-06
Mutex unwrap replaced with expect
InMemoryProducer — Mutex::lock().unwrap() → .expect("InMemoryProducer events mutex poisoned"). Context on panic.
SEC-07
X-Correlation-Id propagated to CloudEvents
with_correlation_id() constructor. Split, merge, and rotate handlers propagate inbound X-Correlation-Id into CloudEvents correlationid field.
SEC-08
64 KB MAX_SPEC_SIZE on spec field
src/api/split.rs and src/api/rotate.rs — streaming size limit on the spec multipart JSON field. Prevents oversized payloads from buffering.
AC-022 Detail — Why CI is PARTIAL
What is missing: A .github/workflows/ CI file confirming gates run on every push. A Redpanda integration test exercising RedpandaProducer end-to-end.

What is present: Local quality gates (cargo fmt, clippy -D warnings, cargo test --all) defined in CLAUDE.md. 13 PostgreSQL integration tests using testcontainers. testcontainers-modules kafka feature in dev-deps confirms intent.

Recommended next steps: Write a Kafka testcontainer test sending an event through RedpandaProducer and confirming delivery. Add .github/workflows/ci.yml running fmt + clippy + test --all on every push to dev.