chore: address read after write issue for secret folders #6056
+31
−28
Claude / Claude Code Review
completed
Apr 16, 2026 in 11m 47s
Code review found 2 potential issues
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | backend/src/services/secret-folder/secret-folder-service.ts:504-528 |
updateFolder: oldFolderPath always equals newFolderPath after rename |
| 🟣 Pre-existing | backend/src/services/secret-folder/secret-folder-service.ts:498-501 |
updateFolder: !doc null guard fires after doc properties are already accessed |
Annotations
Check warning on line 528 in backend/src/services/secret-folder/secret-folder-service.ts
claude / Claude Code Review
updateFolder: oldFolderPath always equals newFolderPath after rename
In `updateFolder`, `doc.id === folder.id` because a SQL UPDATE preserves the row's primary key — only `name`/`description` change. The PR calls `findSecretPathByFolderIds(projectId, [doc.id, folder.id], tx)` with two identical IDs inside the same transaction (after the UPDATE), so both lookups resolve to the same post-rename path, making `oldFolderPath === newFolderPath` whenever a folder is renamed. Current route handlers only use `old.name` for audit logging (not `old.path`), so there is no im
Check notice on line 501 in backend/src/services/secret-folder/secret-folder-service.ts
claude / Claude Code Review
updateFolder: \!doc null guard fires after doc properties are already accessed
The `\!doc` null guard in `updateFolder`'s transaction body fires after `doc.name`, `doc.envId`, `doc.version`, `doc.id`, and `doc.description` are already accessed in `folderVersionDAL.create` and `folderCommitService.createCommit`. If `folderDAL.update` matches no rows and `doc` is `undefined`, a `TypeError` is thrown instead of the intended `NotFoundError`. This is a pre-existing bug that this PR did not introduce or worsen — the guard was already misplaced before the PR and remains so after.
Loading