[codex] Validate affiliate offer tag types#419
Conversation
Greptile SummaryThis PR hardens affiliate offer tag validation by rejecting non-array
Confidence Score: 3/5Safe to merge only after confirming whether null tags can arrive from the database or API; if they can, the guard change will start rejecting previously-valid updates. The core logic protecting src/lib/affiliates/validation.ts — the null-tags guard; src/lib/affiliates/validation.test.ts — missing coverage for non-array inputs Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[validateOfferInput called] --> B{input.tags !== undefined?}
B -- No --> Z[skip tags validation]
B -- Yes --> C{Array.isArray?}
C -- No --> D[push 'tags must be an array']
C -- Yes --> E{length > 10?}
E -- Yes --> F[push 'Maximum 10 tags']
E -- No --> G{any tag typeof !== 'string'?}
F --> G
G -- Yes --> H[push 'tags must be strings']
G -- No --> I{errors.length > 0?}
H --> I
D --> I
I -- Yes --> J[return ok:false]
I -- No --> K["sanitized: tags.map(t => t.trim().toLowerCase()).filter(Boolean)"]
K --> L[return ok:true]
style D fill:#f99,stroke:#c00
style H fill:#f99,stroke:#c00
style B fill:#ffe,stroke:#aa0
Reviews (1): Last reviewed commit: "fix: validate affiliate offer tag types" | Re-trigger Greptile |
| if (input.tags !== undefined) { | ||
| if (!Array.isArray(input.tags)) { | ||
| errors.push("tags must be an array"); | ||
| } else { | ||
| if (input.tags.length > 10) { | ||
| errors.push("Maximum 10 tags"); | ||
| } | ||
| if (input.tags.some((tag) => typeof tag !== "string")) { | ||
| errors.push("tags must be strings"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
null tags now rejected as a non-array
The guard changed from if (input.tags && ...) (truthy check) to if (input.tags !== undefined), so a null value now enters the branch and triggers "tags must be an array". null is a realistic runtime value: JSON round-trips, database reads, or existing API callers may pass tags: null when no tags are set. Previously that was silently accepted; now it becomes a validation error that breaks those callers. The sanitization path at line 153 already handles null correctly via optional chaining (input.tags?.map(...) || []), making this inconsistency easy to miss. Consider using input.tags != null (loose inequality) or explicitly handling the null case alongside undefined to preserve the prior behavior.
| it("rejects non-string tags before sanitizing", () => { | ||
| const result = validateOfferInput({ | ||
| ...validInput, | ||
| tags: ["valid", 123] as any, | ||
| }); | ||
| expect(result.ok).toBe(false); | ||
| expect(result.errors.some((e) => e.includes("tags") && e.includes("strings"))).toBe(true); | ||
| }); |
There was a problem hiding this comment.
No test for non-array
tags value
The PR description says "reject non-array tags values," but the only new test uses an array that contains a non-string element (["valid", 123]). There is no test for the fully non-array case (e.g., tags: "string", tags: 42, or tags: null), leaving the new !Array.isArray(input.tags) branch untested. A test for tags: "not-an-array" would directly exercise the new guard and would also surface the null behavior described in the companion comment.
Summary
tagsvalues during affiliate offer validation.trim().toLowerCase()tags: ["valid", 123]Fixes #418.
Validation
npm.cmd run test:run -- src/lib/affiliates/validation.test.tsnpm.cmd run type-check