Reconcile security remediation with main: gross v1.3, durable budgets, fail-closed routes - #16
Open
enot3615 wants to merge 99 commits into
Open
Reconcile security remediation with main: gross v1.3, durable budgets, fail-closed routes#16enot3615 wants to merge 99 commits into
enot3615 wants to merge 99 commits into
Conversation
Two CI failures from the reconciliation, both mine. Lockfiles: I regenerated them with npm 11, which writes a tree npm 10 will not accept — CI runs Node 20, so `npm ci` failed on both packages with "Missing: utf-8-validate@5.0.10 from lock file". It passed locally, which is exactly how this got through. Regenerated with npm 10 and verified by running `npm ci` under npm 10 for both packages. Versions: the bump gate refuses published files changing under a published version, and it was right to. Set to 1.9.0-rc.1 and 1.6.0-rc.1 — release candidates, so no one can mistake either for a release. New feature (AIFP-1) plus a settlement path that now refuses where it previously succeeded; there is a fair argument that the second makes it a major, and that call belongs to the release step rather than to this branch. MCP's dependency stays at ^1.8.0 rather than the candidate: the RC is not published and must not be, so a range naming it cannot resolve. ^1.8.0 is satisfied by the eventual 1.9.0, and CI already builds MCP against this PR's local SDK in its own step, which is what §5.4 asks for. Verified after regeneration: npm ci (npm 10), build, test and `npm pack --dry-run` on both. Node 162 tests, MCP 34. Production audit unchanged — agent 0 critical / 0 high, mcp 0 critical / 0 high.
§6.2 wants proof that the SDK encodes the entrypoint the deployed contract actually has. The registry tests already refused v1.1/v1.2 targets, but nothing checked the bytes. The selector is asserted as the literal 0x894eb1f3 rather than derived from the ABI under test — deriving it from the same ABI would pass even if that ABI drifted away from the contract. It was read off the compiled B2BSplitterV13 artifact. The argument order is checked word by word, with merchantAmount as its own word, which is the thing that distinguishes v1.3 from v1.2 where the merchant's share was implied by the total. Also refuses a quote naming a legacy function_signature, and re-asserts that a target declaring version 1.1 or 1.2 is rejected before any calldata is built. SPLITTER_PAY_NATIVE_ABI is exported so the test encodes the same constant the signer uses rather than a copy that could drift. 167 tests passing (was 162).
The clean-machine check §14 asks for turned up something CI could not see. Installing both RC tarballs into an empty project resolved a nested @aifinpay/agent@1.8.0 under @aifinpay/mcp — the published, unremediated build — because MCP declares ^1.8.0 and npm does not admit a prerelease into a stable range. MCP resolves to that nested copy at runtime. Nothing failed. The install succeeded, CI was green, and MCP was quietly running against the SDK this candidate exists to replace. CI missed it because the local SDK was installed AFTER the build and test steps, so the tests ran against the registry copy and the override only affected a final rebuild. The swap now happens first, so the tests exercise the SDK that will actually ship. scripts/check-mcp-sdk-pin.mjs makes the manifest itself the thing that is checked, since that is what a consumer running plain `npm install` gets. While the SDK is an unpublished RC the manifest cannot name it — npm ci could not resolve it — so the check reports the blocker and exits 0, and fails hard as soon as the SDK version becomes stable. A stable release cannot ship the mismatch. It deliberately does not reimplement semver. It enforces the one narrow rule that covers this failure: while the SDK is a prerelease, MCP must name it exactly, because that is the only form npm resolves by default. If the versioning scheme outgrows that, add the real dependency rather than widening the guesswork. docs/CLEAN_MACHINE_EVIDENCE.md records the packing, secret scan, import test and the full §14 gate table, including the gates that cannot pass yet because v1.3, Solana v0.6 and Casper v2 are not deployed.
The SDK carried a hand-written address table. Every value in it turned out to be correct — the chain confirmed all six addresses, both treasuries and every fee split — but nothing checked that, and the August audit lost a day to the SDK and a stale deployments.json disagreeing with no way to tell which was right. SPLITTER_DEPLOYMENTS is now built from src/generated/splitter-table.json, vendored from AiFinPay/evm-contract where it is generated from the canonical registry and verified there against live chain state. Everything that decides where money goes comes from that file: address, version, runtime code hash, treasury, fee split, validity window and whether settlement is enabled. What stays local is what a chain cannot tell you — the viem chain object, a default RPC, the explorer, the local USDC address and which env var carries the native price. Those are kept in a separate CHAIN_METADATA map so the split is visible: getting one wrong degrades the experience, getting a registry value wrong sends money to the wrong place. Two import-time guards: a network with no registry entry throws rather than falling back to a hardcoded address, and a registry chainId that disagrees with the viem chain throws rather than building a target from mismatched chains. scripts/sync-registry.mjs vendors the canonical file and --check fails on drift, wired in as npm run registry:check. Demonstrated by editing the vendored polygon treasury to 0xdead… — exit 1, and re-syncing restores it. The fetch fails closed: an unreachable source is not a pass, because silently keeping the local copy would let the SDK drift for as long as the fetch kept failing. Versions set to 2.0.0 for both packages as approved. This also resolves the nested-SDK blocker from the clean-machine check: a prerelease could never satisfy a stable range, which is why MCP was pulling @aifinpay/agent 1.8.0. With both at 2.0.0 a clean install of the two tarballs resolves exactly one copy of the SDK, verified. check-mcp-sdk-pin.mjs is therefore a hard gate now rather than a report. MCP's CI builds the SDK and installs it by path, since 2.0.0 is not published yet and npm ci cannot resolve it. That edit is CI-only and never committed, and the pin check runs against the committed manifest before it happens. Node 169 tests, MCP 34, both green.
The verification line I added to the MCP job did
require('@aifinpay/agent/package.json'), which the package's exports map does
not expose, so the step failed with ERR_PACKAGE_PATH_NOT_EXPORTED and took the
build down. The diagnostic, not the thing being diagnosed.
npm ls @aifinpay/agent prints the same fact without going through exports, and
also shows when the resolution is a local link rather than a registry copy,
which is exactly what this step exists to confirm.
A stray "npm test" line was left under the restore step by an earlier edit. YAML folded it into the same scalar, so the command ran as "git checkout -- package.json npm test" and failed on two pathspecs that are not files. Build, install, tsc and the tests had all passed by then; only my cleanup step broke. Also restores package-lock.json alongside package.json. npm install rewrote both to point at the local SDK path, and leaving the lockfile modified would mean the tarball verified in the next step is not the publishable one.
Second reconcile on this branch (after 6f378aa). `main` had moved 9 commits since the merge base while the branch sat 84 ahead, and nothing was pulling them together — the branch's own version numbers had drifted *behind* what was published on npm. Merged rather than rebased, and the reason is not preference. This branch contains merge commits (6f378aa, 61acb61). A plain `git rebase` flattens them and replays pre-merge commits against post-merge context, which is what makes the rebase look expensive: it stops on files neither lineage actually contested. Measured both ways — rebase stops repeatedly across 82 replayed commits; merging resolves 9 files once, against final states. `node/src/unifiedAgent.ts` is the clearest case: 8 branch commits touched it, `main` touched it zero times, and under merge it combined with no conflict at all. Conflicts and how each was decided: python/pyproject.toml took main's 1.4.0 over the branch's 1.3.1 — 1.4.0 is published; shipping 1.3.1 would put different code under a number users already have. Then bumped to 2.0.0 with the others. node/package.json 2.0.0. Above the published 1.8.2, and honest: this mcp/package.json branch REMOVES behaviour, so it is a major. mcp/package-lock.json branch side; regenerated and verified by install. CHANGELOG.md both sides kept, newest first. .github/workflows/ci.yml branch side. Its mcp-server job already builds the SDK from source and tests against it, covering what main's separate mcp-against-source job was added for — and that job would fail here anyway, because it installs @aifinpay/agent from the registry and this branch asks for ^2.0.0. A comment records why, so nobody re-adds it and reddens CI. examples/exa-x402-bridge/ main side, both files. Main carries the live bridge server.js, store.js work: USD-denominated pricing off a POL rate, the order bound to the request that was quoted, and the quoted price stored so verification compares against the promise instead of a freshly computed number. The branch's side was the older static PRICE_WEI. node/src/facilitators/ both header halves kept. The CODE merged clean and standard-x402.ts already holds both behaviours — isUnsupportedV2 from main, the throwing buildAuth from the branch. Only the file header disagreed, and each half answers a different question: why detect() fails on live endpoints, and why buildAuth refuses to sign. A reader needs both. Also fixed a docstring that named detectV2; the method is isUnsupportedV2. Verified on the merged tree, not on either side: node 174 tests, 16 files — including x402-v2-is-named.test.ts, which is what proves main's 1.8.2 behaviour survived mcp 34 tests, 2 files — built against the SDK in this commit python 107 tests version-gate passes: 1.8.0 → 2.0.0, 1.5.0 → 2.0.0, 1.3.0 → 2.0.0 Removals are deliberate and complete, not merge damage: agent_claim_self went with its registration and its tests (22b9a0c, 3c26dbe); detectSplitterVersion went with splitterVersionDetect.test.ts (855a4ff), replaced by a test asserting the legacy route is gone. No network in the shipped registry is v1.3 and settlementEnabled is false on all of them, so the EVM native settlement path is closed in this release by construction. Enabling one stays a separate, explicit decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01We1vVZLdj2vYtYaj7fjahX
Contributor
Founder economics lock — 14 Aug 2026This PR must align with the canonical route economics in Jira AIFINP-122:
Required SDK changes before approval
Related: AIFINP-119, AIFINP-122. |
Contributor
Additional concrete SDK mismatch found
That surface is part of AIFP-2/MCP and conflicts with founder economics AIFINP-122. It must not present 1%/0.01% as AIFP-2 pricing. Required: make the product route explicit. For AIFP-2, quote/payment tooling must resolve to 0/0 and must not offer an AiFinPay/creator percentage toggle. If the same generic helper is retained for AIFP-1, it needs an explicit AIFP-1 route and exactly 100/0. Update descriptions, schemas, examples and tests together; do not only fix |
enot3615
added a commit
that referenced
this pull request
Aug 14, 2026
Dependabot has 26 open alerts on this repo. Sorted by who they actually reach:
6 vite + postcss, via vitest — a devDependency. `npm pack --dry-run` on
node/ ships 62 files, `dist/ README.md package.json`, no node_modules.
Nobody installing @aifinpay/agent gets vitest. ZERO consumer exposure.
4 uuid, via @solana/web3.js -> jayson. Real, but see below.
16 ws / qs / body-parser in examples/*, which are `private: true` and never
published — but the README calls them "Production bridges in front of
io.net / Exa / Venice". These run on our own box. THIS is the exposure,
and it is to us, not to SDK consumers.
So the four `ws` high-severity memory-exhaustion DoS alerts are the ones that
matter, and they matter because our own bridges serve traffic with them.
`npm audit fix` cleared them and also dragged viem from 2.48.11 to 2.55.13 in
three production bridges — seven minors on the library that builds payment
calldata, as a side effect of a security patch. Reverted. Replaced with an
`overrides` block naming only the three vulnerable packages, which brings the
change from ~84 package moves down to 7 per bridge and leaves viem alone.
uuid is deliberately NOT overridden. The advisory is "missing buffer bounds
check in v3/v5/v6 WHEN buf is provided"; jayson has exactly three uuid call
sites (generateRequest.js:49, utils.js:52, client/browser/index.js:30) and all
three are `uuid.v4()` with no arguments. Not reachable. npm's proposed fix is to
downgrade @solana/web3.js to 0.0.3, which is worse than the risk by any measure.
In _generic-x402-bridge ws moves 7.5.11 -> 8.21.3, which looks like a major but
is a de-dupe: jayson, isomorphic-ws, rpc-websockets and viem/isows in that tree
all already require ws@8. The 7.5.11 entry was a stale lockfile tail.
Verified by running it, not by reading the diff: the generic bridge starts and
answers `POST /chat/completions` with a well-formed 402. (It binds IPv6 `*`, so
probe it on localhost — 127.0.0.1 times out. Same trap as before.)
Two things the smoke test surfaced that are NOT fixed here:
- the 402 says `x402Version: 1` with `maxAmountRequired`; the live standard is
v2 and calls that field `amount`. Our own bridge is on the wrong side of the
same gap the client SDK is.
- the default splitter in the banner is 0xE34Fc0E6…8440, superseded on
2026-07-31 by v1.2 0xbD1fa545…4DDe.
node/ and mcp/ lockfiles are untouched on purpose: PR #16 already edits both and
is under review. Bumping them here would hand that PR a conflict for six alerts
that reach nobody.
Production still runs deploy/bridges-v12; this fix does not reach the box until
someone deploys from a ref that contains it. See AIFINP-92.
Refs AIFINP-107.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
enot3615
added a commit
that referenced
this pull request
Aug 14, 2026
Asked how to give a terminal agent a wallet, the honest answer today is a
Node script. That is the wrong answer to that question.
What happened before: `npx @aifinpay/mcp` starts and warns
no AIFINPAY_AGENT_SECRET set — generated an EPHEMERAL, NON-RECOVERABLE agent
>> DO NOT FUND these addresses
>> For a persistent wallet … (AiFinPayAgent.fromSeed / `aifinpay init`)
`aifinpay init` is not published. `@aifinpay/cli` and `aifinpay` both 404 on
npm — checked. So the documented exit from the dead end did not exist, and the
real path was: install the SDK, learn that `AiFinPayAgent.new()` generates a
seed it never exposes (no `seed`, `secret`, `key` or `export` field anywhere on
the object — the README's `agent.secretB58` belongs to the OTHER class, the
Solana-only `Agent`), generate your own 32 bytes, and wire it up.
Now:
npx @aifinpay/mcp init once
npx @aifinpay/mcp thereafter
`init` writes ~/.aifinpay/agent.json at mode 600 and prints the three addresses
plus a paste-ready MCP config block. The server reads that keystore when
AIFINPAY_AGENT_SECRET is unset, so **the secret never goes into the config
block** — those get pasted into chats and committed to git.
Deliberate details:
- A second `init` does NOT regenerate. The file may already hold funds;
overwriting it to save a line of output would destroy a wallet.
- `--help` and `--version` no longer fall through to the stdio server. They
used to, which meant `--help` looked like a hang: the process was waiting
for MCP framing on stdin that a human never types.
- An unknown argument exits 2 instead of quietly starting a server. Starting
on a typo is how someone funds an ephemeral address.
- The output says the derivation is not BIP-39, so nobody expects Phantom or
MetaMask to recover it from a phrase, and that the addresses hold nothing.
Also declares `bs58` and `tweetnacl`, which this package imports at runtime in
`agent-claim-self.ts` and never declared. Not cosmetic: installing the agent by
path (what CI does, and what building against the local SDK does) changes the
hoisting and `npm run build` fails with two TS2307s. It only ever worked by
accident. PR #16 carries the same two lines — expect a trivial overlap there.
Nine tests, all driving the real bin as a subprocess: a unit test of the
helpers would not have caught the `--help` hang. One of them was wrong first —
the helper read stdout only, while the server logs to stderr, so two assertions
failed against a binary that was behaving correctly.
mcp suite 43 -> 52 tests, all pass. Build clean.
Refs AIFINP-107.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
enot3615
added a commit
that referenced
this pull request
Aug 14, 2026
* mcp: `npx @aifinpay/mcp init` — one command to a usable agent wallet
Asked how to give a terminal agent a wallet, the honest answer today is a
Node script. That is the wrong answer to that question.
What happened before: `npx @aifinpay/mcp` starts and warns
no AIFINPAY_AGENT_SECRET set — generated an EPHEMERAL, NON-RECOVERABLE agent
>> DO NOT FUND these addresses
>> For a persistent wallet … (AiFinPayAgent.fromSeed / `aifinpay init`)
`aifinpay init` is not published. `@aifinpay/cli` and `aifinpay` both 404 on
npm — checked. So the documented exit from the dead end did not exist, and the
real path was: install the SDK, learn that `AiFinPayAgent.new()` generates a
seed it never exposes (no `seed`, `secret`, `key` or `export` field anywhere on
the object — the README's `agent.secretB58` belongs to the OTHER class, the
Solana-only `Agent`), generate your own 32 bytes, and wire it up.
Now:
npx @aifinpay/mcp init once
npx @aifinpay/mcp thereafter
`init` writes ~/.aifinpay/agent.json at mode 600 and prints the three addresses
plus a paste-ready MCP config block. The server reads that keystore when
AIFINPAY_AGENT_SECRET is unset, so **the secret never goes into the config
block** — those get pasted into chats and committed to git.
Deliberate details:
- A second `init` does NOT regenerate. The file may already hold funds;
overwriting it to save a line of output would destroy a wallet.
- `--help` and `--version` no longer fall through to the stdio server. They
used to, which meant `--help` looked like a hang: the process was waiting
for MCP framing on stdin that a human never types.
- An unknown argument exits 2 instead of quietly starting a server. Starting
on a typo is how someone funds an ephemeral address.
- The output says the derivation is not BIP-39, so nobody expects Phantom or
MetaMask to recover it from a phrase, and that the addresses hold nothing.
Also declares `bs58` and `tweetnacl`, which this package imports at runtime in
`agent-claim-self.ts` and never declared. Not cosmetic: installing the agent by
path (what CI does, and what building against the local SDK does) changes the
hoisting and `npm run build` fails with two TS2307s. It only ever worked by
accident. PR #16 carries the same two lines — expect a trivial overlap there.
Nine tests, all driving the real bin as a subprocess: a unit test of the
helpers would not have caught the `--help` hang. One of them was wrong first —
the helper read stdout only, while the server logs to stderr, so two assertions
failed against a binary that was behaving correctly.
mcp suite 43 -> 52 tests, all pass. Build clean.
Refs AIFINP-107.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* mcp: bump to 1.5.3, declare bs58/tweetnacl for the init command
The cherry-pick carried the init command but not a version bump; the new
published behaviour needs one. 1.5.3 because #19 (payable_fetch) claims 1.5.2 —
whichever merges second reconciles. bs58/tweetnacl are declared for the same
reason as elsewhere: agent-claim-self.ts imports them at runtime and the
source-built CI job fails TS2307 without them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-119 P0-2) Under the AIFP-2 agent-x402 profile (0/0 bps) every payment computed a zero treasury fee, and the unconditional floor check rejected all of them with merchant_amount_below_fee_floor. The floor now applies only when the registry declares a non-zero BPS for that leg, and the creator leg gets the same fail-closed rounding check the treasury leg had. Tests: 0/0 quote validates through to signing amounts and runtime verification; non-zero BPS rounding to zero fails closed on either leg; a quote cannot smuggle fee-bearing amounts onto a 0/0 registry target. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… ways (AIFINP-119 P0-3) A payment's route class is now declared by the SDK entry point that initiated it — call() is the AIFP-2 agent-x402 route, fetchPaid() is the AIFP-1 merchant-aifp1 route — and never by the server's 402 challenge. validateQuotedNativePayment requires the route class and only admits a registry target whose fee profile is exactly the one approved for that route (agent-x402 = 0/0, merchant-aifp1 = 100/0, per the economics locked on 14 Aug). Cross-pairings, the retired 100/1 profile, and unknown route classes all fail closed before signing. Cross-route negative tests cover both directions, the retired profile, and an unrecognised class; the canonical acceptance tests now use the approved profiles. ADR-001 documents the policy and flags the two points left for founder confirmation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…send (AIFINP-118)
Live bridges emit the payment block under pay_native, but call() read
only the legacy pay_matic key, so the headline agent.call({provider})
flow died before validation with a misleading error. The block is now
selected by nativePaymentBlock() — pay_native first, legacy pay_matic
as fallback — and everything downstream is unchanged: both keys are
untrusted input resolved against the canonical registry.
The fixture is a real 402 captured from the live Exa bridge on
2026-08-15. It confirms the mismatch and also that live bridges still
quote the legacy fee-INCLUSIVE v1.2 shape (treasury = 1% of total,
4-arg payNative signature, no splitter_version) — the test suite pins
that this quote is refused fail-closed at validation, never settled.
The route reopens only when bridges quote a registered v1.3 target.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
coinsecuritiescompany
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current purpose
This PR is the consolidated SDK/MCP security-remediation candidate. It is not a publication or production enablement.
AIFP-1 / v1.3 changes added in the latest pre-deploy pass
100/0:0/0route profile.payNative(bytes32 paymentId,address merchant,uint256 grossAmount,address ipCreator,uint256 validUntil,string orderId).valid_untilbefore signing.Durable spend cap restored
The previous branch state had removed
node/src/spendLedger.ts, which violated the release gate for restart/concurrency-safe budget enforcement. This PR now restores and wires:daily_usdis configured;unifiedAgent.tsuses the durable ledger for Solana/EVM/AIFP-1 payment execution rather than the old non-atomic in-memory daily check.Current evidence
Latest SDK CI on remediation head
0d7c3ad770661ded7a3eade2c4ff13f6bdf47d7dis green (run31897362761). The passing suite includes the restored spend-ledger tests, gross v1.3 calldata/registry tests and legacy-route refusal tests.Important branch reconciliation blocker
The remediation branch is currently 99 commits ahead and 11 commits behind
main. An internal sync PR was opened as #25 (main → rebase/remediation-onto-main) and GitHub reportsmergeable_state: dirtyacross 39 changed files.Do not force/squash over those conflicts. Resolve #25 explicitly, preserve the already-merged main behavior (including newer payable-fetch/wallet fixes), then rerun full CI on the reconciled result before merging #16.
What this does NOT enable
Release gates after branch reconciliation
Jira: AIFINP-106, AIFINP-119, AIFINP-123, AIFINP-128, AIFINP-130.