Skip to content

Reject NUL characters in the Lidarr root folder with a 400 - #217

Merged
rowkav09 merged 1 commit into
mainfrom
reject-nul-in-lidarr-root-folder
Oct 2, 2026
Merged

rowkav09 merged 1 commit into
mainfrom
reject-nul-in-lidarr-root-folder

Conversation

@rowkav09

@rowkav09 rowkav09 commented Oct 2, 2026

Copy link
Copy Markdown
Member

Same class as #215 and #216. rootFolderPath from POST /api/v1/settings/lidarr is stored in Postgres, which cannot hold NUL, so a payload containing \u0000 passed validation, contacted the Lidarr server, and then failed in the upsert with a 500. The schema now refuses NUL, so the request gets the existing 400 INVALID_REQUEST before any network call or query. Test: on main the request is not rejected with 400 and reaches the network call; with the change it answers 400 and neither fetch nor the database is touched. This route had no api test before. Behaviour change: invalid payload returns 400 instead of 500.

@rowkav09

rowkav09 commented Oct 2, 2026

Copy link
Copy Markdown
Member Author

APPROVE at head 240e8d4.

Checked out the head (origin/main is an ancestor, stacked on #216). rootFolderPath now goes through the shared noNul refine, and both routes that parse LidarrConnectionRequestSchema (save at server.ts:1070 and test at :1096) pick it up. Full suite 366/366 passes, tsc is clean. Fail-first: with main's lidarr.ts under the PR's test, the new NUL test fails (400 expected); with the change it passes.

CI at this head when I looked: CodeFactor and ghostdeps succeeded, "Verify TypeScript workspace" was still in progress, so not CI-green yet. Same-account review.

@codecov

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@rowkav09
rowkav09 merged commit 8adf97f into main Oct 2, 2026
9 checks passed
@rowkav09
rowkav09 deleted the reject-nul-in-lidarr-root-folder branch October 2, 2026 15:20
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