6053e1ee4c531ea8906b9b8bb291300295ebf202
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
4822291707 |
fix(git-wrappers): #865 round-7 addendum — conservative YAML recognizer + review/comment hardening
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Fold the remaining round-7 audit items into the tea-CLI comment-invocation fix. Fallback token parser (detect-platform.sh, test-gitea-login-resolution.sh): Replace the line-by-line scalar fallback with a strict CONSERVATIVE block-YAML recognizer that reconstructs the same object PyYAML would or fails closed the instant it meets anything outside the tea-config subset. Closes 5 structural fail-open classes the old parser missed (nested-shadow logins, block-scalar shadow, duplicate root key / login name / token field, malformed-after-valid, extra-document / end-marker). Validated by a 360k-check differential fuzz vs real PyYAML (0 fail-open) plus explicit forced-PyYAML-absence fixtures. ITEM 1 (pr-review.sh) current-head TOCTOU: after the exact review-id read-back succeeds, re-read the live PR head and fail closed if it advanced past the submitted commit_id, so a review is never reported as covering a superseded tip. ITEM 2 (pr-review.sh comment action): require the returned resource be a pull_request (populated pull_request_url); reject a bare issue_url so a plain issue #N cannot masquerade as a verified PR comment. issue-comment.sh keeps its broader issue-or-PR acceptance. ITEM 3a (both wrappers): move the Authorization bearer OUT of curl argv into a private mode-0600 curl --config file (gitea_write_auth_config), removed on every exit path, so the token never appears in the process table. ITEM 3b (pr-review.sh): bind the review body with presence + string-type + exact equality instead of `(body or "")`, so a non-empty submitted body persisted as null/missing fails closed. Tests: add race, plain-issue, argv-capture (no token printed), and null-body fixtures; broaden temp-leak checks to the auth-config files. Full gate set green (bash -n, shellcheck -x -S warning, prettier, all 3 REST/resolution suites with PyYAML and forced-absent, cold TURBO_FORCE turbo 14/14). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
99856c5567 |
fix(git-tools): scope-aware YAML fallback, port-bound creds, origin-pinned URL + review-body verification (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Round-6 remediation for PR #866 addressing four cross-host exact-diff audit blockers (REQUEST_CHANGES governs): Blocker 1 (detect-platform.sh get_gitea_token_for_login line parser): the PyYAML-absence fallback attributed any `key: value` at any depth to the current login, so a token from a nested sub-map or a mis-indented line could be selected where PyYAML fails closed, and inline comments were not stripped. The fallback is now scope-aware — a field attaches only at the entry's own direct-field indentation, only list items at the login list's own dash indent open an entry — and _strip_scalar strips a trailing inline comment like PyYAML. It is therefore only ever MORE conservative than PyYAML, never less. Blocker 2 (detect-platform.sh host bind): credential binding compared parsed.hostname only, dropping the port, so a :9443 login satisfied a portless host and a matching :8443 login was rejected. Binding now normalizes scheme + host + effective port (scheme default applied symmetrically) exactly like gitea_url_matches_host. Blocker 3 (issue-comment.sh + pr-review.sh read-back URL check): verification used path.endswith, accepting a look-alike host or a decoy path prefix. It now pins the returned issue_url/pull_request_url ORIGIN (scheme+host+effective-port) and FULL path (deployment prefix + exact owner/repo + kind + number). A new GITEA_WEB_BASE is exported from gitea_resolve_api_for_login for this. Blocker 4 (pr-review.sh gitea_submit_review_verified): the submitted review body was not verified, so a finalized/reused pending review id carrying foreign Content passed. The persisted body is now bound to the exact submitted body. Tests: added forced-PyYAML-absence parser-equivalence fixtures (nested sub-map, sibling, mis-indent, inline comment, tab-indent fail-closed, port match/mismatch) to test-gitea-login-resolution.sh; URL-forgery fail-closed cases (wrong-host/owner/repo + prefix injection) to both write suites; and a reused-review-id body-mismatch case to the pr-review suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
2bb3ac4549 |
fix(git-tools): env-robust token parse, host-bound creds, real Gitea URL verify (#865 round-5)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
CI-red root cause (classification a: my round-4 change fails in the clean/cold CI env): get_gitea_token_for_login hard-required PyYAML (`import yaml`), which is absent on CI's node:24-alpine (python3 without py3-yaml). Round-4's --login override cases were the first to exercise that path, turning the mosaic package test (test:framework-shell -> test-pr-review-gitea-comment.sh) RED. Fix: add an indentation-aware line-parser fallback that resolves the SAME per-name token PyYAML would from tea's flat `logins:` list; PyYAML stays the fast path. This also repairs a latent production defect (--login overrides were silently unusable on any PyYAML-less host). Auditor blockers folded into the same round-5: 1. issue_url vs pull_request_url shape (correctness): Gitea populates WEB (html) URLs in issue_url/pull_request_url, not API paths, and a PR-conversation comment carries pull_request_url (issue_url empty). Verification now accepts either web shape scoped to the repo slug + number, so a durable write is never rejected for URL shape. Test stubs now emit the REAL Gitea web shapes. 2. Cross-host credential binding (security): get_gitea_token_for_login now takes the repo host and requires the matched login's configured URL host to equal it; an override login configured for a different host FAILS CLOSED instead of sending a cross-host credential. Regression tests added to both suites. 3. Non-exhaustive enumeration (false-fail): removed the redundant, non-exhaustive post-verification list enumeration (gitea_fetch_all + confirm_*_enumerable) from both wrappers; the exact-id GET is authoritative. Pagination cases dropped; a guard asserts no list enumeration is performed. 4. Trap clobbering / temp-file leak (security/hygiene): removing the nested enumeration eliminates the RETURN-trap nesting that clobbered caller cleanup; remaining RETURN traps are single/non-nested and clean up on all exit paths. Temp-file leak regression tests (success + failure paths) added to both suites. 5. README: corrected the exhaustive-pagination claim and documented host-bound --login selection. Preserves every round-2/3/4 fix (explicit --login fail-closed at all write sites, token->identity attribution seam). Gates: cold `pnpm turbo run test --filter=@mosaicstack/mosaic` green (14/14); full test-*.sh suite green with AND without PyYAML; bash -n, shellcheck -x -S warning, prettier --check README clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
6168f9ac86 |
fix(git-tools): fail closed on unresolvable explicit --login override (#865)
Some checks failed
ci/woodpecker/pr/ci Pipeline failed
gitea_resolve_api_for_login silently fell back to the host-default identity whenever the named login could not be resolved for ANY reason -- including when the name came from an EXPLICIT --login override. A caller passing a dedicated per-role credential could thus have its write attributed to the shared default identity while being told it succeeded as requested. Thread an "override was explicit" signal into gitea_resolve_api_for_login (second param, "explicit" when LOGIN_OVERRIDE is non-empty). When the override is explicit and that login's token cannot be resolved, FAIL CLOSED (return 1, clear error naming the login, no host-default fallback). The best-effort host-default fallback now applies ONLY on the no-override default path. Applied symmetrically to issue-comment.sh and all three pr-review.sh dispatch sites (approve / request-changes / comment). Tests: both scripts' write flows now assert credential attribution via a token->identity seam in the curl stub -- (a) resolvable --login override drives the entire write/read-back chain under THAT login's token, nothing under the default; (b) unresolvable --login override fails closed (nonzero, no success line, no write, no default-identity request); (c) no-override default path still succeeds under the host-default best-effort credential. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
9384f0bc0a |
fix(tools): write Gitea reviews/comments via REST POST and verify by exact created id (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Replace the tea-based write + boundary/author read-back with a direct Gitea
REST POST that returns the created record's id, and verify that exact record.
BLOCKER 2 (credential ordering): resolve the acting identity, the write token,
and the read-back token from the SAME effective login. A --login override now
selects the credential used for the POST, GET /user, and the GET-by-id
read-back, so an overridden write is verified against the identity that
performed it -- not the host default. Login-name resolution is best-effort and
non-fatal (the override always wins; otherwise fall back to the host
credential), so exotic/ported hosts still resolve a token.
BLOCKER 1+3 (attribution + tautological tests): the write is now
POST /issues/{n}/comments or POST /pulls/{n}/reviews (event + body + commit_id
== PR head), parsing the provider-returned created id. Verification GETs that
exact id and checks author == acting identity and body (comments) or state +
commit_id (reviews). Keying on the created id closes the concurrency window:
a no-op create yields no id and fails closed with no list-scan fallback, and a
concurrent same-identity record has a different id. The review body travels in
the review submit, removing the separate detached comment.
Tests: the curl stub now models a real server with persistent on-disk
review/comment state -- a POST actually creates+persists a record and returns
its id, and the read-back reads that same state (no fabricated record for the
wrapper to find). Adds same-identity no-op-concurrent and author-mismatch
fail-closed cases for both comments and reviews, and >page-1 pagination
coverage for both. README "Durable review provenance" refreshed for the REST
mechanism.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
|
16481ece3d |
fix(tools): attribute read-back to acting identity and paginate fully (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Round 2 remediation for the read-back verification in issue-comment.sh and pr-review.sh. BLOCKER A (invocation attribution): id-above-boundary + content/state match only proves temporal ordering — a concurrent write from a different identity could satisfy it while this tea invocation created nothing. Both wrappers now resolve the acting identity once via curl GET /api/v1/user and additionally require the accepted record's author login to equal that identity. Residual same-identity same-body/state concurrency is documented in-code (tea 0.11.1 emits no reliable created-record id to close it further). BLOCKER B (pagination): the comments and reviews list reads now walk every page (?limit=&page=1,2,… until a short/empty page) for both the pre-write boundary and the post-write read-back, so a record beyond page 1 is still found. Adds regressions: concurrent different-identity write fails closed (comments and reviews); a matching review beyond page 1 is still found. README updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
10fdd49e32 |
fix(tools): bound Gitea read-back to this write; verify approve/reject state (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Review remediation for two correctness holes in the #865 fix: BLOCKER 1 — issue-comment.sh read-back was body-only across all history: if `tea comment` silently no-opped (the #865 bug) while an identically bodied comment already existed from a prior run, the read-back matched the OLD comment and falsely reported success. Now record the pre-write maximum comment id as a boundary and require a comment with id > boundary AND exact body match; monotonic Gitea ids make id > boundary mean "created by this write". Fails closed otherwise. BLOCKER 2 — pr-review.sh approve/reject trusted tea's exit code for the review STATE (same never-trust-exit-zero defect class as #865). Removed the TODO deferral and added a real bounded read-back: record the max review id before `tea pr approve`/`reject`, then require a review with id > boundary, the expected state (APPROVED / REQUEST_CHANGES), and commit_id equal to the PR's current head. Fails closed if absent. Tests: extended test-pr-review-gitea-comment.sh to model and assert the new review-state read-back (guardrails preserved, assertions added). Added test-issue-comment-readback.sh proving the pre-existing-identical-body false positive now fails closed and a genuinely new comment verifies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |