rosa/vernier

Fork 0

domain: Micropub category add bypasses the 25-tag cap and bricks the post #51

New issue

Closed

opened 2026-07-03 01:41:44 +00:00 by rosa · 2 comments

rosa commented

2026-07-03 01:41:44 +00:00

Owner

Found during a code review of the domain crate.

Location: crates/domain/src/models/micropub.rs:102 (MicropubUpdate::apply_to)
Severity: Correctness — permanent data corruption

Problem

apply_to merges add categories with no dedupe and no 25-tag cap. PostEdit, update_post, and the non-transactional Postgres UPDATE do not re-validate, so a post pushed to 26 tags is committed, then fails Post::try_from (TooManyTags) on every subsequent read.

Failure scenario

A post already at 25 tags receives a Micropub update adding one category. The UPDATE commits 26 tags, the call returns TooManyTags, and every later find_post_by_slug/feed load of that row fails Post::new — the post is unreadable and unfixable through the domain.

Suggested fix

Enforce the dedupe and cap in apply_to/PostEdit before the write (mirroring Post::new), so an over-cap update is rejected before it persists.

Found during a code review of the `domain` crate. **Location:** `crates/domain/src/models/micropub.rs:102` (`MicropubUpdate::apply_to`) **Severity:** Correctness — permanent data corruption ## Problem `apply_to` merges `add` categories with no dedupe and no 25-tag cap. `PostEdit`, `update_post`, and the non-transactional Postgres UPDATE do not re-validate, so a post pushed to 26 tags is committed, then fails `Post::try_from` (TooManyTags) on every subsequent read. ## Failure scenario A post already at 25 tags receives a Micropub update adding one category. The UPDATE commits 26 tags, the call returns TooManyTags, and every later `find_post_by_slug`/feed load of that row fails `Post::new` — the post is unreadable and unfixable through the domain. ## Suggested fix Enforce the dedupe and cap in `apply_to`/`PostEdit` before the write (mirroring `Post::new`), so an over-cap update is rejected before it persists.

rosa added the

needs-triage

label

2026-07-03 01:43:53 +00:00

rosa commented

2026-07-03 16:12:55 +00:00

Author

Owner

This was generated by AI during triage.

Verified during triage: reproduced at the domain level. A Post at 25 tags plus a MicropubUpdate whose CategoryUpdate::add holds one new tag produces a 26-tag PostEdit from apply_to, and a Post rebuilt from those tags fails Post::new with TooManyTags. The write path commits before that failure surfaces, so the row persists and every later read of the post fails.

Triage also found the same gap on two more paths, so the scope of this issue is all writes, not just Micropub update:

The web editor's edit form builds a PostEdit from comma-separated tag input that validates each tag individually but never counts or dedupes the set.
Both create paths (web form and Micropub create) hand tags to the repository insert without ever constructing a Post, so an over-cap create bricks the post from birth.

Agent Brief

Category: bug
Summary: Enforce the tag dedupe + 25-tag cap on every post write path so an over-cap tag set is rejected before it persists

Current behavior:
Post::new dedupes tags and rejects sets larger than 25 with DomainError::TooManyTags, but it only runs when a post is read back (via Post::try_from on a database row). The write paths never apply the rule:

MicropubUpdate::apply_to merges CategoryUpdate::add into the existing tags with no dedupe and no cap.
The web editor form builds PostEdit.tags from Tag::parse_comma_separated, which has no set-level validation.
create_post passes PostCreate.tags straight to the repository insert.

The repository UPDATE/INSERT commits, then the RETURNING row fails Post::try_from. The caller gets an opaque error, the over-cap row is already persisted, and every subsequent load of that post fails TooManyTags — the post is permanently unreadable and cannot be repaired through the domain.

Desired behavior:
An edit or create whose deduplicated tag set exceeds 25 is rejected with a domain error before anything is written. Duplicates are collapsed before counting (mirroring Post::new, where 50 raw tags deduping to 25 distinct is valid). A Micropub add of a category the post already has is a no-op, not a step toward the cap. Rejections surface as proper validation errors to the client (Micropub error response / form error), not as an opaque internal error. Existing posts and in-range updates behave exactly as before.

Key interfaces:

The dedupe + cap rule currently lives only inside Post::new. Centralize it so Post::new, PostEdit, and PostCreate all share one implementation — e.g. a validated tag-collection value object or a shared constructor/validation function — rather than copying the check into each call site.
MicropubUpdate::apply_to already returns Result<PostEdit, ApplyUpdateError>; an over-cap merge should become a new ApplyUpdateError variant that maps to an appropriate MicropubError (invalid_request) in the service layer.
The web form extractor that parses comma-separated tags should reject an over-cap set with a DomainError like the per-tag errors it already returns.
The repository's update_post/create_post may keep the Post::try_from on the returned row as a last-resort integrity check, but it must no longer be the only guard.

Acceptance criteria:

A Micropub update adding a category to a post already at 25 distinct tags is rejected with a validation error and the post's stored tags are unchanged.
A Micropub update whose add duplicates existing categories (deduped set ≤ 25) succeeds.
A Micropub replace with more than 25 distinct categories is rejected before the write.
Submitting the web editor edit form with more than 25 distinct tags is rejected with a form validation error, not a 500.
Creating a post (web form or Micropub) with more than 25 distinct tags is rejected before the insert.
The dedupe + cap logic exists in exactly one place, shared by construction of Post, PostEdit, and PostCreate.
The ci mise task passes.

Out of scope:

Changing the 25-tag limit or the 50-char per-tag limit.
Repairing rows already persisted with more than 25 tags (no such rows are known; a migration is not needed).
Making the repository update transactional or otherwise restructuring the persistence layer beyond the validation guard.
Tag normalization beyond the existing exact-match dedupe (e.g. case folding).

