🔒(fullstack) harden download flow and address review findings

[Nastaliss]

• drop owner identity email (PII) from the public download payload; compute is_owner server-side
• keep transfers PENDING_FILE_DELETION when an S3 purge partially fails instead of orphaning bytes
• record the expiry audit event only when the expiry sweep wins the deactivation race
• escape download filenames via RFC 6266 Content-Disposition to prevent filename spoofing
• add a strict CSP and security headers (HSTS, Referrer-Policy, X-Frame-Options, nosniff) in Caddy
• match users by email case-insensitively to avoid duplicate accounts
• raise HSTS max-age from 60s to 1 year
• point pyproject repository URL to suitenumerique/st-transfers
• fix sub validator regex range bug that admitted ',' and '/'
• document first-download auto-archive as "first access" semantics

Summary by CodeRabbit

• Bug Fixes

  • Improved account identification by using case-insensitive email matching and consistently resolving duplicate-email situations.
  • Enhanced transfer cleanup reliability: S3 deletions now report success/failure, and background processing only advances statuses or records expiry when appropriate.

• Security & Privacy

  • Removed public exposure of owner email on download transfers; ownership is now shown via an is_owner boolean.
  • Strengthened download-related security headers (longer HSTS and stricter CSP / response hardening).

• Improvements

  • More standards-compliant download filenames (RFC 6266/5987) with safer handling for non-ASCII names and injection attempts.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5cfbb462-e9f3-4950-a959-f3b6b6474366

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbb611 and 00702b1.

📒 Files selected for processing (2)

• src/backend/core/tests/test_api_downloads.py
• src/backend/core/tests/test_models.py

---

📝 Walkthrough

Walkthrough

The PR replaces owner_email with a server-computed is_owner boolean in the download API (serializer, viewset, frontend type, and route). It adds an RFC 6266-compliant Content-Disposition builder for S3 presigned URLs, makes best_effort_delete_objects_from_files return a boolean to enable task-level retry on partial S3 failures, tightens the User.sub field with a new migration and case-insensitive OIDC email fallback with PII redaction in logs, and hardens HTTP security headers in Caddy and Django.

Changes

Backend/Frontend Functional Changes

Layer / File(s)	Summary
is_owner field replacing owner_email src/backend/core/api/serializers.py, src/backend/core/api/viewsets/download.py, src/frontend/src/features/api/types.ts, src/frontend/src/routes/t/$token.tsx, src/backend/core/tests/test_api_downloads.py	DownloadTransferSerializer removes owner_email and adds is_owner as a SerializerMethodField comparing request.user to obj.owner_id. The viewset passes context={"request": request}. The frontend type and route are updated to consume is_owner directly. Tests assert owner_email is absent and verify is_owner for anonymous and authenticated sessions.
RFC 6266 Content-Disposition builder src/backend/core/services/s3.py, src/backend/core/tests/test_s3_content_disposition.py	New _content_disposition(filename) helper sanitizes an ASCII filename fallback and adds a percent-encoded UTF-8 filename* parameter. sign_download_url uses this helper. Four unit tests cover ASCII format, quote-injection resistance, non-ASCII encoding, and empty-sanitization fallback.
Boolean S3 deletion and task retry logic src/backend/core/services/s3.py, src/backend/core/models.py, src/backend/core/tasks.py, src/backend/core/api/viewsets/download.py	best_effort_delete_objects_from_files returns bool (False on any ClientError). Transfer.delete_s3_objects propagates this. The expiry task skips the TRANSFER_DEACTIVATED_AFTER_EXPIRY event when deactivate() returns false. The pending-deletion task leaves the transfer in PENDING_FILE_DELETION on S3 failure for retry. Inline comments clarify auto-archive timing in DownloadFileView.
User sub field migration and OIDC email fallback src/backend/core/migrations/0004_alter_user_sub.py, src/backend/core/models.py, src/backend/core/tests/test_models.py	New migration makes user.sub nullable/blank with max_length=255, unique=True, and a RegexValidator. The sub_validator regex escapes the hyphen. get_user_by_sub_or_email uses email__iexact, resolves MultipleObjectsReturned by picking the earliest-created record, and logs the duplicate event using only user IDs (PII redaction). Tests verify the oldest record is returned and warning omits email strings.

Security Header Hardening

Layer / File(s)	Summary
Caddy CSP/HSTS and Django HSTS duration src/frontend/caddy/Caddyfile, src/backend/transferts/settings.py	Caddyfile replaces header -Server with a full header {} block setting HSTS, X-Content-Type-Options, X-Frame-Options, Referrer-Policy, and a strict Content-Security-Policy. Django Production.SECURE_HSTS_SECONDS is raised from 60 to 31_536_000.

