diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index 36029fa..de63058 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -137,7 +137,7 @@ The person who asked for this usually did not write the project and has never in - **Say what the wait is.** "It's downloading the pieces your site is built out of. Nothing is broken; this is just slow." - **Explain the product once, in plain words.** Your site is built out of hundreds of ready-made pieces of code. New security problems are found in them all the time. Patchstack keeps a list of the pieces this site uses and warns the owner when one of them turns out to be unsafe. - **Say what changes and what does not.** Nothing visitors see changes. A couple of small config files are added, and a line that lets Patchstack recognise the site. -- **Hand over the link like a next step, not a log line.** When `setup` prints the dashboard link, say what it is for: "Open this link and sign in — that's what connects the site to your Patchstack account so you can see the reports." +- **Hand over the link like a next step, not a log line.** When `setup` prints the dashboard link, say what it is for: "Open this link and sign in — that's what connects the site to your Patchstack account so you can see the reports." Once the site is connected, the link it prints opens the site in their dashboard: "Open this link to see what Patchstack found." - **Avoid these words with the user**: lockfile, dependency, manifest, npm, registry, UUID, environment variable, build artifact, package manager. Use them in commands, not in sentences addressed to the person. - **Say plainly whether the app is protected.** If they ask, the answer has three parts, and all three matter: the protection is installed in the project; until the site is connected to a Patchstack account that includes protection, Patchstack's rules report attacks without blocking them; and the live app has none of it until they publish again. Do not call the app protected before all three are true. - **Report a failure in terms of what they can do**, not what errored. If a step is genuinely unavailable on this platform, say which one and what it means for them, rather than reporting success. @@ -677,6 +677,7 @@ AI model. A framework or hosting upgrade requires this review again. ## Rules - Never invent or guess a UUID — the scan provisions it, the widget silently no-ops on a fake one. +- Never build a dashboard link from the site UUID. Dashboard addresses use a number the project does not hold, so a link made from `siteUuid` opens a missing page. Pass on the link `setup`, `status` or `next` prints. - Never invent or guess a claim token either. One is only ever handed to you by the person, from their own Patchstack dashboard; pass it with `--claim-token` (or `PATCHSTACK_CLAIM_TOKEN`) and nowhere else — not into `.patchstackrc.json`, not into a committed file, not into your reply. - The CLI never opens the dashboard link and never asks for Patchstack credentials. - Label hosted workspace scans with `PATCHSTACK_ENVIRONMENT=sandbox` in that process only. Leave production builds unset (a platform's own tier or production branch name, or the hosted builder the project belongs to, makes the build report `production`; a developer machine or a CI runner this does not know reports `local`) and never commit a sandbox label into files shared with production. diff --git a/README.md b/README.md index 3c6ed2e..84eb052 100644 --- a/README.md +++ b/README.md @@ -429,7 +429,7 @@ A `pulseAuth` field is still read if present, and `PATCHSTACK_PULSE_AUTH` still ### Connecting straight to your account -The dashboard's "Connect website" prompt carries a **claim token**. Pass it to the first `setup` (or `scan`) and the site it provisions is created in your account, so there is no dashboard link to open afterwards: +The dashboard's "Connect website" prompt carries a **claim token**. Pass it to the first `setup` (or `scan`) and the site it provisions is created in your account, so there is nothing left to connect: `setup` ends on the link to the site in your dashboard instead: ```bash npx @patchstack/connect setup --claim-token diff --git a/src/cli.ts b/src/cli.ts index 007afe1..4e55cb1 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -770,8 +770,11 @@ async function runScan( report?: StatusReport; /** False prints only what this run did, without the checklist. */ showRemainingSetup?: boolean; - /** What the report established, for a caller that prints its own checklist. */ - onReported?: (outcome: Omit) => void; + /** + * What the report established, for a caller that prints its own checklist, with the site's dashboard + * link when a claim token connected it in this run. + */ + onReported?: (outcome: Omit, dashboardUrl: string | null) => void; } = {}, ): Promise { const dryRun = args.flags.get('dry-run') === true; @@ -924,11 +927,14 @@ async function runScan( // The server always returns the UUID. If we didn't have one, persist it so // every subsequent scan targets the same site. + const tokenConnected = response.claim?.state === 'claimed' || response.claim?.state === 'owned-by-you'; if (provisioning && response.uuid !== undefined && response.uuid.length > 0) { const claimUrl = config.endpointTrusted !== false ? buildClaimUrl(config.endpoint, response.uuid) : null; const target = await persistSiteUuid(process.cwd(), response.uuid, claimUrl); report.done.push('Added this project to Patchstack'); - recordChange(report, path.relative(process.cwd(), target) || target, 'site ID and connect link, safe to commit'); + // A site the claim token connected loses the connect link before this run ends. + const saved = claimUrl !== null && !tokenConnected ? 'site ID and connect link' : 'site ID'; + recordChange(report, path.relative(process.cwd(), target) || target, `${saved}, safe to commit`); say(`Created site ${response.uuid}. Saved it${claimUrl !== null ? ' and the link to connect it' : ''} to ${target}.`); } if (typeof response.api_key === 'string' && response.api_key.length > 0) { @@ -974,7 +980,6 @@ async function runScan( } else if (claim !== null) { report.missing.push(notConnectedItem([`${claim.summary}.`, ...claim.hint])); } - const tokenConnected = response.claim?.state === 'claimed' || response.claim?.state === 'owned-by-you'; // With a UUID in hand (existing or freshly provisioned), ensure the Patchstack widget's managed tag in // the source HTML shell so the next preview reload shows it. Best-effort; a failed post never reaches @@ -997,7 +1002,7 @@ async function runScan( // Only the dashboard can see the live site; a production scan is reported, not confirmed. deployed: false, }; - options.onReported?.(outcome); + options.onReported?.(outcome, claim?.connected === true ? claim.dashboardUrl : null); // Setup uploads its own map; this is the workspace's routine scan, which holds the key. if (options.report === undefined && (response.stored || response.reason === 'duplicate')) { @@ -1560,10 +1565,12 @@ async function runSetup(args: ParsedArgs): Promise { if (createdProject) recordChange(report, 'package.json', 'created, with a build step that copies your site into dist/'); if (addedSelf) recordChange(report, 'package.json', 'added @patchstack/connect'); let reported: Partial = {}; + let dashboardUrl: string | null = null; const scanCode = await runScan(args, { report, - onReported: (outcome) => { + onReported: (outcome, url) => { reported = outcome; + dashboardUrl = url; }, }); if (scanCode !== 0) { @@ -1657,7 +1664,12 @@ async function runSetup(args: ParsedArgs): Promise { // passes on. A terminal already shows the report above, and a build log is not read by a person. if (!isInstallOrBuildHook() && !runningInCi()) { const progress = guideProgress(after, reported); - const summary = renderSummary({ ...report, missing }, progress, context, { widgetOnPage: after.widgetInstalled }); + // The claim link takes an owner who is signed in to the site, so it stands in when the server sent no link. + const link = progress.connected ? (dashboardUrl ?? context.claimUrl) : context.claimUrl; + const summary = renderSummary({ ...report, missing }, progress, context, { + widgetOnPage: after.widgetInstalled, + dashboardUrl: progress.connected ? link : null, + }); // Kept, so `summary` can print it again: long setup output can be cut short before it reaches an assistant. try { ensureProjectDirectorySync(cwd, path.join(cwd, path.dirname(SUMMARY_FILE))); @@ -1671,7 +1683,7 @@ async function runSetup(args: ParsedArgs): Promise { console.log(SUMMARY_HEADING); for (const line of summary) console.log(line); console.log(''); - console.log(summaryClosingLine(progress.connected, context.claimUrl)); + console.log(summaryClosingLine(progress.connected, link)); } } return 0; @@ -1700,7 +1712,7 @@ async function syncWorkspace(config: Config): Promise { const SUMMARY_FILE = path.join('.patchstack', 'summary.md'); /** The summary the last setup wrote, with the same lines before it, so an assistant passes it on as it is. */ -function runSummary(): number { +async function runSummary(args: ParsedArgs): Promise { let summary: string; try { summary = readFileSync(path.join(process.cwd(), SUMMARY_FILE), 'utf8').trimEnd(); @@ -1712,14 +1724,17 @@ function runSummary(): number { console.log(SUMMARY_HEADING); console.log(summary); // Ends the way setup does, from what `.patchstackrc.json` records: whether the site has an owner, and its link. - let recorded: { claimed?: unknown; claimUrl?: unknown } = {}; + let connected = false; + let link: string | null = null; try { - recorded = JSON.parse(readFileSync(path.join(process.cwd(), '.patchstackrc.json'), 'utf8')) as typeof recorded; + const config = await resolveCliConfig(args); + connected = config.claimed === true; + if (config.siteUuid !== null && config.endpointTrusted !== false) link = buildClaimUrl(config.endpoint, config.siteUuid); } catch { // No readable config: the closing line goes without the link. } console.log(''); - console.log(summaryClosingLine(recorded.claimed === true, typeof recorded.claimUrl === 'string' ? recorded.claimUrl : null)); + console.log(summaryClosingLine(connected, link)); return 0; } @@ -2349,7 +2364,7 @@ async function runCommand(args: ParsedArgs): Promise { case 'vulns': return runVulns(args); case 'summary': - return runSummary(); + return runSummary(args); case 'next': return runNext(args); case 'doctor': diff --git a/src/report.ts b/src/report.ts index 36016da..9fc65fd 100644 --- a/src/report.ts +++ b/src/report.ts @@ -268,18 +268,24 @@ export const SUMMARY_INTRO = `Setup is finished. The person may not see this com /** * The line setup's output ends on, after the summary. It is the line an assistant is surest to read, so it - * repeats what the intro asks and, while the site has no owner, carries the link the person needs: an - * assistant that reports only that the command succeeded leaves them with no way to see their results. + * repeats what the intro asks and carries the link the person needs: the one that connects their account + * while the site has no owner, their dashboard once it has one. An assistant left without a link writes + * its own from the site ID, and no dashboard address is built from that. */ -export function summaryClosingLine(connected: boolean, claimUrl: string | null): string { - return !connected && claimUrl !== null - ? `Your reply to the person must include the summary above, as written, with this link to connect their Patchstack account: ${claimUrl}` - : 'Your reply to the person must include the summary above, as written: it is their result and their next step.'; +export function summaryClosingLine(connected: boolean, link: string | null): string { + if (link === null) { + return 'Your reply to the person must include the summary above, as written: it is their result and their next step.'; + } + return connected + ? `Your reply to the person must include the summary above, as written, with this link to their Patchstack dashboard: ${link}` + : `Your reply to the person must include the summary above, as written, with this link to connect their Patchstack account: ${link}`; } export interface SummaryOptions { /** The widget tag is on the page, so the summary says where to see it. */ widgetOnPage: boolean; + /** Where the person sees this site in their dashboard, once it is connected. */ + dashboardUrl: string | null; } /** @@ -310,6 +316,7 @@ export function renderSummary(report: StatusReport, progress: Progress, context: const lines = [...renderMarkdownLogo(), `### 🛡️ ${headline}`, '']; // With counts, the protection rules are listed under what was found; without them, they go here. if (rules !== null && !counted) lines.push(`${rules}.`, ''); + if (progress.connected && options.dashboardUrl !== null) lines.push(`See what Patchstack found in your dashboard: ${options.dashboardUrl}`, ''); // One step, the person's: the steps an assistant can take itself stay in the report above the summary. const next = nextProgressStep(progress); diff --git a/tests/bin-invocation.test.ts b/tests/bin-invocation.test.ts index 62da24e..e12e523 100644 --- a/tests/bin-invocation.test.ts +++ b/tests/bin-invocation.test.ts @@ -774,6 +774,61 @@ describe.skipIf(!built)('the packaged bin, invoked as npm invokes it', () => { } }); + it('ends a setup the claim token connected on the dashboard link Patchstack sent', async () => { + const dashboardUrl = 'https://app.example.com/site/123/monitoring'; + const server = await acceptingServer({ state: 'claimed', site_id: 123, dashboard_url: dashboardUrl }); + const dir = freshProject(); + try { + const setup = await run(dir, server.endpoint, ['setup', '--claim-token', 'tok-123'], CONCISE); + const summary = setup.slice(setup.indexOf('──── Summary ────'), setup.lastIndexOf('\n\nYour reply to the person')); + + expect(summary).toContain(`See what Patchstack found in your dashboard: ${dashboardUrl}`); + expect(setup.trimEnd().split('\n').at(-1)).toBe( + `Your reply to the person must include the summary above, as written, with this link to their Patchstack dashboard: ${dashboardUrl}`, + ); + // The connect link is dropped once the site has an owner, so the file is not said to hold one. + expect(readConfig(dir)).not.toHaveProperty('claimUrl'); + expect(setup).toContain('`.patchstackrc.json`: site ID, safe to commit'); + + const again = await run(dir, server.endpoint, ['summary'], CONCISE); + expect(again).toContain(`See what Patchstack found in your dashboard: ${dashboardUrl}`); + expect(again.trimEnd().split('\n').at(-1)).toMatch(/with this link to their Patchstack dashboard: .*\/monitor\/claim\?site=/); + } finally { + await server.close(); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('gives a site that already has an owner the link that takes its owner to it', async () => { + const server = createServer((req, res) => { + req.resume(); + req.on('end', () => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + if (req.method === 'GET') { + res.end(JSON.stringify({ state: 'owned-by-you' })); + return; + } + res.end(JSON.stringify({ uuid: SITE, api_key: 'a-secret-40-chars-long-enough-for-this-1', stored: true, manifest_id: 7, checksum: 'abc' })); + }); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as AddressInfo; + const endpoint = `http://127.0.0.1:${port}/monitor/pulse/manifest`; + const dir = freshProject(); + try { + const setup = await run(dir, endpoint, ['setup'], CONCISE); + const link = `http://127.0.0.1:${port}/monitor/claim?site=${SITE}`; + + expect(setup).toContain(`See what Patchstack found in your dashboard: ${link}`); + expect(setup.trimEnd().split('\n').at(-1)).toBe( + `Your reply to the person must include the summary above, as written, with this link to their Patchstack dashboard: ${link}`, + ); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + rmSync(dir, { recursive: true, force: true }); + } + }); + it('adds the widget when setup is asked for it', async () => { const server = await acceptingServer(); const dir = freshProject(); diff --git a/tests/install-prompt.test.ts b/tests/install-prompt.test.ts index 401c979..1107beb 100644 --- a/tests/install-prompt.test.ts +++ b/tests/install-prompt.test.ts @@ -52,7 +52,7 @@ describe('the install prompt', () => { { done: [], missing: [], checked: 3 }, { installed: true, connected: false, synced: true, deployed: false }, { installCommand: 'npm install --save @patchstack/connect', siteUuid: 's', claimUrl: 'https://example.com/claim', environment: 'sandbox' }, - { widgetOnPage: true }, + { widgetOnPage: true, dashboardUrl: null }, ).join('\n'); expect(summary).toMatch(/Refresh your app's preview\*\* to see the Patchstack widget/); }); diff --git a/tests/report.test.ts b/tests/report.test.ts index 8105cd2..4127c84 100644 --- a/tests/report.test.ts +++ b/tests/report.test.ts @@ -14,6 +14,7 @@ import { SUMMARY_AGAIN, SUMMARY_HEADING, SUMMARY_INTRO, + summaryClosingLine, type StatusReport, } from '../src/report.js'; import type { NextStepContext, Progress } from '../src/progress.js'; @@ -185,7 +186,7 @@ const LOGO = ['```', '████████ P A T C H S T A C K', '▄▄▄ describe('renderSummary', () => { it('writes the message to pass on: findings as the heading, what to do with the full link, and the word to say next', () => { - const lines = renderSummary(report({ rules: 12 }), notConnected, context, { widgetOnPage: true }); + const lines = renderSummary(report({ rules: 12 }), notConnected, context, { widgetOnPage: true, dashboardUrl: null }); expect(lines).toEqual([ ...LOGO, @@ -212,7 +213,7 @@ describe('renderSummary', () => { { file: SECRET_CONFIG_FILENAME, what: 'API key, ignored by git. Never commit it' }, { file: 'index.html', what: 'the Patchstack widget' }, ]; - const lines = renderSummary(report({ checked: 42, findings: clean, missing: [noServerItem], changed }), notConnected, context, { widgetOnPage: true }); + const lines = renderSummary(report({ checked: 42, findings: clean, missing: [noServerItem], changed }), notConnected, context, { widgetOnPage: true, dashboardUrl: null }); expect(lines).toEqual([ ...LOGO, @@ -232,7 +233,7 @@ describe('renderSummary', () => { it('makes no protection claims for an app with no server, and leaves the missing server to the dashboard', () => { const input = report({ rules: 12, confirmed: ['Rules for this app are ready.'], missing: [noServerItem] }); - const text = renderSummary(input, notConnected, context, { widgetOnPage: false }).join('\n'); + const text = renderSummary(input, notConnected, context, { widgetOnPage: false, dashboardUrl: null }).join('\n'); expect(text).not.toMatch(/protection rules|turn on protection|Restart your app/); expect(text).not.toContain('No server was found'); @@ -244,9 +245,9 @@ describe('renderSummary', () => { it('says only the parts of the project it changed', () => { const installOnly = [{ file: 'package.json', what: INSTALL_SCRIPT_CHANGE }]; - expect(renderSummary(report({ changed: installOnly }), notConnected, context, { widgetOnPage: false }).join('\n')).toContain('_Patchstack is now part of your install._'); - expect(renderSummary(report(), notConnected, context, { widgetOnPage: false }).join('\n')).not.toContain('part of your'); - expect(renderSummary(report(), notConnected, context, { widgetOnPage: false }).join('\n')).not.toContain('API key'); + expect(renderSummary(report({ changed: installOnly }), notConnected, context, { widgetOnPage: false, dashboardUrl: null }).join('\n')).toContain('_Patchstack is now part of your install._'); + expect(renderSummary(report(), notConnected, context, { widgetOnPage: false, dashboardUrl: null }).join('\n')).not.toContain('part of your'); + expect(renderSummary(report(), notConnected, context, { widgetOnPage: false, dashboardUrl: null }).join('\n')).not.toContain('API key'); }); it('gives the person one step and leaves the steps an assistant takes to the report', () => { @@ -258,7 +259,7 @@ describe('renderSummary', () => { { key: 'dev-only-install', text: 'Installed as a development tool only', plain: { text: 'Patchstack is installed for development only', hint: ['Run: npm install --save-prod @patchstack/connect'] } }, ]; const input = report({ confirmed: [confirmed], missing }); - const text = renderSummary(input, notConnected, context, { widgetOnPage: false }).join('\n'); + const text = renderSummary(input, notConnected, context, { widgetOnPage: false, dashboardUrl: null }).join('\n'); expect(text).toContain('#### 👉 Connect your Patchstack account'); expect(text).toContain(confirmed); @@ -267,8 +268,22 @@ describe('renderSummary', () => { for (const item of missing.slice(1)) expect(status).toContain(item.plain.text); }); + it('gives a connected site its dashboard link, so an assistant never has to write one', () => { + const dashboardUrl = 'https://app.example.com/site/123/monitoring'; + const lines = renderSummary(report({ missing: [] }), { ...notConnected, connected: true }, context, { widgetOnPage: false, dashboardUrl }); + + expect(lines.slice(LOGO.length, LOGO.length + 4)).toEqual([ + '### 🛡️ Checked 89 packages: 5 vulnerabilities in 3 packages (highest severity: high)', + '', + `See what Patchstack found in your dashboard: ${dashboardUrl}`, + '', + ]); + expect(lines.join('\n')).not.toContain(context.claimUrl); + expect(renderSummary(report(), notConnected, context, { widgetOnPage: false, dashboardUrl }).join('\n')).not.toContain(dashboardUrl); + }); + it('never states a count the run did not get', () => { - const lines = renderSummary(report({ findings: null, rules: 12 }), notConnected, context, { widgetOnPage: false }); + const lines = renderSummary(report({ findings: null, rules: 12 }), notConnected, context, { widgetOnPage: false, dashboardUrl: null }); expect(lines[5]).toBe('### 🛡️ Checked 89 packages, now watching them'); expect(lines[7]).toBe('12 protection rules available.'); }); @@ -324,6 +339,20 @@ describe('renderNext', () => { }); }); +describe('summaryClosingLine', () => { + it('carries the link to connect until the site has an owner, then the link to its dashboard', () => { + expect(summaryClosingLine(false, context.claimUrl)).toBe( + `Your reply to the person must include the summary above, as written, with this link to connect their Patchstack account: ${context.claimUrl}`, + ); + expect(summaryClosingLine(true, 'https://app.example.com/site/7/monitoring')).toBe( + 'Your reply to the person must include the summary above, as written, with this link to their Patchstack dashboard: https://app.example.com/site/7/monitoring', + ); + expect(summaryClosingLine(true, null)).toBe( + 'Your reply to the person must include the summary above, as written: it is their result and their next step.', + ); + }); +}); + describe('the summary heading', () => { it('is the one AGENT-INSTALL.md tells an assistant to pass on, in the usual path and the plain HTML path', () => { const doc = readFileSync(new URL('../AGENT-INSTALL.md', import.meta.url), 'utf8');