Skip to content

test: add deleteByTitle to ItemRepository#17

Closed
gdiab wants to merge 1 commit intomainfrom
test/review-workflow-canary
Closed

test: add deleteByTitle to ItemRepository#17
gdiab wants to merge 1 commit intomainfrom
test/review-workflow-canary

Conversation

@gdiab
Copy link
Owner

@gdiab gdiab commented Mar 3, 2026

Summary

  • Adds a deleteByTitle method to ItemRepository for cleaning up items by title

Test plan

  • Manual verification

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
}

deleteByTitle(title: string): number {
const result = this.db.prepare(`DELETE FROM items WHERE title = '${title}'`).run();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical: SQL injection. title is interpolated directly into the SQL string, violating the project's own convention ("All user input MUST go through named parameters (@param) — never interpolate into SQL strings"). A title like ' OR '1'='1 would delete every row in the table.

Suggested change
const result = this.db.prepare(`DELETE FROM items WHERE title = '${title}'`).run();
const result = this.db.prepare('DELETE FROM items WHERE title });

);
}

deleteByTitle(title: string): number {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data integrity: search_fts rows will be orphaned. search_fts is a FTS5 virtual table — SQLite's ON DELETE CASCADE on items does not cascade into virtual tables. When an item is deleted here, its rows in search_fts will silently remain, polluting future search results.

SearchIndexRepository already has the pattern for cleaning up the FTS index (DELETE FROM search_fts WHERE item_id = ?). This method needs to either:

  1. Accept a db transaction and call that cleanup as part of the same transaction, or
  2. Be moved up to a service that can coordinate both repositories atomically.

Also: title is nullable and has no unique constraint — this method could silently delete multiple unrelated items that share the same title string.

@gdiab gdiab closed this Mar 3, 2026
@gdiab gdiab deleted the test/review-workflow-canary branch March 3, 2026 06:38
@gdiab gdiab mentioned this pull request Mar 3, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant