cheapnud/CheapClerk
Watch
1
0
Fork
You've already forked CheapClerk
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add expiring share links with per-document revocation #5

Merged
cheapnud merged 1 commit from d/share-links into master 2026-08-15 19:09:23 +00:00
Conversation 1 Commits 1 Files changed 16 +436 -1
cheapnud commented 2026-08-15 19:06:37 +00:00
Owner
Copy link

SAS-style HMAC share links: pick 1h, 24h or 7d on the document page or via the share_document tool, anyone with the link can fetch the file until expiry, and a per-document generation in the cache database lets you kill all outstanding links at once.

SAS-style HMAC share links: pick 1h, 24h or 7d on the document page or via the share_document tool, anyone with the link can fetch the file until expiry, and a per-document generation in the cache database lets you kill all outstanding links at once.
Add expiring share links with per-document revocation
All checks were successful
Claude PR Review / AI Code Review (pull_request) Successful in 16s
Details
Build & Test / Build and Test (pull_request) Successful in 2m21s
Details
5f62559786
forgejo-actions commented 2026-08-15 19:06:54 +00:00
Copy link

🤖 Claude AI Code Review

  1. Summary
    Adds stateless HMAC share tokens with per-document generation-based revocation, exposed via a web menu, an anonymous /share/* endpoint, and MCP tools. Design is clean and well-tested.

  2. Code Quality ⭐⭐⭐⭐☆
    Solid crypto hygiene (FixedTimeEquals, constant 404 oracle, bounded expiry) and thorough tests including tamper/malformed cases.

  3. Issues

🟡 ShareLinkBuilder.cs — no minimum length / entropy check on SigningKey. A short or weak configured key silently weakens the HMAC. Consider rejecting keys below a sane length at startup, otherwise sharing may be effectively forgeable with a misconfigured secret.

🟢 ShareGenerationStore.cs — BumpAsync can throw on a race (two concurrent bumps inserting the same DocumentId primary key). Low impact since revocation is rarely concurrent, but a unique-constraint violation would surface as an unhandled exception. Consider catch-and-retry or upsert.

🟢 Program.cs (share endpoint) — the /share/* endpoint has no rate limiting. Since every failure returns 404 with a full HMAC verification per request, it's a mild DoS/oracle-timing surface. Not blocking given the constant-time compare, but worth noting for a public anonymous endpoint.

  1. Verdict: APPROVE

The generation-check-after-signature ordering, the ValueGeneratedNever on Paperless-assigned IDs, and the disabled-when-unset behavior are all correct. The flagged items are hardening improvements, not blockers.

📊 Tokens: 13619 input + 581 output | 💰 Cost: ~$0.0826
Generated by Claude Opus 4.8 via Gitea Actions

## 🤖 Claude AI Code Review 1. **Summary** Adds stateless HMAC share tokens with per-document generation-based revocation, exposed via a web menu, an anonymous `/share/*` endpoint, and MCP tools. Design is clean and well-tested. 2. **Code Quality** ⭐⭐⭐⭐☆ Solid crypto hygiene (FixedTimeEquals, constant 404 oracle, bounded expiry) and thorough tests including tamper/malformed cases. 3. **Issues** 🟡 ShareLinkBuilder.cs — no minimum length / entropy check on `SigningKey`. A short or weak configured key silently weakens the HMAC. Consider rejecting keys below a sane length at startup, otherwise sharing may be effectively forgeable with a misconfigured secret. 🟢 ShareGenerationStore.cs — `BumpAsync` can throw on a race (two concurrent bumps inserting the same `DocumentId` primary key). Low impact since revocation is rarely concurrent, but a unique-constraint violation would surface as an unhandled exception. Consider catch-and-retry or upsert. 🟢 Program.cs (share endpoint) — the `/share/*` endpoint has no rate limiting. Since every failure returns 404 with a full HMAC verification per request, it's a mild DoS/oracle-timing surface. Not blocking given the constant-time compare, but worth noting for a public anonymous endpoint. 4. **Verdict**: **APPROVE** The generation-check-after-signature ordering, the `ValueGeneratedNever` on Paperless-assigned IDs, and the disabled-when-unset behavior are all correct. The flagged items are hardening improvements, not blockers. --- 📊 **Tokens**: 13619 input + 581 output | 💰 **Cost**: ~$0.0826 *Generated by Claude Opus 4.8 via Gitea Actions*
cheapnud merged commit eb23421f8c into master 2026-08-15 19:09:23 +00:00
cheapnud deleted branch d/share-links 2026-08-15 19:09:24 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
cheapnud/CheapClerk!5
No description provided.