Skip to content

fix(bounties): normalizeGittBountySnapshot trusts untyped fields from the untrusted Gitt payload #9313

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

normalizeGittBountySnapshot (src/bounties/ingest.ts:18-38) parses payload: unknown from an
external, untrusted source (the Gitt bounty snapshot import route feeds this function whatever
c.req.json() returns, including any shape) into BountyRecord[]. Its per-issue validation is:

if (issue.id === undefined || !issue.repository_full_name || !issue.issue_number || !issue.status) return [];
const amountText = issue.bounty_alpha ?? (issue.bounty_amount === undefined ? undefined : String(issue.bounty_amount));
return [
  {
    id: String(issue.id),
    repoFullName: issue.repository_full_name,
    issueNumber: issue.issue_number,
    status: issue.status,
    amountText,
    sourceUrl: `gitt://issues/${issue.id}`,
    payload: toJsonRecord(issue),
  },
];

These are truthy checks only — they confirm a field is present but never that it has the
type the BountyRecord contract (src/types.ts) declares: repoFullName: string,
issueNumber: number, status: string. A payload where repository_full_name is a number, or
issue_number is a numeric string like "42", or status is an object, passes every one of
these truthy checks and is written straight into a BountyRecord that violates its own declared
type — the value flows on to whatever persists/reads BountyRecords downstream with no further
guard.

The sibling module src/registry/normalize.ts, which has the exact same job (turning an
untrusted external JSON payload into typed internal records), already solves this with typed
value guards:

// src/registry/normalize.ts:162-172
function numberValue(value: JsonValue | undefined): number | null {
  return typeof value === "number" && Number.isFinite(value) ? value : null;
}

function stringValue(value: JsonValue | undefined): string | null {
  return typeof value === "string" && value.length > 0 ? value : null;
}

function booleanValue(value: JsonValue | undefined): boolean | null {
  return typeof value === "boolean" ? value : null;
}

normalizeRegistryPayload uses these (stringValue(raw.repo) ?? stringValue(raw.full_name) ?? ...)
to skip/degrade a field that is present but the wrong type, instead of trusting a bare truthy
check. src/bounties/ingest.ts never adopted the same guard for its own untrusted-payload parsing.

test/unit/adapters.test.ts's normalizeGittBountySnapshot test coverage only asserts the
missing-field case ({} → dropped) and a fully well-typed happy path — no case exercises a
present-but-wrong-typed field.

Requirements

  • In src/bounties/ingest.ts, add local type-guard helpers for the fields
    normalizeGittBountySnapshot reads off payload.issues[] — mirroring
    src/registry/normalize.ts's stringValue/numberValue pattern (a local copy in this file is
    fine and matches this codebase's existing convention of small per-module copies of tiny pure
    checks — see src/github/issues.ts:5-15's own comment on that convention).
  • issue.repository_full_name and issue.status must be validated as non-empty strings (reuse
    the stringValue-style guard); an issue where either is present but not a string must be
    dropped from the result exactly the same way a missing field is dropped today (i.e. excluded via
    the existing flatMap short-circuit, not thrown).
  • issue.issue_number must be validated as a finite number (reuse the numberValue-style guard);
    an issue where it is present but not a finite number must be dropped the same way.
  • issue.id may remain number | string per its existing declared type (it is immediately coerced
    via String(issue.id)), but must still be rejected if it is a type String() cannot
    meaningfully stringify for this purpose (object, boolean, undefined) — only undefined is
    currently rejected; add rejection for non-number/non-string id values too.
  • Do not change the existing fallback behavior for amountText (bounty_alpha string, else
    String(bounty_amount) when numeric) beyond what's needed to keep it type-safe — if
    bounty_alpha is present but not a string, or bounty_amount is present but not a finite
    number, amountText must resolve to undefined (matching the existing "field absent" behavior)
    rather than passing the wrong-typed value through.

Deliverables

  • normalizeGittBountySnapshot in src/bounties/ingest.ts rejects (drops, not throws) an
    issue whose repository_full_name, issue_number, or status is present but the wrong
    type, using local type-guard helpers mirroring src/registry/normalize.ts's
    stringValue/numberValue.
  • issue.id is rejected (dropped) when present but neither number nor string.
  • amountText degrades to undefined (not a stringified wrong-typed value) when
    bounty_alpha/bounty_amount are present but wrong-typed.
  • Regression tests added to test/unit/adapters.test.ts covering: a numeric
    repository_full_name, a string issue_number (e.g. "42"), a non-string status, a
    boolean/object id, and a non-numeric bounty_amount — each asserting the malformed issue
    is dropped (or, for amountText, that it resolves to undefined) rather than producing a
    type-violating BountyRecord.

All of the above Deliverables are required in the same PR.

Test Coverage Requirements

99%+ Codecov patch coverage, branch-counted, on every changed/added line in
src/bounties/ingest.ts, including both the new rejection branch and the existing valid-input
branch for each guarded field.

Expected Outcome

normalizeGittBountySnapshot never produces a BountyRecord whose fields violate their declared
types when fed a malformed-but-present field from the untrusted Gitt snapshot payload — matching
the type-safety bar src/registry/normalize.ts already holds itself to for the same class of
input.

Links & Resources

  • src/bounties/ingest.ts:18-38 (the function to fix)
  • src/registry/normalize.ts:162-172 (stringValue/numberValue/booleanValue — the pattern to
    mirror)
  • src/types.ts (BountyRecord's declared field types)
  • test/unit/adapters.test.ts (existing, incomplete coverage for normalizeGittBountySnapshot)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions