Skip to content

feat(cli): accept the deployment's known origins as one deployment - #199

Merged
A1igator merged 5 commits into
mainfrom
A1igator/dual-origin-accept
Aug 22, 2026
Merged

feat(cli): accept the deployment's known origins as one deployment#199
A1igator merged 5 commits into
mainfrom
A1igator/dual-origin-accept

Conversation

@A1igator

Copy link
Copy Markdown
Contributor

Scope

Teaches the CLI that the deployment's known origins are one deployment, so the tenjin.sh flip (BackTrackCo/tenjin#402) does not break an installed client. Closes the CLI half of BackTrackCo/tenjin#738.

  • src/lib/production-origin.ts gains the deployment origin set and isSameDeployment. Membership means one server, one set of publishers, one SIWX domain acceptance. It is not trust: aliasing applies only when the configured base is itself a member, so a self-hosted, preview, or localhost baseUrl keeps the exact comparison it has today.
  • src/lib/resource-ref.ts routes assertOnBaseOrigin through it. This is the money-path pin, so the refusal branch is unchanged for everything outside the set: same USAGE code, same message naming both origins, same fix line that reads the configured value and never coaches re-pointing the CLI at the URL that just failed. Risk is the widened surface itself, bounded by a two-member set both sides must belong to.
  • src/commands/pay.ts and the generated hook in src/lib/hook-scripts.ts break identically at the flip and get the same rule. pay would drop a Tenjin URL into the Bazaar lane, which refuses it as a third-party endpoint; the hook would silently drop every candidate. HOOK_SCRIPT_VERSION moves to 19 because the generated body changed.

PRODUCTION_ORIGIN does not move; the shipped default flips in a later release. Stored config is not rewritten.

Stacked

Stacked on #150, base A1igator/centralize-production-origin. Required checks may not run while the base is an unmerged branch.

Deliberately left

  • Session-key origin matching (src/lib/session-present.ts, src/commands/doctor.ts). A session is a delegation whose SIWX domain is one host, and the server compares host-with-port, so presenting a tenjin.blog session at tenjin.sh would be rejected server-side. Aliasing it would send a credential to be refused instead of refusing locally. A read on the other origin falls back to the 402 path; buy signs fresh against the target and works.
  • The blanket redirect block in src/lib/http.ts. It compares nothing, so the alias set does not reach it, but if the deployment ever 301s tenjin.blog to tenjin.sh instead of dual-serving, every signed request fails blocked-redirect and no client-side change helps. Dual-serve has to hold.

Testing

  • pnpm format:check, pnpm lint, pnpm typecheck, pnpm build: clean.
  • pnpm test: 2341 passed, 10 skipped, across 74 files (1 skipped). Existing refusal tests pass unchanged.
  • CI=true bash scripts/pack-smoke.sh: PASS.

New tests pin the property in both directions: a candidate on the other known origin is accepted when the base is one of the deployment's origins (search, resolveResourceRef, pay lane), an origin outside the set is refused with the same error from either base, a self-hosted base gets no aliasing, scheme and port still separate origins, and a lookalike host (tenjin.blog.evil.example) does not ride in.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator A1igator added the priority: critical Blocks other PRs; merge-priority label Aug 21, 2026

@vraspar vraspar 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.

Review: comments-only — no majors; 5 minors, 2 nits; aliasing design holds under adversarial review

Reviewed against the diff and the money-path threat model; verified at 63c51c9 in a clean worktree. Three-panel review (security/adversarial, backend+testing, hygiene+CI+agent-usability). The core property survives attack enumeration: comparisons are exact string equality on canonical URL.origin everywhere (CLI and inline hook), the set is a build-time literal with no config/env influence, both-sides membership is symmetric between CLI and hook, self-hosted bases gain no aliasing, lookalike hosts are refused, SIWX binds only to origins within the set, redirects fail closed, and every downgrade window fails closed. 265 tests pass across the touched files locally; lint/typecheck/prettier clean.

What's solid:

  • One shared helper decides the money question (isSameDeployment consumed by assertOnBaseOrigin and the pay lane); the hook's duplication is forced (self-contained .mjs), named back to the source of truth, and generation-time-inlined from the same constant — no second definition exists.
  • Refusal envelope is bit-identical before/after for anything outside the set: same USAGE code, message naming both origins, fix line that never coaches re-pointing.
  • Both migration directions tested with real artifacts (flipped-server candidates accepted into storage, stored candidates re-resolved against either base, pay lane kept cross-origin).

Minor

  1. testing The hook's alias rule is pinned as a string literal, never executed: the only pin asserts KNOWN_ORIGINS is inlined verbatim; every behavioral hook test runs against a localhost base outside the set, so only the exact-compare branch executes. A regression dropping the request-side half — one-sided KNOWN_ORIGINS.includes(origin) — would let a non-deployment listener launder deployment-origin candidate URLs into payable pointers with the suite green (mitigated downstream by resolveResourceRef re-asserting before send). Fix: strengthen the containment assertion to require the full rule text, or better, one runScript case exercising the sibling-origin branch.
    return KNOWN_ORIGINS.includes(origin) && KNOWN_ORIGINS.includes(requestUrl.origin);
  2. security The trust-anchor set has no exact-membership pin and no revocation runbook: KNOWN_DEPLOYMENT_ORIGINS is baked into every shipped CLI and install-time hook; from cutover until clients update, whoever controls a member origin receives wallet-signed credentials from CLIs configured on the sibling. Tests pin properties of the set but not its contents, so adding/failing-to-remove a member is never a forced deliberate diff. Fix: expect(knownDeploymentOrigins()).toEqual([PRODUCTION_ORIGIN, 'https://tenjin.sh']) plus a recorded operator runbook for removing a member relative to the #402 flip-back. Severity minor because exploitation requires losing control of a member origin — which is what membership asserts.
    const KNOWN_DEPLOYMENT_ORIGINS: ReadonlySet<string> = new Set([
    PRODUCTION_ORIGIN,
    'https://tenjin.sh',
    ]);
  3. ci CI has never run on this PR: gh pr checks reports no checks on any commit of A1igator/dual-origin-accept — the stacked-branch artifact the PR body predicts. Local runs are not CI. Fix: land the base (or trigger workflows against this branch) so a real green run exists at the merged SHA before merge.
  4. ci HOOK_SCRIPT_VERSION bump has no pin: nothing asserts the value, so a future body change with a forgotten bump stays green and the header silently misreports the writer version (regeneration itself is safe — it's byte-drift based). Fix: expect(HOOK_SCRIPT_VERSION).toBe(19) or pin the emitted (v19) header.
    export const HOOK_SCRIPT_VERSION = 19;
  5. agent-usability command-reference.md now lies about the pay lane split: "Any other https origin is the Bazaar lane" — after this PR a deployment-member origin routes to the tenjin lane without bazaarPay. An agent reading the reference gets the wrong lane model at cutover. Fix: amend to "any other origin outside the deployment's known origins…".
    The configured base URL is always payable. Any other https origin is the
Nits (2), none blocking
  • search.test.ts / pay.test.tsconst [base, sibling] = knownDeploymentOrigins() leans on Set insertion order (index 0 differing from the ctx base); an explicit .filter(o => o !== …) like sibling tests use is robust to reordering. Resolved for free by finding 2's exact-membership pin.
  • src/lib/production-origin.ts:23-41 — 19-line comment block over a 4-line const, well past the ≤12–15% guideline; consistent with this repo's house style, so informational unless the standard binds here.
Verified, not issues

No arbitrary-origin spoofing (exact URL.origin equality everywhere; userinfo tricks yield attacker origin and fail; port/scheme/http-downgrade/lookalike-suffix/dash-prefix all refused and tested); set is build-time-safe, hook-inlined via JSON.stringify with version bumped so installers rewrite; consumer enumeration complete (aliasing applies only at assertOnBaseOrigin ingest paths, resolveLane, hook sameOrigin — fund untouched, bazaar identity unchanged); pay-lane mid-flow swap blocked (redirects pinned on probe and paid leg, SIWX binds target origin in pay / configured base in buy, payTo from decoded challenge unmodified); old-client/new-server CONTRACT_MISMATCH, new-client/old-server exact match, stale v18 hook drops candidates — all fail closed; session-key matching and http.ts redirect block deliberately strict and untouched; refusal envelope regression-tested including fix-line shape.

Verdict: comments-only. No majors; findings 1–2 are the ones worth landing before merge since they guard the trust boundary this PR widens. What ran locally: touched vitest files (265 passed), pnpm typecheck, pnpm lint, prettier --check, gh pr checks.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator

Copy link
Copy Markdown
Contributor Author

All five minors and both nits are in 8960fff, which also merges #150's fixes (d6720e6) forward.

Finding 1, the alias rule is now executed. Two runScript cases against the real generated bytes. The security-relevant one needs no substitution: a localhost stub returning a candidate on PRODUCTION_ORIGIN must be dropped. Verified it catches the regression you described by rewriting the rule to the one-sided KNOWN_ORIGINS.includes(origin) form, which reds it. The positive sibling case rewrites only the inlined KNOWN_ORIGINS line, so the run stays offline.

Finding 2, the trust anchor. toEqual([PRODUCTION_ORIGIN, 'https://tenjin.sh']), written out independently, so adding or failing to remove a member is a forced deliberate diff. The revocation runbook is in docs/safety-model.md under money-moving boundaries, and it carries the part that is easy to miss: the set is inlined into installed hooks and only rewritten on a version change, so removing a member takes a HOOK_SCRIPT_VERSION bump and is effective only once operators upgrade.

Finding 4, pinned by digest rather than by value. expect(HOOK_SCRIPT_VERSION).toBe(19) cannot catch a forgotten bump, which is the failure you named. Instead there is a digest pin over all four generated scripts plus a header-stamp assertion. Confirmed live: the one-sided regression above also reds the digest.

Finding 5 is corrected in command-reference.md: a deployment-member origin takes the tenjin lane without bazaarPay when the configured base is itself a member, and a self-hosted base gets no alias.

Nits: search.test.ts names PRODUCTION_ORIGIN rather than indexing the set, so Set order cannot matter (pay.test.ts and resource-ref.test.ts already used find/filter). The comment block in production-origin.ts is down from 18 lines to 11, dropping the restated aliasing rule.

Finding 3 stands as you wrote it. No checks have run on this branch, and none will while the base is unmerged, so a real green run at the merged SHA needs #150 to land first.

Gates: format, lint, typecheck, build clean; 2349 tests pass; pack-smoke PASS; HOOK_SCRIPT_VERSION is 19 by design here, while #150 keeps 18 with byte-identity re-proved.

@A1igator
A1igator requested a review from vraspar August 21, 2026 19:57

@vraspar vraspar 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.

Re-review (R2) at 8960fff: five of seven findings resolved with genuine execution tests — one blocker-class item remains: CI still has never run

Delta reviewed: 63c51c9..8960fff. Local tests at this head: 273/273 across the five touched suites, typecheck clean. Merge-forward of #150 resolved cleanly (no duplicated tests; two-file-anchor literals coexist correctly with constant-derived pins).

Resolved since round 1

  • Hook alias rule now executed: two real runScript cases — sibling candidate kept when base is a member (hook-scripts.test.ts:1797), deployment candidate dropped when base is a non-member stub using shipped bytes (:1776); the one rewritten line is asserted present in shipped bytes first.
  • Trust-anchor pin + runbook: exact-membership toEqual([PRODUCTION_ORIGIN, 'https://tenjin.sh']) with independently written expected values, revocation runbook at docs/safety-model.md.
  • HOOK_SCRIPT_VERSION pinned to 19 with sha256 digests of all four generated scripts.
  • Pay-lane doc amended to the membership-conditioned rule.
  • Both nits: insertion-order reliance replaced with named-origin .find(); comments trimmed to invariants.

Still open

  1. CI has never run on any head of this branchgh pr checks reports no checks at 8960fff, including after the merge-forward. Everything above was verified locally; a green CI run at the merged SHA should exist before merge. This is the one item I'd hold the merge on.

New (nits, non-blocking): the runbook's set-removal step should also name the digest test at hook-scripts.test.ts:203, or the fixer ships stale digests; odd-but-harmless http://127.0.0.1:9 fixture port.

Verdict: approve-once-CI-runs-green-at-head. The code itself is done from my side — every round-1 finding landed as specified.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@vraspar vraspar 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.

Re-review delta at 45f98d7: runbook nit fixed; one trivial nit remains and CI still cannot run on this branch

Delta reviewed: 8960fff..45f98d7 — docs-only, adds the digest-pin step to the member-revocation runbook as suggested. The remaining nit from R2 (the http://127.0.0.1:9 fixture port in hook-scripts.test.ts:1799) is untouched — harmless, cosmetic, non-blocking.

On the CI hold from R2: verified the cause is structural, not neglect — ci.yml triggers only on pull_request targeting main, so a stacked PR cannot produce a check run until its base lands. The practical sequence: merge #150, retarget this PR to main, confirm green at the merged SHA. Everything else stands resolved from R2; local verification at 8960fff was 273/273 across the five touched suites plus typecheck clean.

Verdict: unchanged — approve once a green CI run exists at head. Say the word if you'd rather I approve on the strength of the local runs alone.

Base automatically changed from A1igator/centralize-production-origin to main August 22, 2026 00:52
A1igator and others added 5 commits August 21, 2026 20:54
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cip8Fc8VvouqatPNkXxcr3
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cip8Fc8VvouqatPNkXxcr3
Review follow-ups on the deployment-alias PR.

The hook's alias rule was pinned as a string and never run: every
behavioural hook test uses a localhost base outside KNOWN_ORIGINS, so
only the exact-compare branch executed. Two runScript cases now drive the
real generated bytes through it. The one that matters is the two-sided
half: a localhost listener that hands back a candidate on the deployment
origin must have it dropped, because a one-sided
KNOWN_ORIGINS.includes(candidateOrigin) would let anything the operator
ever configured launder a payable pointer at the marketplace into the
store. Verified by reverting the rule to one-sided, which reds it.

KNOWN_DEPLOYMENT_ORIGINS gets an exact-membership pin, written out
independently of the module, so adding a member or failing to remove one
is a deliberate line in a reviewed diff. The set ships baked into every
released CLI and every install-time hook, so a member origin receives
wallet-signed credentials from CLIs configured on the sibling until each
client updates. docs/safety-model.md carries the removal runbook.

HOOK_SCRIPT_VERSION gets a digest pin over all four generated scripts. A
body change with a forgotten bump was doubly quiet: installs keep the old
script, and the new header names a version whose bytes it is not.

docs/command-reference.md no longer says any other https origin is the
Bazaar lane; a deployment-member origin takes the tenjin lane without
bazaarPay. search.test.ts names its origins instead of indexing the set.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cip8Fc8VvouqatPNkXxcr3
18 comment lines over 4 lines of code, with the aliasing rule stated
twice. The invariants stay (what membership means, why the set exists,
both sides must be members, widening it is a security change); the
restatement goes, and the removal runbook is named rather than retold.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cip8Fc8VvouqatPNkXxcr3
Following the runbook without updating the generated-script digest
pin in hook-scripts.test.ts leaves a failing suite after the
HOOK_SCRIPT_VERSION bump.
@A1igator
A1igator force-pushed the A1igator/dual-origin-accept branch from 45f98d7 to eb50018 Compare August 22, 2026 00:57

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator

Copy link
Copy Markdown
Contributor Author

Owed you a reply on two rounds. Both are answered here, and the CI hold is resolved.

R2's runbook nit is done (45f98d7, carried into the rebuild). The member-revocation runbook gains the digest-pin update as step 3, between the version bump and the release, which is the order an operator hits the failure:

  1. Update the digest pin in hook-scripts.test.ts's HOOK_SCRIPT_VERSION test. The bump changes every script's bytes, so the old digests fail until they're repasted.

The http://127.0.0.1:9 fixture port stays, and here is the reasoning rather than just agreement. sameOrigin in the generated hook compares new URL(candidate).origin as a string; nothing ever dials the URL and the test opens no socket. Port 9 is discard, the conventional "guaranteed nothing listening" choice, and distinct from the real stub server on baseUrl. Changing it would be churn without clarity.

R3's CI hold is resolved, and your diagnosis was exactly right. #150 squash-merged as e0eff8b, GitHub auto-retargeted this PR to main, and it went CONFLICTING, because the squash means this branch still carried #150's individual commits. All three conflicts were that duplication, not disagreement.

Rebuilt at eb50018: #199's own layer replayed on origin/main as five clean commits with no #150 replay. Verified by content rather than by ceremony. Diffing the rebuild against the old head 45f98d7 leaves six files, and all six match main byte for byte, because they are #181's merge arriving. Every reviewed artifact survived: isSameDeployment with both-sides membership, the exact-membership pin, the executed hook-alias runScript cases, HOOK_SCRIPT_VERSION at 19 with the four digest pins, the runbook, and the pay-lane correction. git merge-tree against main reports zero conflicts.

CI is running on this PR for the first time. No need to approve on local runs alone.

@A1igator
A1igator requested a review from vraspar August 22, 2026 01:04
@A1igator

Copy link
Copy Markdown
Contributor Author

CI finding is now closed by evidence: run 32541978214 is green at eb50018b377a514476aad43104503baffa154d7c, which is head. Event pull_request, conclusion success. That was the first real CI run on this branch, and it covers the rebuilt commits rather than the pre-rebase head.

Nothing else moved. The only other event since my last comment is a greptile-apps[bot] review whose body reads "Your trial has ended", with zero inline comments.

@vraspar vraspar 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.

Approve at eb50018. Branch rebased onto post-#150/#181 main — diff verified scoped to exactly this feature's 12 files (the notify-registry-pin content visible in earlier range diffs came from main, not this PR). Both R2 holds are cleared: CI now runs and passes at exactly this head (retarget to main fixed the structural trigger gap), and the runbook digest-pin step landed. All round-1 minors and both round-2 nits resolved. One cosmetic item remains — the never-dereferenced http://127.0.0.1:9 fixture constant at hook-scripts.test.ts:1799 — waived here as below the bar, recorded so it isn't lost.

@A1igator
A1igator merged commit 9ec8da8 into main Aug 22, 2026
1 check passed
@A1igator
A1igator deleted the A1igator/dual-origin-accept branch August 22, 2026 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: critical Blocks other PRs; merge-priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants