Skip to content

fix(anchor): default to a Rekor shard that exists, and name it when it fails - #9845

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
fix/rekor-shard-default
Jul 29, 2026
Merged

fix(anchor): default to a Rekor shard that exists, and name it when it fails#9845
loopover-orb[bot] merged 1 commit into
mainfrom
fix/rekor-shard-default

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #9844

Found in production, not in review

Provisioning anchoring on a live self-host instance (1,626 decision-ledger rows) produced this published anchor row:

{"seq":1626,"keyId":"6b6490126ad44b51","backend":"rekor","status":"failed","error":"fetch failed"}

The signing worked — the key was used and the tip read. The submission did not, because the default shard does not exist:

log2026-1.rekor.sigstore.dev  -> DNS: NXDOMAIN
log2025-1.rekor.sigstore.dev  -> DNS: 34.160.235.130  POST /api/v2/log/entries: 400   (endpoint live)

Sigstore shards Rekor v2 annually as log<year>-<rev>, and the 2026 shard has not shipped. The constant's comment described it as "the current shard as of when this was written" — but it was written ahead of a shard that never existed. Every deployment that enabled anchoring without overriding the env var published no anchor at all.

Why the tests did not catch it

expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining("rekor.sigstore.dev"), expect.anything());

A nonexistent subdomain satisfies that substring perfectly. The exact host is the thing under test here, so it is now asserted exactly — a future wrong guess fails.

The second, compounding fix

"fetch failed" is Node's generic message with no URL in it. An operator cannot tell apart:

  • the shard hostname does not resolve
  • egress is blocked
  • the log is down

Three different fixes. Diagnosing the real one required probing DNS from inside the container by hand.

That matters more here than in ordinary code: #9271 made anchoring failures publicly visible so "anchoring has been failing for a week" is checkable by anyone. A published failure that does not say what failed only half-delivers that.

The thrown-error path now wraps the cause with the attempted endpoint and preserves the original as cause, so nothing previously inspectable is lost. The !response.ok path already carried status and body and is untouched, as is the recordLedgerAnchorAttempt contract — the raw value still reaches the persistence layer, which stays the single place that normalizes an unknown error.

Note on the pre-existing test I changed

records status:'failed' … on a network exception asserted error: "network down" exactly. It now asserts the cause is still legible and that the endpoint appears. That is a deliberate contract change, called out here rather than slipped in.

Validation

tsc --noEmit clean. 60 tests pass across the rekor, scheduler and persistence suites. ledger-anchor-rekor.ts at 100% lines / 100% branches.

Verified end to end on the instance that surfaced it: with the shard corrected, the container reaches POST /api/v2/log/entries and gets 400 on a deliberately empty body — endpoint live, request rejected as expected.

…t fails

DEFAULT_REKOR_SHARD_BASE_URL was https://log2026-1.rekor.sigstore.dev -- a shard
Sigstore has not deployed. Found on a live self-host instance with 1,626 ledger
rows: the anchor signed correctly, then failed with `fetch failed` because the
hostname is NXDOMAIN. Every deployment that enabled anchoring without overriding
the env var was in that state, publishing no anchor at all.

Sigstore shards annually as log<year>-<rev>; 2025-1 is live and answers the v2
POST path, so the default moves there. Guessing the next shard ahead of its
deployment is strictly worse than lagging behind it -- a stale-but-real default
still anchors, a not-yet-existent one silently anchors nothing.

The test that should have caught this asserted
`stringContaining("rekor.sigstore.dev")`, which a nonexistent subdomain
satisfies. It now pins the host exactly.

Second fix: the recorded failure said only "fetch failed", Node's generic
message, with no URL. An operator could not distinguish a shard that does not
resolve from blocked egress from a log that is down -- three different fixes, and
diagnosing the real one meant probing DNS inside the container by hand. The
thrown-error path now wraps the cause with the attempted endpoint, preserving it
as `cause` so nothing inspectable is lost. #9271 published these failures so
anyone can see anchoring is broken; naming the endpoint is what makes that
actionable.

Closes #9844
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-29 14:06:27 UTC

2 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR fixes a real production bug: the default Rekor shard `log2026-1` never resolved (NXDOMAIN), so any self-host deployment relying on the default silently published no anchor. Swapping to `log2025-1` (confirmed live per the description) is correct and low-risk since it's overridable via env var, and the fix is well-tested with the previously-loose `stringContaining` assertion now pinned exactly so a future wrong guess fails. The error-wrapping change usefully names the attempted endpoint in the persisted failure while preserving the original cause via `Error(..., { cause: error })`, and the accompanying tests (thrown TypeError, non-Error string, network Error) correctly exercise the real `catch` path in `submitToRekor`.

Nits — 5 non-blocking
  • The hardcoded default `https:​//log2025-1.rekor.sigstore.dev` in src/review/ledger-anchor-rekor.ts:21 is itself just as susceptible to going stale when Sigstore rotates shards again — worth a short comment or a TODO/tracking issue for the next rotation, though the PR already discusses this tradeoff at length in the comment above it.
  • The wrapped Error's `.message` embeds the raw `error.message` via string interpolation (ledger-anchor-rekor.ts, catch block) — if the underlying error message ever contains something unexpected (e.g. very long strings), it could bloat the persisted error text; not a real risk currently but worth capping length similar to the `.slice(0, 200)` treatment given to the non-2xx response body a few lines above for consistency.
  • Consider truncating the wrapped error message similarly to how the !response.ok path truncates the response body to 200 chars, for consistency in persisted error sizes.
  • Consider adding a lightweight CI/lint check or a periodic reminder (e.g., a dated TODO or tracked issue) to revisit DEFAULT_REKOR_SHARD_BASE_URL before the next annual Rekor shard rotation, since this exact bug pattern (guessing an undeployed shard) is what caused orb(anchor): the default Rekor shard was never deployed, and the failure it causes does not name the URL #9844.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9844
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 14 registered-repo PR(s), 13 merged, 345 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 345 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Partially addressed
The PR correctly fixes the dead default shard and adds targeted tests, but it violates an explicit constraint in the issue: the requirements state the raw caught value must keep passing through unchanged so the persistence layer remains the single normalizer of unknown errors, yet the diff replaces `error` with a freshly constructed `new Error(...)` in the catch block, always coercing it to an Err

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: Python, TypeScript, Ruby, Go, MDX, Shell, Solidity, JavaScript
  • Official Gittensor activity: 14 PR(s), 345 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 1 step in the Signals table above.
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.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored JSONbored self-assigned this Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.77%. Comparing base (60d900a) to head (8c67b70).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9845      +/-   ##
==========================================
- Coverage   91.64%   90.77%   -0.88%     
==========================================
  Files         916      916              
  Lines      112753   112754       +1     
  Branches    27085    27086       +1     
==========================================
- Hits       103338   102348     -990     
- Misses       8126     9315    +1189     
+ Partials     1289     1091     -198     
Flag Coverage Δ
backend 94.10% <100.00%> (-1.58%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/review/ledger-anchor-rekor.ts 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 31e0d4e into main Jul 29, 2026
8 checks passed
@loopover-orb
loopover-orb Bot deleted the fix/rekor-shard-default branch July 29, 2026 14:06
JSONbored added a commit that referenced this pull request Jul 29, 2026
…t fails (#9845)

DEFAULT_REKOR_SHARD_BASE_URL was https://log2026-1.rekor.sigstore.dev -- a shard
Sigstore has not deployed. Found on a live self-host instance with 1,626 ledger
rows: the anchor signed correctly, then failed with `fetch failed` because the
hostname is NXDOMAIN. Every deployment that enabled anchoring without overriding
the env var was in that state, publishing no anchor at all.

Sigstore shards annually as log<year>-<rev>; 2025-1 is live and answers the v2
POST path, so the default moves there. Guessing the next shard ahead of its
deployment is strictly worse than lagging behind it -- a stale-but-real default
still anchors, a not-yet-existent one silently anchors nothing.

The test that should have caught this asserted
`stringContaining("rekor.sigstore.dev")`, which a nonexistent subdomain
satisfies. It now pins the host exactly.

Second fix: the recorded failure said only "fetch failed", Node's generic
message, with no URL. An operator could not distinguish a shard that does not
resolve from blocked egress from a log that is down -- three different fixes, and
diagnosing the real one meant probing DNS inside the container by hand. The
thrown-error path now wraps the cause with the attempted endpoint, preserving it
as `cause` so nothing inspectable is lost. #9271 published these failures so
anyone can see anchoring is broken; naming the endpoint is what makes that
actionable.

Closes #9844
loopover-orb Bot pushed a commit that referenced this pull request Jul 29, 2026
submitToRekor was v2 on the request side -- hashedRekordRequestV002, POST
/api/v2/log/entries -- while parseRekorResponse expected v1: an object keyed by
entry uuid, a numeric logIndex, and a uuid field.

Rekor v2 returns the TransparencyLogEntry directly, encodes logIndex as a proto3
int64 (JSON "42", a string), and has no uuid at all. All three checks failed on
every real response, so this backend could never record a successful anchor even
when the log accepted the submission -- it wrote status: failed with "did not
match the expected TransparencyLogEntry shape".

Surfaced on a live self-host instance: once #9845 fixed the shard URL, the error
moved from fetch failed (DNS) to this parse failure, i.e. the submission now
reaches a real log and is rejected only at our end.

The fixture was itself v1-shaped and its assertion's title read "reading the
entry under its dynamic uuid key", so the test agreed with the bug and could not
fail. Replaced with a real-shaped entry; the scheduler's inline mock had the same
problem.

backendRef carries the inclusion proof's checkpoint where uuid used to be -- v2's
locator, since an entry is identified by log index plus the checkpoint the proof
was issued against. An absent checkpoint parses to null rather than failing: the
entry IS in the log, only the offline re-check is unavailable, and discarding a
real submission over an optional field is the worse trade. An unparseable
logIndex still fails, so NaN can never reach a published backendRef.

Closes #9854
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orb(anchor): the default Rekor shard was never deployed, and the failure it causes does not name the URL

1 participant