Security fix: migrate JSON field reads from jsonparser to jsoniter

[jbsmith7741]

• Add apps replace directive for local task-tools during development
• Resolves security issue around old version of github.com/buger/jsonparser

[qodo-code-review]

Review Summary by Qodo

Migrate JSON parsing from jsonparser to jsoniter for security

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Migrate JSON parsing from vulnerable jsonparser to jsoniter library
• Replace jsonparser.GetString calls with jsoniter.Get API
• Add type checking for StringValue in JSON field extraction
• Update go.mod dependencies and add local replace directive

Diagram

flowchart LR
  A["jsonparser library<br/>v1.1.1/v1.1.2"] -->|"migrate to"| B["jsoniter library<br/>v1.1.12"]
  C["KeyFromJSON function"] -->|"refactor"| D["Use jsoniter.Get<br/>with type checking"]
  E["jsonDateExtractor"] -->|"refactor"| F["Use jsoniter.Get<br/>with error handling"]
  G["go.mod dependencies"] -->|"update"| H["Remove jsonparser<br/>Add jsoniter"]

Loading

File Changes

1. dedup/dedup.go 🐞 Bug fix +6/-3

Migrate dedup JSON parsing to jsoniter

• Replace jsonparser import with jsoniter
• Refactor KeyFromJSON to use jsoniter.Get instead of jsonparser.GetString
• Add type checking to verify StringValue before concatenating
• Improve error handling by checking ValueType

dedup/dedup.go

---

2. file/writebyhour.go 🐞 Bug fix +7/-4

Migrate writebyhour JSON parsing to jsoniter

• Replace jsonparser import with jsoniter
• Refactor jsonDateExtractor.ExtractDate to use jsoniter.Get API
• Add explicit type checking for StringValue
• Improve error handling with LastError() and type validation

file/writebyhour.go

---

3. go.mod Dependencies +1/-2

Update dependencies from jsonparser to jsoniter

• Remove github.com/buger/jsonparser v1.1.2 dependency
• Add github.com/json-iterator/go v1.1.12 as direct dependency
• Move jsoniter from indirect to direct dependencies

go.mod

---

View more (4)

4. go.sum Dependencies +0/-2

Update checksums for dependency migration

• Remove jsonparser v1.1.2 hash entries
• Maintain jsoniter v1.1.12 hash entries

go.sum

---

5. apps/go.mod ⚙️ Configuration changes +2/-2

Update apps module dependencies and add replace directive

• Remove jsonparser v1.1.1 from indirect dependencies
• Remove golang.org/x/groupcache indirect dependency
• Add local replace directive for github.com/pcelvng/task-tools

apps/go.mod

---

6. apps/go.sum Dependencies +0/-4

Update apps module checksums

• Remove jsonparser v1.1.1 hash entries
• Remove task-tools v0.29.0 hash entries

apps/go.sum

---

7. go.work.sum Miscellaneous +3/-0

Update workspace sum file entries

• Add envoyproxy/go-control-plane v0.14.0 entry
• Add golang/glog v1.2.5 entry
• Add golang/term v0.38.0 entry

go.work.sum

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (1)   📎 Requirement gaps (0)   🎨 UX Issues (0)

🐞\ ☼ Reliability (1) ◔ Observability (1)

📘\ ≡ Correctness (1)

1. Committed local replace path 🐞 ☼

Description

apps/go.mod commits replace github.com/pcelvng/task-tools => ../, which forces resolution from a
relative filesystem path. Any build of the apps module outside this monorepo/workspace will fail
to resolve (or will compile against unintended local code), and it also masks the declared
github.com/pcelvng/task-tools v0.29.0 dependency.

Code

apps/go.mod[112]

+replace github.com/pcelvng/task-tools => ../

Evidence

The apps module hard-codes a relative replace to ../ while the repo already uses a Go workspace
that includes both modules; additionally, the apps module still declares a versioned dependency on
github.com/pcelvng/task-tools, meaning the new replace overrides (and hides) which version is
actually being built.