> *This was generated by AI during triage.* Verified during triage: reproduced at the domain level. A `Post` at 25 tags plus a `MicropubUpdate` whose `CategoryUpdate::add` holds one new tag produces a 26-tag `PostEdit` from `apply_to`, and a `Post` rebuilt from those tags fails `Post::new` with `TooManyTags`. The write path commits before that failure surfaces, so the row persists and every later read of the post fails. Triage also found the same gap on two more paths, so the scope of this issue is **all writes**, not just Micropub update: - The web editor's edit form builds a `PostEdit` from comma-separated tag input that validates each tag individually but never counts or dedupes the set. - Both create paths (web form and Micropub create) hand tags to the repository insert without ever constructing a `Post`, so an over-cap create bricks the post from birth. ## Agent Brief **Category:** bug **Summary:** Enforce the tag dedupe + 25-tag cap on every post write path so an over-cap tag set is rejected before it persists **Current behavior:** `Post::new` dedupes tags and rejects sets larger than 25 with `DomainError::TooManyTags`, but it only runs when a post is *read back* (via `Post::try_from` on a database row). The write paths never apply the rule: - `MicropubUpdate::apply_to` merges `CategoryUpdate::add` into the existing tags with no dedupe and no cap. - The web editor form builds `PostEdit.tags` from `Tag::parse_comma_separated`, which has no set-level validation. - `create_post` passes `PostCreate.tags` straight to the repository insert. The repository UPDATE/INSERT commits, then the `RETURNING` row fails `Post::try_from`. The caller gets an opaque error, the over-cap row is already persisted, and every subsequent load of that post fails `TooManyTags` — the post is permanently unreadable and cannot be repaired through the domain. **Desired behavior:** An edit or create whose *deduplicated* tag set exceeds 25 is rejected with a domain error before anything is written. Duplicates are collapsed before counting (mirroring `Post::new`, where 50 raw tags deduping to 25 distinct is valid). A Micropub `add` of a category the post already has is a no-op, not a step toward the cap. Rejections surface as proper validation errors to the client (Micropub error response / form error), not as an opaque internal error. Existing posts and in-range updates behave exactly as before. **Key interfaces:** - The dedupe + cap rule currently lives only inside `Post::new`. Centralize it so `Post::new`, `PostEdit`, and `PostCreate` all share one implementation — e.g. a validated tag-collection value object or a shared constructor/validation function — rather than copying the check into each call site. - `MicropubUpdate::apply_to` already returns `Result<PostEdit, ApplyUpdateError>`; an over-cap merge should become a new `ApplyUpdateError` variant that maps to an appropriate `MicropubError` (invalid_request) in the service layer. - The web form extractor that parses comma-separated tags should reject an over-cap set with a `DomainError` like the per-tag errors it already returns. - The repository's `update_post`/`create_post` may keep the `Post::try_from` on the returned row as a last-resort integrity check, but it must no longer be the *only* guard. **Acceptance criteria:** - [ ] A Micropub update adding a category to a post already at 25 distinct tags is rejected with a validation error and the post's stored tags are unchanged. - [ ] A Micropub update whose `add` duplicates existing categories (deduped set ≤ 25) succeeds. - [ ] A Micropub `replace` with more than 25 distinct categories is rejected before the write. - [ ] Submitting the web editor edit form with more than 25 distinct tags is rejected with a form validation error, not a 500. - [ ] Creating a post (web form or Micropub) with more than 25 distinct tags is rejected before the insert. - [ ] The dedupe + cap logic exists in exactly one place, shared by construction of `Post`, `PostEdit`, and `PostCreate`. - [ ] The `ci` mise task passes. **Out of scope:** - Changing the 25-tag limit or the 50-char per-tag limit. - Repairing rows already persisted with more than 25 tags (no such rows are known; a migration is not needed). - Making the repository update transactional or otherwise restructuring the persistence layer beyond the validation guard. - Tag normalization beyond the existing exact-match dedupe (e.g. case folding).

rosa added

bug

ready-for-agent

and removed

needs-triage

labels

2026-07-03 16:13:05 +00:00

rosa referenced this issue from a commit

2026-07-03 16:30:33 +00:00

fix: Enforce the tag dedupe and 25-cap on every post write path

rosa referenced this issue from a pull request that will close it,

2026-07-03 16:33:15 +00:00

fix: Enforce the tag dedupe and 25-cap on every post write path #73

rosa closed this issue

2026-07-03 17:16:42 +00:00

rosa commented

2026-07-03 17:17:51 +00:00

Author

Owner

This was generated by AI during triage.

Fixed by PR #73 (merged): the tag dedupe + 25-cap now lives in Tag::dedupe_and_cap (with the cap as Tag::MAX_PER_POST) and is enforced before the write on every path — Micropub update (add and replace), Micropub create, and the web editor's new/edit forms. Over-cap writes are rejected with a validation error (Micropub invalid_request / form 400) instead of committing a row that bricks the post. Integration tests cover the original failure scenario and assert the stored post is untouched after a rejected update.

> *This was generated by AI during triage.* Fixed by PR #73 (merged): the tag dedupe + 25-cap now lives in `Tag::dedupe_and_cap` (with the cap as `Tag::MAX_PER_POST`) and is enforced before the write on every path — Micropub update (`add` and `replace`), Micropub create, and the web editor's new/edit forms. Over-cap writes are rejected with a validation error (Micropub `invalid_request` / form 400) instead of committing a row that bricks the post. Integration tests cover the original failure scenario and assert the stored post is untouched after a rejected update.