Skip to content

feat(orb): add an AMS-facing secret type to the generalized broker - #8111

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
ams-broker-secret-type
Jul 22, 2026
Merged

feat(orb): add an AMS-facing secret type to the generalized broker#8111
loopover-orb[bot] merged 1 commit into
mainfrom
ams-broker-secret-type

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Implements #7674, ratified on #4941: hosted AMS reuses ORB's installation-based broker rather than a parallel identity system.

  • ORB_SECRET_TYPE_AMS_GITHUB_TOKEN: mechanically identical to ORB_SECRET_TYPE_GITHUB_TOKEN — a GitHub App installation token's permissions come from the App and what the installer granted, not from anything the broker's caller specifies, so there's no real behavioral difference to build here. brokerOrbToken's eligibility check now accepts either value, routing both through the exact same mint/cache/install-eligibility flow. The distinct value exists purely so an enrollment row records which product's container it was issued for (bookkeeping/audit).
  • Deliberately distinct from the self-host session-based GitHub auth packages/loopover-miner/lib/github-token-resolution.ts uses (a human's own OAuth token via /v1/auth/github/token, from a loopover-mcp login) — that flow authenticates an interactive human tool as themselves; this one authorizes a headless hosted container as the installed App, the same reason ORB's own broker exists at all. These are not duplicative auth systems — one models a human identity, the other a machine service identity — confirmed against Design an auth/identity layer for AMS #4941's own ratified resolution before building anything.

Scope boundary

Mirroring the #8064/#8066 split: this adds only the broker's capability to mint this type. It does not wire up a way for a real caller to request it at issuance time — POST /v1/internal/orb/enrollments and oauth.ts's self-enrollment landing page both still hardcode github_token. That's a natural follow-up once there's an actual hosted-AMS consumer ready to call it.

Test plan

  • npx tsc --noEmit clean
  • npx vitest run test/integration/orb-broker.test.ts — 42/42 passing
  • npx vitest run — full suite, 1092 files / 20392 tests, 0 failures
  • Targeted coverage on src/orb/broker.ts — 100% statements/functions/lines; the one branch gap (readCachedOrbToken's entry.permissions ?? {}) is pre-existing, untouched code
  • npx tsx scripts/check-migrations.ts / check-schema-drift.ts — no schema changes in this PR, both clean
  • npx tsx scripts/write-ui-openapi.ts --check clean (internal-only routes, not part of the public OpenAPI surface)
  • npm audit --audit-level=moderate — pre-existing findings only, no dependency changes

Closes #7674

…7674)

Adds ORB_SECRET_TYPE_AMS_GITHUB_TOKEN to src/orb/broker.ts, ratified on #4941:
hosted AMS reuses ORB's installation-based broker rather than a parallel
identity system. Mechanically identical to ORB_SECRET_TYPE_GITHUB_TOKEN -- a
GitHub App installation token's permissions come from the App and what the
installer granted, not from anything the broker's caller specifies, so there
is no real behavioral difference to build. brokerOrbToken's eligibility check
now accepts either value, routing both through the exact same mint/cache/
install-eligibility flow; the distinct value exists purely so an enrollment
row records which product's container it was issued for.

Deliberately distinct from the self-host session-based GitHub auth
packages/loopover-miner/lib/github-token-resolution.ts uses (a human's own
OAuth token via /v1/auth/github/token, from a loopover-mcp login) -- that
flow authenticates an interactive human tool as themselves; this one
authorizes a headless hosted container as the installed App, the same reason
ORB's own broker exists at all. The two are not duplicative: one models a
human identity, the other a machine service identity.

Scope is deliberately narrow, mirroring the #8064/#8066 split: this adds only
the broker's capability to mint this type. It does not wire up a way for a
real caller to request it at issuance time (POST /v1/internal/orb/enrollments
and oauth.ts's self-enrollment landing page both still hardcode
github_token) -- that's a follow-up once a real hosted-AMS consumer exists to
call it.

Tests mirror the existing github_token coverage (mint, cache, install-
eligibility re-check) for the new type, plus a regression test confirming a
genuinely unrecognized secret type is still rejected.
@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 22, 2026
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.55%. Comparing base (321c192) to head (ade2845).
⚠️ Report is 9 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8111      +/-   ##
==========================================
- Coverage   92.01%   89.55%   -2.46%     
==========================================
  Files         754       98     -656     
  Lines       77188    22645   -54543     
  Branches    23335     3882   -19453     
==========================================
- Hits        71021    20279   -50742     
+ Misses       5061     2187    -2874     
+ Partials     1106      179     -927     
Flag Coverage Δ
shard-1 98.61% <100.00%> (+43.51%) ⬆️
shard-2 16.66% <33.33%> (-34.52%) ⬇️
shard-3 6.94% <33.33%> (-49.26%) ⬇️

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

Files with missing lines Coverage Δ
src/orb/broker.ts 98.61% <100.00%> (+0.03%) ⬆️

... and 656 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 22, 2026
@loopover-orb

loopover-orb Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-22 23:01:23 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a second mint-style secret type (`ORB_SECRET_TYPE_AMS_GITHUB_TOKEN`) that is intentionally byte-identical in behavior to the existing `github_token` type, widening the `brokerOrbToken` eligibility check in src/orb/broker.ts:158-163 to accept either value while routing both through the same mint/cache/eligibility path. The change is narrowly scoped (capability only, no new issuance surface), well-documented with rationale for why no behavioral divergence is needed, and backed by four new integration tests covering minting, caching, eligibility re-check, and the negative case that the widened check still rejects unrecognized types. The diff is correct as written and matches its stated intent.

Nits — 4 non-blocking
  • The external brief's 'leaked secret' flags on src/orb/broker.ts:44 and test/integration/orb-broker.test.ts:186 are false positives — these are string constant/enum values (`ams_github_token`) and a test-fixture placeholder, not real secrets.
  • The '8064' cited by the external brief as a 'magic number' at src/orb/broker.ts:141 is an issue-number reference inside a comment, not a numeric literal in code — no action needed.
  • None beyond what's already covered by the tests; the PR is tightly scoped and consistent with the Add a tenant-DB-credential secret type + generic revoke path to src/orb/broker.ts #8064 store-style split precedent.
  • Possible secret-shaped assignment in the diff (generic_secret_assignment) — Verify the value is not a real credential.

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 #7674
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), 14 merged, 242 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 242 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Partially addressed
The PR adds ORB_SECRET_TYPE_AMS_GITHUB_TOKEN and wires it through brokerOrbToken's mint/cache/eligibility logic with matching tests, satisfying the core deliverable and mirroring the GitHub-token strategy as requested. However the issue also names src/orb/oauth.ts as an area to change, and the PR explicitly states it does not wire up any caller (enrollment endpoint or oauth.ts's landing page) to r

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, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 14 PR(s), 242 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
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

@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 b51103f into main Jul 22, 2026
14 checks passed
@loopover-orb
loopover-orb Bot deleted the ams-broker-secret-type branch July 22, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add AMS-specific secret type to the generalized broker (src/orb/broker.ts)

1 participant