Skip to content

fix(review): prevent backdated duplicate-winner claims - #3956

Merged
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-duplicate-winner-vulnerability
Jul 7, 2026
Merged

fix(review): prevent backdated duplicate-winner claims#3956
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-duplicate-winner-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The duplicate-winner election was vulnerable because createdAt (GitHub PR creation time) could override the observed linkedIssueClaimedAt, letting older placeholder PRs that later edit in an issue steal winner credit from the PR that actually claimed the issue first.

Description

  • Restore claim-time ordering by changing the election to use linkedIssueClaimedAt exclusively for winner comparisons and deliberately ignore createdAt when deciding cluster winners.
  • Retain the createdAt field on DuplicateClaimMember for caller compatibility but document it is not used for winner ordering.
  • Update the exported wrapper doc and the engine implementation and add regression tests that exercise the anti-backdating scenario and the duplicate-count/winner-number seams.
  • Files changed: packages/gittensory-engine/src/duplicate-winner.ts, packages/gittensory-engine/src/signals/duplicate-winner.ts, src/signals/duplicate-winner.ts, and test/unit/duplicate-winner.test.ts.

Testing

  • Ran git diff --check which produced no issues and is clean.
  • Ran npm run typecheck which passed with no TypeScript errors.
  • Ran npx vitest run test/unit/duplicate-winner.test.ts and all tests passed (39 tests passed).
  • Ran npm run build:miner which completed successfully.
  • Attempted npm run test:coverage -- test/unit/duplicate-winner.test.ts; the tests executed successfully but coverage remapping failed with TypeError: jsTokens is not a function from the local coverage provider (tests themselves passed).
  • Attempted npm audit --audit-level=moderate but the audit endpoint returned 403 Forbidden, so the dependency-review step could not complete locally.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 7, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 651a236 Commit Preview URL

