From a4bcfaa19f6c756b17f86f1448a6cb753d9d0240 Mon Sep 17 00:00:00 2001 From: Damir Mujic Date: Sat, 6 Jun 2026 03:11:51 +0200 Subject: [PATCH] fix(aip18): close legacy-helper budget leak + buyer-link path bug + buyer UX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings against the v4.4.4 AIP-18 implementation: 1. (Medium) publishAgirailsMd() — the exported legacy helper — uploaded a pay-only file to IPFS *before* its on-chain pay-only short-circuit, so a direct caller could still leak a buyer's budget. Intent is now detected up front; the IPFS/Arweave upload is skipped entirely for intent:pay (cid='', no config_cid written). The CLI path was already correct; this guards the helper too. 2. (Medium) The buyer-link marker was written via getActpDir() (cwd/.actp), not the project root of the published {slug}.md. `actp publish path/to/ buyer.md` from another directory dropped the marker where the runtime client couldn't find it → client fell back to EOA. buyerLink now accepts an explicit actpDir; `actp publish` passes the {slug}.md project root (ACTP_DIR still wins, matching the client). 3. (Low/UX) Pay-only `actp publish` printed "Config published to IPFS" and "Mainnet activation on first payment" — re-muddying exactly what AIP-18 clarifies. Output now branches by intent: a buyer sees "linked, budget stays local, no on-chain registration" + buyer next-steps. Tests: publishPipeline pay-only (no upload, no register, no config_cid) + buyerLink explicit-actpDir. Full SDK suite green (2315 passed, 0 failed). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/cli/commands/publish.ts | 155 ++++++++++++++++++----------- src/config/buyerLink.test.ts | 15 +++ src/config/buyerLink.ts | 38 ++++--- src/config/publishPipeline.test.ts | 39 ++++++++ src/config/publishPipeline.ts | 63 +++++++----- 5 files changed, 212 insertions(+), 98 deletions(-) diff --git a/src/cli/commands/publish.ts b/src/cli/commands/publish.ts index 8963822..403457c 100644 --- a/src/cli/commands/publish.ts +++ b/src/cli/commands/publish.ts @@ -568,13 +568,22 @@ async function runPublish( // buyer has no on-chain configHash and no pending-publish, so without // this marker the gate would fall back to the EOA wallet and require // ETH. The marker triggers NO lazy on-chain activation. + // + // Write it into the published agent's project root (.actp beside the + // {slug}.md), not the cwd — otherwise `actp publish path/to/buyer.md` + // run from elsewhere would drop the marker where the runtime client + // can't find it. ACTP_DIR (if set) still wins, matching the client. try { - saveBuyerLink({ - version: 1, - slug: v4Config!.slug, - wallet: (walletAddress || '').toLowerCase(), - linkedAt: new Date().toISOString(), - }); + const buyerLinkActpDir = process.env.ACTP_DIR || join(projectRoot, '.actp'); + saveBuyerLink( + { + version: 1, + slug: v4Config!.slug, + wallet: (walletAddress || '').toLowerCase(), + linkedAt: new Date().toISOString(), + }, + buyerLinkActpDir, + ); } catch { // Best-effort — publish/link still succeeds without the gas marker. } @@ -800,7 +809,8 @@ async function runPublish( const updatedFrontmatter = { ...(frontmatter as Record), config_hash: configHash, - config_cid: cid, + // Pay-only buyers upload nothing — omit config_cid rather than write undefined. + ...(cid ? { config_cid: cid } : {}), published_at: new Date().toISOString(), ...(arweaveTxId ? { arweave_tx: arweaveTxId } : {}), ...publishMetadata, @@ -825,64 +835,95 @@ async function runPublish( } } - // Output results - output.result( - { - configHash, - cid, - arweaveTxId: arweaveTxId || null, - pendingPublish: true, - testnetActivated: !!testnetTxHash, - ...(testnetTxHash ? { testnetTxHash } : {}), - }, - { quietKey: 'configHash' } - ); + // ──────────────────────────────────────────────────────────────── + // Output results — branch by intent so a buyer's mental model stays + // clean: a pure buyer LINKS (DEC-3/DEC-4). It does NOT publish to IPFS + // and has NO mainnet lazy activation. Reporting otherwise re-muddies + // exactly what AIP-18 set out to clarify. + // ──────────────────────────────────────────────────────────────── + if (isPayOnly) { + output.result( + { + configHash, + linked: true, + intent: 'pay', + // No cid, no on-chain activation — a buyer publishes nothing. + pendingPublish: false, + testnetActivated: false, + }, + { quietKey: 'configHash' } + ); - output.blank(); - output.success('Config published to IPFS and saved locally.'); + output.blank(); + output.success('Buyer profile linked to agirails.app. Budget stays local and private.'); + output.print(''); + output.print('No on-chain registration and no IPFS upload — a pure buyer needs neither.'); + output.print('Gas is sponsored via your auto wallet; top up test USDC with `actp mint`.'); - if (testnetTxHash) { output.print(''); - output.success('Testnet: activated on-chain.'); - } + output.print('Next steps:'); + output.print(' 1. Check your balance: actp balance'); + output.print(' 2. Discover providers: actp find '); + output.print(' 3. Pay a provider: actp pay '); + } else { + output.result( + { + configHash, + cid, + arweaveTxId: arweaveTxId || null, + pendingPublish: true, + testnetActivated: !!testnetTxHash, + ...(testnetTxHash ? { testnetTxHash } : {}), + }, + { quietKey: 'configHash' } + ); - output.print(''); - output.print('Mainnet: on-chain activation will happen on your first payment.'); - - // Context-aware next steps. Endpoint is optional; when absent we - // send the agent's agirails.app profile URL on-chain as a default. - const customEndpoint = frontmatter.endpoint - && frontmatter.endpoint !== PENDING_ENDPOINT - && frontmatter.endpoint !== defaultDiscoveryEndpoint(v4Config?.slug); - output.print(''); - output.print('Next steps:'); - output.print(' 1. Check your balance: actp balance'); - output.print(' 2. Verify config match: actp diff'); - if (customEndpoint) { - output.print(' 3. Probe endpoint: actp health'); - } + output.blank(); + output.success('Config published to IPFS and saved locally.'); + + if (testnetTxHash) { + output.print(''); + output.success('Testnet: activated on-chain.'); + } - // Suggest test payment on testnet - if (testnetTxHash && v4Config?.slug) { output.print(''); - output.print(` Try a test payment: actp pay agirails.app/a/${v4Config.slug} 5`); - } + output.print('Mainnet: on-chain activation will happen on your first payment.'); - // Inform about endpoint default — it is OPTIONAL. When unset, the - // on-chain endpoint defaults to the agent's profile URL on - // agirails.app, which is a real navigable page (vs the legacy - // pending.agirails.io 404). Set a custom endpoint only if you want - // x402 atomic HTTP payments or off-protocol job intake. - if (!customEndpoint && v4Config?.slug) { + // Context-aware next steps. Endpoint is optional; when absent we + // send the agent's agirails.app profile URL on-chain as a default. + const customEndpoint = frontmatter.endpoint + && frontmatter.endpoint !== PENDING_ENDPOINT + && frontmatter.endpoint !== defaultDiscoveryEndpoint(v4Config?.slug); output.print(''); - output.info( - `No custom endpoint set — using your profile URL as the discovery anchor: ` + - `${defaultDiscoveryEndpoint(v4Config.slug)}` - ); - output.print( - ' Set a custom endpoint only if you want x402 instant HTTP payments or ' + - 'off-protocol job intake (HTTPS webhook). Otherwise leave it as is.' - ); + output.print('Next steps:'); + output.print(' 1. Check your balance: actp balance'); + output.print(' 2. Verify config match: actp diff'); + if (customEndpoint) { + output.print(' 3. Probe endpoint: actp health'); + } + + // Suggest test payment on testnet + if (testnetTxHash && v4Config?.slug) { + output.print(''); + output.print(` Try a test payment: actp pay agirails.app/a/${v4Config.slug} 5`); + } + + // Inform about endpoint default — it is OPTIONAL. When unset, the + // on-chain endpoint defaults to the agent's profile URL on + // agirails.app, which is a real navigable page (vs the legacy + // pending.agirails.io 404). Set a custom endpoint only if you want + // x402 atomic HTTP payments or off-protocol job intake. + if (!customEndpoint && v4Config?.slug) { + output.print(''); + output.info( + `No custom endpoint set — using your profile URL as the discovery anchor: ` + + `${defaultDiscoveryEndpoint(v4Config.slug)}` + ); + output.print( + ' Set a custom endpoint only if you want x402 instant HTTP payments or ' + + 'off-protocol job intake (HTTPS webhook). Otherwise leave it as is.' + ); + } } } catch (error) { spinner.stop(false); diff --git a/src/config/buyerLink.test.ts b/src/config/buyerLink.test.ts index 82d2f5a..71313d2 100644 --- a/src/config/buyerLink.test.ts +++ b/src/config/buyerLink.test.ts @@ -60,6 +60,21 @@ describe('buyerLink', () => { expect(hasBuyerLink('base-mainnet')).toBe(true); }); + it('honours an explicit actpDir (publish writes beside the {slug}.md, not cwd)', () => { + const projectActp = join(dir, 'some-project-root', '.actp'); + saveBuyerLink(SAMPLE, projectActp); + + // The marker is NOT in the default (cwd/ACTP_DIR) location... + expect(loadBuyerLink()).toBeNull(); + expect(hasBuyerLink()).toBe(false); + + // ...but is found when the same project-root dir is supplied (what the + // runtime client does when run from that project root). + expect(getBuyerLinkPath(projectActp)).toBe(join(projectActp, 'buyer-link.json')); + expect(loadBuyerLink(undefined, projectActp)).toEqual(SAMPLE); + expect(hasBuyerLink(undefined, projectActp)).toBe(true); + }); + it('deletes the marker (and is a no-op when already absent)', () => { saveBuyerLink(SAMPLE); deleteBuyerLink(); diff --git a/src/config/buyerLink.ts b/src/config/buyerLink.ts index cf5ee3b..727e9f1 100644 --- a/src/config/buyerLink.ts +++ b/src/config/buyerLink.ts @@ -50,22 +50,27 @@ export interface BuyerLink { /** * Path to the buyer-link marker. Network-agnostic by design. + * + * @param actpDir - the `.actp` directory to use. Defaults to `getActpDir()` + * (ACTP_DIR env or `cwd/.actp`). `actp publish` passes the project root of + * the published `{slug}.md` so the marker lands beside that agent's config — + * not in whatever directory the command happened to run from. */ -export function getBuyerLinkPath(): string { - return join(getActpDir(), 'buyer-link.json'); +export function getBuyerLinkPath(actpDir?: string): string { + return join(actpDir ?? getActpDir(), 'buyer-link.json'); } /** - * Save the buyer-link marker to `.actp/buyer-link.json`. + * Save the buyer-link marker to `{actpDir}/buyer-link.json`. * - * Mirrors pending-publish: creates `.actp/` if missing, refuses to write + * Mirrors pending-publish: creates the dir if missing, refuses to write * through a symlinked directory, and writes atomically with mode 0o600. */ -export function saveBuyerLink(link: BuyerLink): void { - const dir = getActpDir(); +export function saveBuyerLink(link: BuyerLink, actpDir?: string): void { + const dir = actpDir ?? getActpDir(); - // Verify .actp/ is a real directory (symlink-attack prevention) — use - // lstatSync so a symlinked or broken-symlink .actp is rejected, not followed. + // Verify the dir is real (symlink-attack prevention) — use lstatSync so a + // symlinked or broken-symlink dir is rejected, not followed. let dirExists = false; try { const stat = lstatSync(dir); @@ -80,7 +85,7 @@ export function saveBuyerLink(link: BuyerLink): void { mkdirSync(dir, { recursive: true, mode: 0o700 }); } - const filePath = getBuyerLinkPath(); + const filePath = getBuyerLinkPath(dir); const tmpPath = filePath + '.tmp'; writeFileSync(tmpPath, JSON.stringify(link, null, 2), { mode: 0o600 }); renameSync(tmpPath, filePath); @@ -91,9 +96,12 @@ export function saveBuyerLink(link: BuyerLink): void { * * @param network - accepted for call-site symmetry with loadPendingPublish; * the marker is network-agnostic so the argument is ignored. + * @param actpDir - the `.actp` directory to read from. Defaults to + * `getActpDir()` — at runtime ACTPClient runs from the project root, so the + * default matches where `actp publish` wrote the marker. */ -export function loadBuyerLink(_network?: string): BuyerLink | null { - const filePath = getBuyerLinkPath(); +export function loadBuyerLink(_network?: string, actpDir?: string): BuyerLink | null { + const filePath = getBuyerLinkPath(actpDir); if (!existsSync(filePath)) return null; try { return JSON.parse(readFileSync(filePath, 'utf-8')) as BuyerLink; @@ -104,8 +112,8 @@ export function loadBuyerLink(_network?: string): BuyerLink | null { } /** Whether a buyer-link marker exists. */ -export function hasBuyerLink(network?: string): boolean { - return loadBuyerLink(network) !== null; +export function hasBuyerLink(network?: string, actpDir?: string): boolean { + return loadBuyerLink(network, actpDir) !== null; } /** @@ -114,9 +122,9 @@ export function hasBuyerLink(network?: string): boolean { * Called when an agent transitions away from pure-buyer (e.g. it now publishes * a provider config and gains a real configHash), so the marker doesn't linger. */ -export function deleteBuyerLink(): void { +export function deleteBuyerLink(actpDir?: string): void { try { - const filePath = getBuyerLinkPath(); + const filePath = getBuyerLinkPath(actpDir); if (existsSync(filePath)) unlinkSync(filePath); } catch { // Best-effort cleanup. diff --git a/src/config/publishPipeline.test.ts b/src/config/publishPipeline.test.ts index bfefa04..7143f8b 100644 --- a/src/config/publishPipeline.test.ts +++ b/src/config/publishPipeline.test.ts @@ -99,6 +99,17 @@ version: "1.0.0" # Test Agent `; +const SAMPLE_MD_PAY_ONLY = `--- +name: buyer-agent +version: "1.0.0" +intent: pay +servicesNeeded: + - code-review +budget: 25 +--- +# Buyer Agent +`; + // ============================================================================ // extractRegistrationParams (tested indirectly via publishAgirailsMd) // ============================================================================ @@ -153,6 +164,34 @@ describe('publishAgirailsMd', () => { ); }); + test('pay-only (intent: pay) uploads NOTHING and never registers (AIP-18 DEC-2/DEC-4)', async () => { + mockReadFileSync.mockReturnValue(SAMPLE_MD_PAY_ONLY); + const mockFilebase = createMockFilebaseClient(); + const mockArweave = createMockArweaveClient(); + + const result = await publishAgirailsMd({ + path: '/test/buyer.md', + network: 'base-sepolia', + registryAddress: '0xreg', + signer: createMockSigner(), + filebaseClient: mockFilebase as any, + arweaveClient: mockArweave as any, + skipArweave: false, + }); + + // A pure buyer publishes no service file: no IPFS upload (budget can't + // leak), no Arweave, no on-chain registration. + expect(mockFilebase.uploadBinary).not.toHaveBeenCalled(); + expect(mockArweave.uploadJSON).not.toHaveBeenCalled(); + expect(result.cid).toBe(''); + expect(result.registered).toBe(false); + expect(result.txHash).toBeUndefined(); + + // Frontmatter writeback carries no config_cid (nothing was uploaded). + const written = mockWriteFileSync.mock.calls[0][1] as string; + expect(written).not.toContain('config_cid:'); + }); + test('full publish with Arweave', async () => { const mockFilebase = createMockFilebaseClient(); const mockArweave = createMockArweaveClient(); diff --git a/src/config/publishPipeline.ts b/src/config/publishPipeline.ts index 217a86e..7c1653f 100644 --- a/src/config/publishPipeline.ts +++ b/src/config/publishPipeline.ts @@ -342,29 +342,42 @@ export async function publishAgirailsMd(options: PublishOptions): Promise 0 (contract guard), so a buyer-only agent // cannot be on-chain at all under the current kernel. We skip both // registerAgent and publishConfig — pay-only identity lives off-chain - // (wallet + agirails.app DB record). IPFS upload above already gave - // them a content-addressed config; that's enough. - const intent = typeof frontmatter.intent === 'string' - ? frontmatter.intent.toLowerCase() - : 'earn'; + // (wallet + agirails.app DB record). The IPFS upload was also skipped + // above, so a buyer publishes nothing. (`intent` computed above.) let registered = false; let txHash: string | undefined; @@ -404,7 +414,8 @@ export async function publishAgirailsMd(options: PublishOptions): Promise