apps/go.mod[1-33]
apps/go.mod[112-112]
go.work[1-7]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`apps/go.mod` contains a committed `replace github.com/pcelvng/task-tools => ../`, which makes the `apps` module depend on a relative local filesystem layout and overrides the declared module version.

### Issue Context
This repo already has a `go.work` that includes both the root module and `apps`, so local development can use workspace resolution without committing a `replace`.

### Fix Focus Areas
- apps/go.mod[112-112]
- go.work[1-7]

### Expected fix
- Remove the `replace github.com/pcelvng/task-tools => ../` line from `apps/go.mod`.
- Ensure `apps/go.mod` depends on an appropriate released/pseudo-version of `github.com/pcelvng/task-tools` (or rely on `go.work` for local dev), and regenerate `apps/go.sum` accordingly (e.g., via `go mod tidy` in `/apps`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. KeyFromJSON ignores parse errors 📘 ≡

Description

KeyFromJSON uses jsoniter.Get and then proceeds based only on ValueType() without checking
val.LastError(), which can silently ignore malformed JSON or failed field lookups and produce
incorrect keys. This violates the requirement to explicitly handle errors from fallible operations
before using the result.

Code

dedup/dedup.go[R54-57]

+		val := jsoniter.Get(b, field)
+		if val.ValueType() == jsoniter.StringValue {
+			key += val.ToString()
+		}

Evidence

PR Compliance ID 1 requires explicit error checks for fallible operations before using returned
values. The added code calls jsoniter.Get and then concatenates val.ToString() without checking
val.LastError() anywhere in the loop.

dedup/dedup.go[54-57]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`KeyFromJSON` uses `jsoniter.Get` but never checks `val.LastError()` before relying on `ValueType()` and calling `val.ToString()`, which can mask parse/lookup failures.

## Issue Context
`jsoniter.Any` stores parse/lookup failures in `LastError()`. This function currently has no way to return an error, so it should explicitly handle `LastError()` (e.g., skip the field, return empty key, or otherwise safely handle failure) instead of silently proceeding.

## Fix Focus Areas
- dedup/dedup.go[54-57]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. JSON errors misclassified 🐞 ◔

Description

jsonDateExtractor.ExtractDate converts jsoniter parse errors into the same generic "field not in"
error used for missing/wrong-type fields. This hides the real failure mode (malformed JSON vs
missing field) and makes troubleshooting/alerting inaccurate.

Code

file/writebyhour.go[R267-273]

+	val := jsoniter.Get(b, p.fieldName)
+	if err := val.LastError(); err != nil {
+		return t, fmt.Errorf(`field "%v" not in '%v'`, p.fieldName, string(b))
+	}
+	if val.ValueType() != jsoniter.StringValue {
		return t, fmt.Errorf(`field "%v" not in '%v'`, p.fieldName, string(b))
	}

Evidence

The code checks val.LastError() but drops the underlying error and returns a misleading
missing-field message; it also returns the exact same message for the separate wrong-type case,
conflating distinct error conditions.

file/writebyhour.go[263-276]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`jsonDateExtractor.ExtractDate` discards the `val.LastError()` value and returns a generic "field not in" message, even when the JSON is malformed.

### Issue Context
There are two distinct error paths:
- JSON parsing failure (`val.LastError() != nil`)
- Field missing or not a string (`val.ValueType() != jsoniter.StringValue`)
These should not return the same error message.

### Fix Focus Areas
- file/writebyhour.go[265-276]

### Expected fix
- If `val.LastError()` is non-nil, return an error that includes/wraps the underlying parse error (e.g., `fmt.Errorf("invalid JSON: %w", err)` or `fmt.Errorf("failed reading field %q: %w", p.fieldName, err)`).
- Keep a separate message for missing-field / wrong-type cases (optionally distinguish missing vs wrong-type using `ValueType()`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more