Metadata

Layer / File(s)	Summary
Repository URL update src/backend/pyproject.toml	The project repository URL in [project.urls] is updated to the new canonical GitHub path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is generic and uses a vague emoji-prefixed format, making it unclear what the primary security hardening measure is without reading the full changeset.	Replace with a more specific title that highlights the main change, such as 'Remove owner email from download payload and harden download security' or focus on the most critical security improvement.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

{}

[Nastaliss]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/backend/core/models.py`:
- Around line 87-90: The code silently handles the MultipleObjectsReturned
exception without logging the issue. Add a WARNING level log statement in the
except block that catches self.model.MultipleObjectsReturned to record that
duplicate email entries were found and a stable one was selected. Ensure the
logger is defined at module level using logging.getLogger(__name__) if not
already present, and include the email value in the log message to help admins
identify and investigate the data-quality issue.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7a15fb6f-2316-4308-aa89-2026104bb3b9

📥 Commits

Reviewing files that changed from the base of the PR and between 1f54766 and b108f0f.

📒 Files selected for processing (13)

• src/backend/core/api/serializers.py
• src/backend/core/api/viewsets/download.py
• src/backend/core/migrations/0004_alter_user_sub.py
• src/backend/core/models.py
• src/backend/core/services/s3.py
• src/backend/core/tasks.py
• src/backend/core/tests/test_api_downloads.py
• src/backend/core/tests/test_s3_content_disposition.py
• src/backend/pyproject.toml
• src/backend/transferts/settings.py
• src/frontend/caddy/Caddyfile
• src/frontend/src/features/api/types.ts
• src/frontend/src/routes/t/$token.tsx

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/backend/core/models.py`:
- Around line 94-98: The logger.warning call is exposing a raw email address
(PII) in the log message, which violates coding guidelines. Replace the email
parameter being logged with the user IDs of the duplicate accounts instead. This
allows admins to still identify and reconcile the duplicate accounts while
keeping sensitive email data out of the logs. Modify the warning message and its
formatting argument to reference user IDs rather than the email variable.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4072cf8d-ddb3-44ef-9919-2ba5c30521f8

📥 Commits

Reviewing files that changed from the base of the PR and between b108f0f and 804cbcc.

📒 Files selected for processing (2)

• src/backend/core/models.py
• src/backend/pyproject.toml

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/backend/core/migrations/0004_alter_user_sub.py`:
- Line 17: The help_text and the RegexValidator message in the field definition
both claim that forward slash `/` is an allowed character, but the regex pattern
`^[\w.@+\-:]+\Z` does not actually permit forward slashes. Update both the
help_text and the message parameter in the RegexValidator to remove the forward
slash `/` from the character list so they accurately describe only the
characters that the regex actually allows: letters, numbers, and @, ., +, -, _,
and : characters.

In `@src/backend/core/tests/test_api_downloads.py`:
- Around line 38-47: The test_is_owner_true_for_authenticated_owner method
validates that is_owner is True for authenticated owners but does not enforce
the privacy contract for the response. Add an assertion after the is_owner check
to verify that owner_email is not present in the response data (check that it is
either absent from response.data or explicitly None) to ensure no email
information leaks to the client, consistent with the privacy guarantees stated
in the test comment.

In `@src/backend/core/tests/test_models.py`:
- Around line 64-65: The test assertion on line 73 depends on ordering by the
created_at field, but the UserFactory calls for the older and newer variables do
not explicitly set created_at timestamps, causing the test to be flaky across
different database backends with varying timestamp resolutions. Add explicit
created_at parameters to both the UserFactory call for older and the UserFactory
call for newer, ensuring the older variable has an earlier timestamp than the
newer variable (for example, using datetime values where older precedes newer).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8287de16-a82f-4000-922d-44e78c0c31fe

📥 Commits

Reviewing files that changed from the base of the PR and between 804cbcc and 9fbb611.

📒 Files selected for processing (14)

• src/backend/core/api/serializers.py
• src/backend/core/api/viewsets/download.py
• src/backend/core/migrations/0004_alter_user_sub.py
• src/backend/core/models.py
• src/backend/core/services/s3.py
• src/backend/core/tasks.py
• src/backend/core/tests/test_api_downloads.py
• src/backend/core/tests/test_models.py
• src/backend/core/tests/test_s3_content_disposition.py
• src/backend/pyproject.toml
• src/backend/transferts/settings.py
• src/frontend/caddy/Caddyfile
• src/frontend/src/features/api/types.ts
• src/frontend/src/routes/t/$token.tsx

[Nastaliss]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.