Feature 89/filtering by fields #95

Merged

noe.trevino merged 12 commits from feature-89/filtering-by-fields into main

2026-02-15 22:04:07 -06:00

noe.trevino commented

2026-02-15 20:10:17 -06:00

Owner

Checklist

Read CONTRIBUTING.md
Ran tests (go test ./... -race) locally
Formatted and linted with gofmt -w . and golangci-lint run ./... respectively
Added tests if necessary (significant changes/complex logic)
Updated documentation if applicable
Updated README.md if applicable

Summary

Filterable options
Closes #89
Closes #49
Closes #90
Closes #91
Closes #92
Closes #93
Closes #94

## Checklist - [x] Read `CONTRIBUTING.md` - [x] Ran tests (`go test ./... -race`) locally - [x] Formatted and linted with `gofmt -w .` and `golangci-lint run ./...` respectively - [x] Added tests if necessary (significant changes/complex logic) - [x] Updated documentation if applicable - [x] Updated `README.md` if applicable ## Summary  Filterable options Closes #89 Closes #49 Closes #90 Closes #91 Closes #92 Closes #93 Closes #94

claude[bot] (Migrated from github.com) reviewed

2026-02-15 20:12:00 -06:00

internal/services/task/service.go

Security: Potential SQL injection via label IDs

The buildLabelIdsCsv function constructs a CSV string from user-provided label IDs using fmt.Sprintf("%d", id) without validation. While the current implementation expects integers, the SQL queries use this string in a position() or instr() call with string concatenation.

If a caller passes non-integer values or manipulates the LabelIDs slice, this could potentially be exploited. Although the current code path appears safe (IDs come from the database), this pattern is fragile.

Recommendation:
Add validation to ensure all label IDs are positive integers before constructing the CSV, or use a safer approach like parameterized array queries if the database supports them.

**Security: Potential SQL injection via label IDs** The `buildLabelIdsCsv` function constructs a CSV string from user-provided label IDs using `fmt.Sprintf("%d", id)` without validation. While the current implementation expects integers, the SQL queries use this string in a `position()` or `instr()` call with string concatenation. If a caller passes non-integer values or manipulates the `LabelIDs` slice, this could potentially be exploited. Although the current code path appears safe (IDs come from the database), this pattern is fragile. **Recommendation:** Add validation to ensure all label IDs are positive integers before constructing the CSV, or use a safer approach like parameterized array queries if the database supports them.

claude[bot] commented

2026-02-15 20:12:11 -06:00

(Migrated from github.com)

Review Summary

Found 1 security concern related to the label ID CSV construction in the filter query.

Issues Found:

Security concern in buildLabelIdsCsv (service.go:789-801) - The function constructs a CSV string from label IDs without validation, creating a potential SQL injection risk if non-integer values are passed.

Code Quality Assessment:

✅ Guard patterns are appropriate
✅ No clear DRY violations
✅ Good test coverage for the new filtering functionality
✅ Proper separation of concerns between TUI and service layer
✅ Consistent naming conventions (renamed from FilteredTaskSummary to WithFiltersTaskSummary)
✅ SQL queries use proper NULL handling for optional filters

Score: 8/10

The implementation is solid overall with good architectural decisions. The unified filter query design is clean and the UI integration is well-structured. The one security concern should be addressed to ensure robust input validation.

## Review Summary Found 1 security concern related to the label ID CSV construction in the filter query. ### Issues Found: 1. **Security concern** in `buildLabelIdsCsv` (service.go:789-801) - The function constructs a CSV string from label IDs without validation, creating a potential SQL injection risk if non-integer values are passed. ### Code Quality Assessment: - ✅ Guard patterns are appropriate - ✅ No clear DRY violations - ✅ Good test coverage for the new filtering functionality - ✅ Proper separation of concerns between TUI and service layer - ✅ Consistent naming conventions (renamed from `FilteredTaskSummary` to `WithFiltersTaskSummary`) - ✅ SQL queries use proper NULL handling for optional filters ### Score: 8/10 The implementation is solid overall with good architectural decisions. The unified filter query design is clean and the UI integration is well-structured. The one security concern should be addressed to ensure robust input validation.

noe.trevino reviewed

2026-02-15 22:00:33 -06:00

internal/services/task/service.go