fix(work-items): fail closed when a gitea label page is not an array

[kyle-sexton]

Closes #3439

Summary

The gitea adapter's label walk treated any 200 body as iterable. A proxy error page or auth-redirect object has jq length equal to its key count, so the walk accepted garbage instead of erroring.

Fix

Add wit_gitea_require_array in common.sh (jq 'type' == array) and call it after every label-page wit_gitea_require_ok in create-item.sh. A non-array 200 now exits 1 with expected a JSON array, does not request page 2, and does not POST an issue.

work-items 0.39.48 -> 0.39.49 (rebased onto #3647).

Verification

create-item.test.sh 49/49 and common.test.sh 69/69 after updating the #3484 fixture from expected exit 5 to exit 1.

Related

Refs #3647
Refs #3651
Refs #3484

  

[claude]

Claude finished @kyle-sexton's task in 1m 43s —— View job

---

Security review

• Gather context (PR diff, changed files)
• Skip-gate check
• Review changed files for security-relevant issues
• Post findings

No security issues found in this diff.

This PR is itself a security-hardening fix: wit_gitea_require_array (common.sh#L351-L363) type-checks a gitea label-page body against jq 'type' before treating it as an array, closing a fail-open path where a 200 proxy-error page or auth-redirect object ({"message":"..."}) was silently accepted as an empty/garbage label list instead of aborting.

Checked for the usual concerns and found nothing exploitable in this diff:

• Injection: the new check pipes $WIT_GITEA_BODY into a static jq 'type' filter via <<< — no untrusted data reaches the jq program text, so there's no jq/command injection surface.
• Fail-open bypass: the function exits at top level (not inside a $(...) subshell), so a failing type check actually terminates create-item.sh rather than being swallowed — verified this holds at all four new call sites (repo labels page 1/page N, org labels page 1/page N).
• Downstream use: label names/ids continue to flow through jq --arg/--argjson, so no new string-interpolation risk was introduced alongside the fix.
• Ordering: wit_gitea_require_array is correctly called after wit_gitea_require_ok at every site, and the org-labels 404 special case still skips both checks appropriately (no org labels present is not an error).

One out-of-scope observation, not a finding against this PR: list-items.sh has its own hand-rolled non-array guard (jq 'length' <<<"$WIT_GITEA_BODY" || exit) that doesn't actually catch this bug class, since jq length succeeds on a JSON object too (returning its key count) — the same shape of proxy-error body this PR defends against in create-item.sh would pass silently there. Worth a follow-up using the new wit_gitea_require_array helper, but it's a pre-existing gap in a file this PR doesn't touch.

[github-actions]

Last security-reviewed head: 89f0a28ec2214ba98247edd22bcde2b1db49ee60. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-02T10:12:17.375249Z	89f0a28	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 1m 25s —— View job

---

Security review — /review:security-review

• Gather context (gh pr diff, changed files)
• Diff against the last-reviewed head (ad4e0942) to isolate what's new
• Skip-gate check
• Post findings

Skip gate hit: no new security-relevant surface since the last review.

Comparing the current head (89f0a28) to the head this PR was last reviewed at
(ad4e0942, per the prior security-review comment), the security-relevant
production files are byte-identical:

• common.sh (wit_gitea_require_array) — unchanged
• create-item.sh — unchanged

The only delta since that review is in create-item.test.sh: the second
commit (89f0a28) removes a now-redundant standalone test case for the
non-array-label-page fixture and updates the remaining, equivalent test's
expected exit code from 5 to 1 and asserts the expected a JSON array
message plus the no-POST-issued invariant. This is test-only churn re-covering
the same fixture already exercised when the fix landed — no new logic, no new
input-handling path, nothing that changes the security posture already
assessed.

Per the skip-gate criteria (open PR, no new security-relevant surface, prior
review still applies), no further review is needed here. The prior review's
conclusion stands: no security issues found.

[claude]

Claude finished @kyle-sexton's task in 18s —— View job

---

Code review — skipped

• Gather context (PR diff, changed files, skip-gate check)

Skip gate hit: PR is closed (state = MERGED). Per the /review:code-review skill's skip gate ("PR is closed or not open"), no deep review runs and nothing further is posted.

For reference, this PR was already reviewed on this head SHA by the security lane (see the earlier comment above) with no issues found, and it has since merged.

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.