marcel

marcel opened issue marcel/familienarchiv#619

2026-05-18 15:20:52 +02:00

feat(import): surface S3 upload failures in skippedFiles

marcel pushed to worktree-feat+issue-529-pdf-magic-bytes at marcel/familienarchiv

2026-05-18 15:20:30 +02:00

53dba772ae fix(import): add @Schema annotations and fix IOException test coverage

marcel commented on pull request marcel/familienarchiv#618

2026-05-18 15:12:34 +02:00

security(import): validate PDF magic bytes before S3 upload

Review response (commit 53dba772)

Fixed

@Schema annotations missing on SkippedFile / ImportStatus (Markus) — added @Schema(requiredMode = REQUIRED) to all non-nullable record…

marcel commented on pull request marcel/familienarchiv#617

2026-05-18 15:07:28 +02:00

feat(security): CSRF protection, session revocation, login rate limiting (#524)

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Verdict: ✅ Approved

The rate-limit UI change is small but well-considered. The two main accessibility improvements (adding…

marcel commented on pull request marcel/familienarchiv#617

2026-05-18 15:07:14 +02:00

feat(security): CSRF protection, session revocation, login rate limiting (#524)

📋 Elicit — Senior Requirements Engineer

Verdict: ✅ Approved

All three requirements from issue #524 are implemented and traceable. The ADR-022 provides the requirements rationale. I…

marcel commented on pull request marcel/familienarchiv#617

2026-05-18 15:06:59 +02:00

feat(security): CSRF protection, session revocation, login rate limiting (#524)

🚀 Tobias Wendt — DevOps & Platform Engineer

Verdict: ✅ Approved

No infrastructure changes required, no new Docker services, no new ports. The implementation stays entirely within the…

marcel commented on pull request marcel/familienarchiv#617

2026-05-18 15:06:45 +02:00

feat(security): CSRF protection, session revocation, login rate limiting (#524)

🧪 Sara Holt — Senior QA Engineer

Verdict: ⚠️ Approved with concerns

The backend test coverage for this PR is excellent. LoginRateLimiterTest covers 6 scenarios including the…

marcel commented on pull request marcel/familienarchiv#617

2026-05-18 15:06:29 +02:00

feat(security): CSRF protection, session revocation, login rate limiting (#524)

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ✅ Approved

Clean implementation overall. The code is well-structured, test names are descriptive, and the TypeScript side…

marcel commented on pull request marcel/familienarchiv#617

2026-05-18 15:06:13 +02:00

feat(security): CSRF protection, session revocation, login rate limiting (#524)

🏛️ Markus Keller — Senior Application Architect

Verdict: 🚫 Changes requested

The implementation is architecturally sound. The design decisions (in-memory rate limiter, double-submit…

marcel commented on pull request marcel/familienarchiv#618

2026-05-18 15:06:05 +02:00

security(import): validate PDF magic bytes before S3 upload

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Verdict: ⚠️ Approved with concerns

The collapsible <details>/<summary> pattern is semantically correct and accessible by…

marcel commented on pull request marcel/familienarchiv#617

2026-05-18 15:05:55 +02:00

feat(security): CSRF protection, session revocation, login rate limiting (#524)

🔐 Nora "NullX" Steiner — Application Security Engineer

Verdict: ⚠️ Approved with concerns

The three security controls implemented here are all conceptually correct. The double-submit…

marcel commented on pull request marcel/familienarchiv#618

2026-05-18 15:05:51 +02:00

security(import): validate PDF magic bytes before S3 upload

🧪 Sara Holt — QA Engineer & Test Strategist

Verdict: ⚠️ Approved with concerns

Four new behavioral tests cover the primary scenarios well. The makeStatus factory function is already…

marcel commented on pull request marcel/familienarchiv#618

2026-05-18 15:05:39 +02:00

security(import): validate PDF magic bytes before S3 upload

🔐 Nora "NullX" Steiner — Application Security Engineer

Verdict: ✅ Approved

This PR does what it says and does it correctly. The magic bytes check is a legitimate server-side control…

marcel commented on pull request marcel/familienarchiv#618

2026-05-18 15:05:28 +02:00

security(import): validate PDF magic bytes before S3 upload

📋 Elicit — Requirements Engineer

Verdict: ⚠️ Approved with concerns

The implementation addresses the core requirement from #529: PDF magic bytes are validated before S3 upload, and the…

marcel commented on pull request marcel/familienarchiv#618

2026-05-18 15:05:16 +02:00

security(import): validate PDF magic bytes before S3 upload

🛠️ Tobias Wendt — DevOps & Platform Engineer

Verdict: ✅ Approved

Pure application code change — no infrastructure footprint at all.

Checked:

No Docker Compose changes
No new…

marcel commented on pull request marcel/familienarchiv#618

2026-05-18 15:05:11 +02:00

security(import): validate PDF magic bytes before S3 upload

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ⚠️ Approved with concerns

Overall the code is clean. isPdfMagicBytes is a sharp, single-purpose predicate. ProcessResult and…

marcel commented on pull request marcel/familienarchiv#618

2026-05-18 15:04:59 +02:00

security(import): validate PDF magic bytes before S3 upload

🏗️ Markus Keller — Senior Application Architect

Verdict: ⚠️ Approved with concerns

Good containment: the entire change lives inside the importing module. No cross-domain repository…

marcel pushed to worktree-feat+issue-529-pdf-magic-bytes at marcel/familienarchiv

2026-05-18 14:59:09 +02:00

e770b81ea5 fix(test): skip IOException test when running as root

marcel pushed to worktree-feat+issue-529-pdf-magic-bytes at marcel/familienarchiv

2026-05-18 14:46:45 +02:00

50441f558f fix(import): address PR review concerns

marcel commented on pull request marcel/familienarchiv#617

2026-05-18 14:12:15 +02:00

feat(security): CSRF protection, session revocation, login rate limiting (#524)

📋 Elicit — Requirements Engineer

Verdict: ⚠️ Approved with concerns

All three security requirements from issue #524 are implemented and traceable to code. One ambiguity in the…