Branch Preview URL
Jul 07 2026, 07:42 AM

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
10938 1 10937 7
View the top 1 failed test(s) by shortest run time
test/unit/predicted-gate-engine-coverage.test.ts > predicted-gate engine module coverage (#2283) > exercises duplicate-winner election helpers
Stack Traces | 0.03s run time
AssertionError: expected false to be true // Object.is equality

- Expected
+ Received

- true
+ false

 ❯ test/unit/predicted-gate-engine-coverage.test.ts:109:155

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-07 08:55:56 UTC

4 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI failing · blocked

🛑 Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR correctly reverts a prior createdAt-based ordering rule in the duplicate-winner election, restoring pure linkedIssueClaimedAt comparison to close a backdating exploit where an older placeholder PR could edit in a linked issue later and steal winner credit via its earlier GitHub creation time. I traced prPrecedesSibling in packages/gittensory-engine/src/duplicate-winner.ts and its signals/ mirror: the createdAt comparison block is fully removed, both branches (siblingClaim < prClaim, equal-timestamp tie-break) are preserved and covered by tests, and the sparse-legacy fail-closed behavior (null/invalid claim time) is retained and tested. The DuplicateClaimMember.createdAt field is correctly kept for caller compatibility (src/queue/processors.ts still threads it through) rather than removed, avoiding a type-break, and the two engine-package copies of the file were edited identically so they don't drift.

Nits — 5 non-blocking
  • The new 'SECOND CONSUMER (feat(miner-foundation): extract duplicate-winner adjudication into gittensory-engine #2278)' doc block in both duplicate-winner.ts files speculates about a future miner adjudication use case unrelated to this bug fix — consider trimming it to keep the diff focused on the anti-backdating change.
  • The PR description doesn't cite a 'Fixes #N' issue link even though this reverts a documented prior election rule (#dup-winner true-creation-time) — worth linking the tracking issue for this regression per repo convention.
  • packages/gittensory-engine/src/duplicate-winner.ts and packages/gittensory-engine/src/signals/duplicate-winner.ts remain fully duplicated files kept in sync by hand; a future edit could drift them again — consider a comment flagging the pairing requirement if one doesn't already exist elsewhere.
  • Confirm CHANGELOG or issue tracker cross-reference exists for the reverted #dup-winner true-creation-time behavior so downstream consumers relying on the old createdAt precedence are aware of the semantic flip.
  • Since src/queue/processors.ts (dupWinnerLinkedDuplicateCount/dupWinnerLinkedDuplicateWinnerNumber) still accepts a createdAt-shaped parameter per the updated tests, add a short inline note there too pointing to the engine module's ELECTION ORDER doc so future readers don't reintroduce createdAt-based ordering at the call site.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

CI checks failing

  • validate
  • validate-code
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 343 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 343 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 343 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 7, 2026
@JSONbored
JSONbored merged commit ddd7e51 into main Jul 7, 2026
8 of 10 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-duplicate-winner-vulnerability branch July 7, 2026 09:47
JSONbored added a commit that referenced this pull request Jul 7, 2026
Three independent test/doc mismatches accumulated on main from recently
merged PRs, each blocking every subsequent PR's validate-code:

- queue.test.ts: 4 disposition-metric assertions predated #3946's repo
  label redaction and still expected the unredacted literal repo name.
- predicted-gate-engine-coverage.test.ts: 3 duplicate-winner assertions
  predated #3956's anti-backdating fix and asserted the old createdAt-
  based ordering instead of the new claim-time-only, fail-closed one.
- gittensory.full.yml: missing the review.shared_config (#2046) doc
  section that .gittensory.yml.example already had, breaking their
  required byte-identical-from-marker sync.
JSONbored added a commit that referenced this pull request Jul 7, 2026
Three independent test/doc mismatches accumulated on main from recently
merged PRs, each blocking every subsequent PR's validate-code:

- queue.test.ts: 4 disposition-metric assertions predated #3946's repo
  label redaction and still expected the unredacted literal repo name.
- predicted-gate-engine-coverage.test.ts: 3 duplicate-winner assertions
  predated #3956's anti-backdating fix and asserted the old createdAt-
  based ordering instead of the new claim-time-only, fail-closed one.
- gittensory.full.yml: missing the review.shared_config (#2046) doc
  section that .gittensory.yml.example already had, breaking their
  required byte-identical-from-marker sync.
JSONbored added a commit that referenced this pull request Jul 7, 2026
…4019)

Three independent test/doc mismatches accumulated on main from recently
merged PRs, each blocking every subsequent PR's validate-code:

- queue.test.ts: 4 disposition-metric assertions predated #3946's repo
  label redaction and still expected the unredacted literal repo name.
- predicted-gate-engine-coverage.test.ts: 3 duplicate-winner assertions
  predated #3956's anti-backdating fix and asserted the old createdAt-
  based ordering instead of the new claim-time-only, fail-closed one.
- gittensory.full.yml: missing the review.shared_config (#2046) doc
  section that .gittensory.yml.example already had, breaking their
  required byte-identical-from-marker sync.
JSONbored added a commit that referenced this pull request Jul 7, 2026
Three independent test/doc mismatches accumulated on main from recently
merged PRs, each blocking every subsequent PR's validate-code:

- queue.test.ts: 4 disposition-metric assertions predated #3946's repo
  label redaction and still expected the unredacted literal repo name.
- predicted-gate-engine-coverage.test.ts: 3 duplicate-winner assertions
  predated #3956's anti-backdating fix and asserted the old createdAt-
  based ordering instead of the new claim-time-only, fail-closed one.
- gittensory.full.yml: missing the review.shared_config (#2046) doc
  section that .gittensory.yml.example already had, breaking their
  required byte-identical-from-marker sync.
JSONbored added a commit that referenced this pull request Jul 8, 2026
)

packages/gittensory-engine's own test script (tsconfig.test.json + node
--test) was broken and silently ungated -- test:ci never invoked it, only
the narrower root-level typecheck/test:engine-parity/test:coverage. Three
independent issues, all pre-existing:

- test/ai-policy-map.test.ts: a fixture was missing the required
  repoFullName field AiPolicyFatigueInput has always required.
- test/gate-verdict-calibration.test.ts: two tests deliberately pass
  malformed/extra-field input to verify the sanitizer strips it, but typed
  the fixture as the strict output type instead of casting through unknown
  first -- TypeScript's excess-property/enum checks correctly rejected the
  literals. Cast via `as unknown as GateVerdictCalibrationIngestion`,
  matching what "untrusted external input" actually looks like at this
  boundary.
- test/duplicate-winner.test.ts: three tests asserted createdAt-based
  election ordering that #3956 deliberately removed (anti-backdating fix,
  since an older PR could otherwise steal winner credit by editing its
  body later) -- root test/unit/duplicate-winner.test.ts was updated in
  that same commit, this parallel copy wasn't, because nothing runs it.
  Replaced with the equivalent claim-time-only assertions; the file
  already had adequate non-createdAt coverage for the rest.

Adds `npm run test --workspace @jsonbored/gittensory-engine` to test:ci so
this can't silently drift again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant