Custom support-request form at /support/new with Intercom-backed /api/support - #21030
Conversation
…port backend As support moves from Zendesk to Intercom, Intercom provides no prebuilt request form, so this adds our own: - /support/new/: a hand-built form (fields mirroring the Zendesk form) rendered by layouts/page/support-new.html from front matter, using the shared .form-* design system. Client-side validation, org-name normalization, sessionStorage draft persistence, char counter, query-param prefill, and a honeypot live in theme/src/ts/support-form.ts. - /api/support: a Lambda (Function URL) mounted same-origin via a new CloudFront origin + /api/support* behavior in infrastructure/index.ts, gated by the enableSupportForm stack config (on for www-testing and www-production). The Function URL is sealed by an x-origin-verify shared secret injected by CloudFront. - The handler validates submissions server-side (infrastructure/support-form/validation.ts, unit-tested via yarn test-support-form) and stubs the Intercom integration by logging accepted entries as structured JSON to CloudWatch (90-day retention). The Intercom ticket call and attachment upload land once the API spec is available; attachments currently submit metadata only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DtyVE5R4oYsgzBU7x1AC45
github-actions
Bot
added
the
review:triaging
CamSoper
added
do-not-merge
and removed
review:triaging
Claude
github-actions
Bot
added
domain:website
![unblocked[bot]](https://avatars.githubusercontent.com/in/166219?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No issues found
About Unblocked
Unblocked has been set up to automatically review your team's pull requests to identify genuine bugs and issues.
📖 Documentation — Learn more in our docs.
💬 Ask questions — Mention @unblocked to request a review or summary, or ask follow-up questions.
👍 Give feedback — React to comments with 👍 or 👎 to help us improve.
⚙️ Customize — Adjust settings in your preferences.
github-actions
Bot
added
review:outstanding-issues
Lighthouse Performance ReportCommit: b7bbe13 | Metric definitions
|
The fourth "I need help with:" option was carried over verbatim from the Zendesk form, typo included. The option value (docs) is unchanged, so the closed enum in infrastructure/support-form/validation.ts, the client-side validation, and the unit tests are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DtyVE5R4oYsgzBU7x1AC45
github-actions
Bot
added
review:stale
|
@claude #update-review Fixed the outstanding finding in e31402d: the category label now reads "Pulumi Documentation/Blog". The label was carried over verbatim from the Zendesk form, typo included; Notes on the four low-confidence findings, none of which I've acted on:
Generated by Claude Code |
github-actions
Bot
added
review:in-progress
|
🤖 Review updated on @CamSoper's request. |
github-actions
Bot
added
review:no-blockers
github-actions
Bot
added
review:stale
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The honeypot is indiscriminate. It is a plain <input name="website"> labelled "Website" in the raw HTML, and anything that reads raw HTML sees it -- including a legitimate agent filing a ticket for someone. That agent fills in a plausible URL, gets HTTP 200 with a real-looking id, and reports success for a ticket that was never created. Silent data loss with a fabricated confirmation, which is worse than an outright failure. It cannot be fixed inside the form: any signal that tells a good agent it was trapped tells a bot the same thing. So the fix is a different door. The "For agents" section of /llms.txt now documents POST /api/support -- the six fields, the priority enum, the org-name rules, and the status codes -- and an agent following it sends exactly those six keys and never touches the honeypot. The documentation deliberately does not mention that field. Relabels the trap from "Website" to "Leave this field empty" as well. Naive bots fill every field regardless of what it says, so this costs nothing against them and rescues anything that reads a label before writing to it. The label is inside an aria-hidden sr-only block, so no person or screen reader sees either version. Documenting the endpoint does not weaken it. Obscurity was never the control: the path is already in the shipped JS bundle, it is same-origin, and CloudFront injects the origin secret for anything running in a browser. WAF rate limiting is the actual defense. Every documented claim was checked against the deployed endpoint: the six field names are accepted, a pasted console URL is normalized to the bare org name, and an unknown top-level key is rejected with 422. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The client module had no test coverage at all, and its draft, prefill and validation logic was rewritten twice in one day on review findings -- verified only by reading. Every defect found in it was an interaction bug rather than a bad function: the draft restore, the query-param prefill and the rendered <option> list all read and write the same controls, and the bugs lived in which one won. So these drive the real module against a real DOM (jsdom, already a root dependency) rather than unit-testing pieces in isolation, which would have caught none of them. 20 tests, pinning the regressions specifically: ?priority= actually prefills a <select>; a recovered draft outranks the URL; a deliberately chosen default is NOT reverted by the URL; an untouched field never enters the draft; a restored draft survives the next save; the priority list comes from the rendered options rather than a hardcoded copy; recap values are set as text, not markup; and the form still works when sessionStorage throws. Writing them found a real bug in code from earlier today. The draft save was debounced on `input` only, while touch-tracking listened to `input` and `change` -- so a visitor who changed the priority and nothing else never had that choice written to the draft, and lost it on a failed submit. A <select> is not guaranteed to fire `input`. Both listeners now schedule the save. The suite runs in CI beside the server one, and the existing "did anything actually run" assertion now covers both logs, so neither can quietly report zero tests. Two notes on the setup. The theme is pinned to TypeScript 3.9, which cannot parse the .d.ts syntax current @types/node ships -- and skipLibCheck does not help, those being syntax errors rather than type errors. tsconfig.test.json therefore sets "types": [] and node-test-shims.d.ts declares the handful of Node APIs the tests touch; bumping the theme's compiler would remove the need, but that compiler also builds the production bundle. And because the fixture is hand-written, a final test reads layouts/page/support-new.html and asserts every id and data- hook the module binds to is still present, so the two halves of the DOM contract cannot drift apart without something failing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two reviews of the current head: one on the infrastructure and the published agent docs, one mutation-testing both suites. The mutation pass confirmed the security-critical paths are genuinely pinned -- the honeypot, the origin-secret gate, IP provenance, the 502 and the recap's XSS surface all fail a test when broken -- and found the suites blind to value drift and to the validation -> side-effect boundary. Fixes for both halves. Bugs: - The link-checker WAF exemption is keyed on a User-Agent string anyone can send, and it sits at priority 0 with action allow, so it lifted the rate limit that is the only thing throttling /api/support. Harmless while everything behind the WAF was a static GET; not harmless in front of an unauthenticated endpoint that creates Intercom contacts. Scoped to everything except /api/. - normalizeOrganization's schemeless branch did not allow www., so a pasted "www.app.pulumi.com/my-org" survived as far as the host name and then failed validation on its dots -- a character-set error for an ordinary paste. The two patterns are now one. - A draft naming an option the page no longer renders counted as restored even though the control never took the value, which suppressed the ?priority= prefill. Someone following an urgent link would have filed a normal ticket. - The origin request policy was created at module scope, so every stack with enableSupportForm off built a CloudFront policy nothing references. - clientAddress split on the last colon unconditionally, so a viewer address that arrived without a port lost its final group and was still labelled attributed. The trailing segment now has to look like a port, and RFC 3986 bracketing is stripped. - Submitting is guarded by an in-flight flag rather than only by the disabled button, since the cost of being wrong is a duplicate ticket. /llms.txt corrections, all checked against the code: 403 is reachable (the WAF rate limit, and an origin-secret rotation window) and now documented with back-off guidance; a form-level 422 is keyed _form, not a field name, which is exactly the case the surrounding paragraph emphasises; organization must start with a letter or number. Test gaps, each verified by re-running the mutation that previously survived: - Nothing asserted that the *validated* value crosses into Intercom rather than the caller's raw JSON -- every handler test posted an already-normalized payload, so normalization was a no-op in all of them. - The 403 gate was tested at one body shape, so a body-conditional escape hatch was invisible. It is now proven independent of the payload. - Four of five length caps were unenforced, and the organization test used LIMITS.organization + 1, which pins the rule's shape but not its value. The boundaries are literals now, and LIMITS itself is pinned because /llms.txt publishes those numbers. - KNOWN_KEYS is pinned; a "rejects an unknown key" test cannot see the way that rule actually erodes, which is a key being added. - The client half of the honeypot could be deleted outright: the only test asserted absence on a clean submission, so nothing proved the browser ever sends the signal the server is well tested for. - assert.ok(layout.includes("data-support-form")) could never fail -- it is a prefix of all seven other hooks, and even a delimiter check passes on the layout's own comment. Removing that attribute makes the whole form inert, so it is now anchored to the <form> tag. Tag identity and option values are checked too: a <select> turned into an <input> kept every id intact. - Two prefill tests asserted only that priority stayed "normal", which is also what you see if the module never runs. Both have a positive control now. - Also covered: clearing the draft on success, the banner's two unreached failure paths, non-JSON-object payloads, and method/content-type case normalization. The theme half had no equivalent of check-suites-compiled.js while using the same explicit files array -- a second *.test.ts nobody registered would never compile, never run, and never fail, because the count assertion is satisfied by the existing tests. The guard now takes its directories as arguments and moves to scripts/, where both packages call it. Verified by adding an unregistered suite and watching it fail. Server 36 -> 47 tests, client 20 -> 27. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ncpq6vLuwyNafJ82pLp4CZ
|
@claude #update-review Third adversarial pass fixed. Two reviewers ran against The mutation pass confirmed the security-critical paths are genuinely pinned — the honeypot, the origin-secret gate, IP provenance, the 502, and the recap's XSS surface all fail a test when broken. What it found was blindness to value drift and to the validation → side-effect boundary. Bugs fixed
Test gaps, each verified by re-running the mutation that previously survived:
The theme half had no Server 36 → 47 tests, client 20 → 27. Staging redeploy and browser/API red-teaming next. |
|
🤖 Review updated on @CamSoper's request. |
All three are mine, introduced by the last commit. The theme half's path filter was narrower than what its guard inspects. check-test-suites-compiled walks every *.test.ts under theme/src/ts at any depth and fails on one missing from tsconfig.test.json's files array, but the workflow watched only support-form.test.ts by name -- so the PR adding theme/src/ts/lightbox.test.ts would land green and turn this job red on the next unrelated support-form PR, which would then look like the culprit. That is the exact failure the server filter was widened to avoid, reintroduced on the other half. Now theme/src/ts/**/*.test.ts. Scoped to test files rather than all of theme/src/ts because tsconfig.test.json compiles a fixed three-file list: an unrelated theme module cannot break this suite, only an unregistered suite can. The /api/ exclusion on the link-checker WAF exemption byte-matched with no text transformations, guarding a priority-0 terminating allow -- so a path shape the match failed to recognise would hand the rate-limit exemption back to anyone setting the header. Added URL_DECODE and LOWERCASE per AWS's guidance for URI-path matching. Whether /API/ or /%61pi/ actually reach the Lambda depends on CloudFront's own normalization, so this is defence in depth rather than a closed bypass. Rule set re-validated with wafv2 check-capacity: accepted, 26 WCU. Adding a positive control to "a deliberately chosen default is not reverted by the query string" made it byte-identical to the precedence test above it, so any mutation killing one killed the other. Rewritten as the case its comment always described but the fixture never exercised: the visitor opens ?priority=urgent, picks normal themselves, and returns to the same link. That drives the touched set -- normal is the rendered default, so nothing else distinguishes a chosen default from an untouched control -- rather than re-testing draft precedence from a hand-written draft. Verified by removing the change->markTouched listener and watching it fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ncpq6vLuwyNafJ82pLp4CZ
Two adversarial passes against the deployed staging endpoint and form. Duplicate-ticket paths, both of which end with a filed request sitting in the form waiting to be sent again: - Browser Back after a successful submission returned a fully repopulated form. sessionStorage was cleared, but browsers restore form state themselves for a history entry, and nothing on screen said the request had already been filed. The controls are reset before the card is hidden, so there is nothing left to restore. - clearDraft removed the key without cancelling the pending debounced save, so a submit landing inside the 500ms window had the timer fire afterwards and write the draft straight back. Reproducible by typing the last character and submitting immediately. The honeypot announced itself twice over. It was checked before validation, so a knowingly invalid payload returned 200 with the trap set and 422 without it -- one request pair revealed the field. And the fake success omitted ticketId, which the published contract says a real success carries. It now validates first and mints a plausible id, so the drop is indistinguishable from an accept. Client/server drift, in the direction the module's own header warns about -- the client stricter than the API, hard-blocking values the server accepts: - normalizeOrganization had the same two-pattern bug fixed server-side last commit, so "www.app.pulumi.com/my-org" was rejected outright. - Only email and organization mirrored a maximum, so an over-long subject, name or description reached the API by prefill or programmatic fill and came back 422. The email pattern now matches the server's tightened one too. Input handling. Nothing stripped control characters or bidi overrides, and the values land in an Intercom ticket a support engineer reads -- and from there often a Slack relay or a terminal. NUL truncates, a bare CR overwrites the visible line, ESC opens ANSI sequences, and U+202E reorders a rendered line without changing its bytes. All are stripped; tab and newline are kept because the description is Markdown. Email additionally rejects the separators that mean something downstream (angle brackets, quotes, comma, semicolon). Also: the content-type gate matched a prefix, so application/jsonlines and application/json-patch+json were accepted as JSON; the unknown-key error echoed an unbounded attacker-controlled key into every consumer's logs; and the Intercom client had no per-call timeout, so a hung call was killed by the runtime outside the handler's try/catch -- returning the platform's 502 instead of the documented envelope and never logging support_request_ticket_failed, leaving a contact created with no ticket and no trace. Smaller UI fixes from the QA pass: the no-JS path posted urlencoded data to a JSON API and navigated to a raw error with everything typed lost, so the form is hidden without script and the Slack alternative offered instead; the failure banner's Slack URL was bare text at the one moment a visitor needs it; the character counter was stale after a draft restore and read "1 characters left"; the counter was not announced to assistive tech; focus was dropped to <body> after every failed submit; ?name= was not prefillable and a prefilled console URL was not normalized until blur. Test harness: mount() now closes the previous jsdom window. The module reads a BARE global sessionStorage and mount() re-points that global at each new window, so a pending draft-save timer left by an earlier test fired during a later one and wrote the OLD form's values into the NEW test's storage. Every draft assertion downstream was reading another test's work; this was found by a new test failing for a reason that had nothing to do with its subject. Server 47 -> 53 tests, client 27 -> 32. Each new test verified by re-running the mutation it is meant to catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ncpq6vLuwyNafJ82pLp4CZ
No blockers: XSS came back clean on every reflected path, twelve hostile drafts were all handled without pollution or an uncaught throw, and sixteen concurrent submit attempts produced exactly one API call. Four real findings, three fixed here. The honeypot was named "website", which is precisely what a password manager stores and matches on. An autofilled trap destroys a real request completely and silently: the user is shown the confirmation, their draft is deleted so the text is unrecoverable, no ticket exists, and the analytics counter agrees with the lie. Renamed to leave_blank, which has no autofill semantics. This costs nothing against the naive bots the trap actually catches -- they fill every field regardless of what it is called -- and a competent spammer was never fooled by the old name either, since the field is sr-only and now documented in /llms.txt. The prefill parameters are consumed rather than left in the address bar, which closes three things at once. They are the requester's own email, name, org and subject line, so leaving them there put PII in the referrer and in anything reading location.search. A Back navigation re-applied them, so a visitor who had already filed a request landed on a repopulated form -- form.reset() defends against the browser's own restore, but not against the URL doing it again. And the browser's session-history form restore runs after this code, overwriting a prefilled <select> with its reset value: ?priority=urgent came back as a normal ticket, the exact failure the comment above restoreDraft was written to prevent. The client now applies the same sanitizeText the server does. Previously the confirmation recap could render a bidi override the ticket would not contain -- the wrong way round for a screen whose only job is to confirm what was sent. Left for a decision rather than fixed: the page's X-Frame-Options: DENY is inert because a CSP frame-ancestors directive is also present, and that allowlist includes a third-party LMS. The two headers disagree about intent and the allowlist wins. Excluding /support/new/ from it is a site-wide CSP change that belongs with whoever owns the academy embed, not in this PR. Also unfixed here: the Segment page call fires before this module runs, so the prefill PII still reaches it on the initial view. Stripping it properly means touching the shared analytics snippet. Client 32 -> 36 tests. Both new behaviours verified by re-running the mutation they are meant to catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ncpq6vLuwyNafJ82pLp4CZ
|
@claude #update-review Three more adversarial passes since the last refresh: an API red team and a UI QA pass against deployed staging, then a browser red team against the fixed build. Fixes in Two duplicate-ticket paths. Browser Back after a successful submit returned a fully repopulated form — The honeypot announced itself twice. It was checked before validation, so an invalid payload returned 200 with the trap set and 422 without it — a one-request-pair oracle. And the fake success omitted Input handling. Nothing stripped control characters or bidi overrides, and those values land in an Intercom ticket a support engineer reads — and from there a Slack relay or a terminal. A bare CR overwrites the visible line; U+202E reorders it. Stripped on both sides now, tab and newline kept because the description is Markdown. Email additionally rejects the separators that mean something downstream. Client stricter than the API — the direction the module's own header warns about. Prefill parameters are now consumed rather than left in the URL. They are the requester's email, name, org and subject, so they were riding along in the referrer; a Back navigation re-applied them past Also: content-type matched a prefix ( Test harness bug worth flagging: the module reads a bare global Server 47 → 53 tests, client 27 → 36. Every new test verified by re-running the mutation it is meant to catch. Deliberately not fixed here, flagged for owners: the page sets |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Optional style suggestions from the pre-merge review — apply or dismiss; none of them block.
Generated by Claude Code
|
🤖 Review updated on @CamSoper's request. |
Both introduced by the previous commit. The client validated the raw control value while buildPayload posted the sanitized one, so every length check measured a different string than the one sent -- and it broke in both directions. A 201-character subject containing one stripped character was blocked here although the server would have measured it as 200 and accepted it, which is the "client stricter than the API" failure the file's own header calls out. And a 12-character description with five stripped characters passed here, sanitized to seven on the server, and came back a 422 for a rule this module mirrors precisely to avoid that round trip. validateField now sanitizes first, so the string it measures is the string that gets posted. The new honeypot test asserted autocomplete was absent, with a message claiming the fixture mirrors the layout. It did not: the layout sets autocomplete="off" and the fixture omitted it, so the assertion pinned the omission, checked the fixture against itself, and would have turned red for anyone who fixed the drift. The fixture now carries the attribute, the test asserts "off", and the layout half moved into the contract test where the real file is read -- verified by removing the attribute from the layout and watching that test fail. The parity test uses a value at the limit padded with a stripped character, since the existing "a".repeat(201) case cannot see this class of bug. Client 36 -> 37 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ncpq6vLuwyNafJ82pLp4CZ
|
@claude #update-review Both findings from the last review are fixed at [L238] client validated a different string than it posted. [L724] the honeypot autocomplete assertion pinned the fixture's omission. Correct on all three counts — it checked the fixture against itself, its message pointed at a layout that did not exist, and it would have turned red for anyone who fixed the drift. The fixture now carries Client 36 → 37 tests. I am now working through every remaining item on this review, including the two carried |
There was a problem hiding this comment.
🧹 Optional style suggestions from the pre-merge review — apply or dismiss; none of them block.
Generated by Claude Code
|
🤖 Review updated on @CamSoper's request. |
Closes the three outstanding items, all five low-confidence ones, and the pre-existing note. Three comments the previous push falsified. The layout's honeypot comment still named a "Website" field that the rename removed, and claimed /llms.txt never mentions the field -- the same commit's llms.txt edit names it deliberately, so a legitimate agent knows not to send it; the sentence is inverted rather than deleted. KNOWN_KEYS said the handler checks the honeypot "before validation runs", which the oracle fix reversed, and called membership a tolerance when it is load-bearing: drop leave_blank from that array and a trapped submission takes the unknown-key path to a 422, restoring the status-code oracle the reorder closed. And check-test-suites-compiled.js still described itself as walking support-form/ after it became generic over srcDir. The replaceState dropped the entire query string rather than the five prefill keys, so a campaign link to this page lost utm_*, gclid and anything else after page load -- Segment stamps location.search onto every event at call time. Now only the prefill keys are deleted; the privacy win is unaffected because the PII is all in those five. sanitizeText covered the Trojan Source set but not the weaker bidi marks. U+061C, U+200E and U+200F reorder only neutral characters, but flipping the punctuation in a URL is the same "displays something other than what it contains" failure, and none is a character anyone types. Both mirrors moved together, and a test now reads the two files and asserts the character classes are identical -- that property is what makes the "recap shows what was filed" test meaningful. syntheticTicketId built all 15 digits uniformly, so one in ten began with a zero where a real Intercom id never does. Confirmed live: the testing workspace returned 215475647261127, 215475647300185 and 372996254723247, and a synthetic id observed on staging was 042090389459334. First digit is now 1-9 and the test regex rejects a leading zero. The residual timing tell is documented rather than closed: padding the drop path to match three round trips to api.intercom.io holds a Lambda invocation open to serve a bot, and the trap only catches naive ones. The confirmation page no longer promises a response time. This was filed as unverifiable; data/pulumi_pricing.yaml actually contradicts it -- contracted support is Enterprise and above, and the normal-ticket SLA there is "1 or 5 business days", so "usually within one business day" overstates it for every reader of a form any anonymous visitor can reach. It now says how soon depends on the edition, with a comment recording that a published figure needs support's sign-off and has to match that file. The blog link whose text named contracted support tiers now points at /pricing/, which describes them, rather than at a generic request form. The other three posts swap "Pulumi Support" and "our support team" from the Zendesk portal to the same destination -- like-for-like replacements of a link that disappears at cutover, so no updated: stamp: that field is for changes worth surfacing to readers, and a routing swap is not one. Also took the pre-existing RBAC qualifier while on the line, since data/pulumi_pricing.yaml scopes configurable RBAC to Enterprise and above. Client 37 -> 40 tests. Each fix verified by re-running the mutation it catches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ncpq6vLuwyNafJ82pLp4CZ
|
@claude #update-review Every finding on this review worked through at 🚨 Outstanding — all three fixed[L238] validator/payload sanitization seam — fixed ( [L724] self-referential autocomplete assertion — fixed ( [L124] three false comments — all three fixed.
|
There was a problem hiding this comment.
🧹 Optional style suggestions from the pre-merge review — apply or dismiss; none of them block.
Generated by Claude Code
|
🤖 Review updated on @CamSoper's request. |
data/pulumi_pricing.yaml:770-775 draws a distinction the previous wording blurred: the built-in Admin/Member/Billing Manager roles are available in every edition, and it is the *configurable* RBAC that starts at Enterprise. So "built-in RBAC (Enterprise edition and above)" priced the thing every edition has and left a Team-edition reader believing they have no role model at all. One word: configurable. Also tightened the SCIM troubleshooting line this PR already touches. Unlike /content/blog/, which AGENTS.md treats as historical, the style guide applies to docs, and "please contact our customer support for assistance" carries both a "please" and a wordy tail. The two blog prose nits Vale raised alongside it are left alone: those lines are historical, the double space predates this branch, and the only change we made there was the link itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ncpq6vLuwyNafJ82pLp4CZ
|
@claude #update-review [L102] RBAC noun — you are right, and the fix is at Worth noting this one came from a suggestion in the previous review and I took it without checking the noun against the row's own comment. That comment is three lines above the Style suggestions — took one, left two, on the same historical-vs-current split
Everything else on the review is closed. Final staging deploy is running for a manual pass in the morning. |
|
🤖 Review updated on @CamSoper's request. |
4 participants
Proposed changes
WIP — do not merge. Custom support-request form for the Zendesk → Intercom migration (context). Intercom has no prebuilt request form, so this builds our own and files submissions as Intercom tickets.
Form page —
/support/new/(content/support/new/_index.md+layouts/page/support-new.html)<form>— email, full name, Pulumi org, priority, subject, description — styled entirely with the shared.form-*/.btn/.cardprimitives, no new CSS. All copy lives in front matter; two-card layout (form → confirmation) modeled on/extend-trial.theme/src/ts/support-form.ts(registered inmain.ts): per-field validation mirroring the server rules (aria-invalid+.form-error, focus-first-invalid, clear-on-input), org-name normalization (pastedhttps://app.pulumi.com/<org>URLs reduce to the org name), sessionStorage draft persistence so a failed submit never loses the description, a character counter,?priority=/?subject=prefill, a honeypot, and a graceful error banner for environments where the endpoint doesn't exist (PR previews, localmake serve).block_external_search_index: trueuntil cutover makes this the canonical support entry point.Backend —
POST /api/support(infrastructure/supportForm.ts+infrastructure/support-form/)/api/support*behavior on the existing distribution — no CORS, and traffic stays behind the site's WAF rate limiting. The Function URL is AuthType NONE but rejects any request lacking thex-origin-verifyshared secret that only CloudFront injects (secret generated by@pulumi/random, delivered via Lambda env var; comma-separated rotation supported).422 {fields}errors the frontend maps back onto inputs. All responses areCache-Control: no-store(newapi-response-headerspolicy; the API behavior also drops the edge-redirect Lambda association and markdown-negotiation functions thatbaseCacheBehaviorwould otherwise attach).support-form/intercom.ts): finds or creates the submitter's contact by email, then files a ticket carrying the subject, description, org, and priority. Credentials come from stack config (intercomApiKeyas a secret,intercomTicketTypeId) and are delivered as Lambda env vars — never in this repo, never shipped to the frontend. Accepted submissions are also logged as structured JSON (support_request_accepted) to a pre-created CloudWatch log group with 90-day retention (submissions contain PII); a ticket-creation failure returns 502 and logssupport_request_ticket_failedwith the payload so it can be replayed.enableSupportFormstack config, on for bothwww-testingandwww-production(each with its own Intercom ticket type), so the endpoint and the link cutover go live together in one merge. Dev stacks and PR previews are unaffected — they get no origin or behavior at all, and the form degrades to the error banner.cd infrastructure && yarn test-support-form— 30 tests covering validation, every handler status path (403/405/400/413/422/502/200, honeypot, base64), and the Intercom client (contact found, contact created, request shape, each leg's failure). The Intercom API is faked by stubbingglobalThis.fetchin the tests, sohandler.tsandintercom.tscarry no test seam and the Lambda's serialized closure is unaffected..github/workflows/support-form-tests.ymlruns that suite on any PR touchinginfrastructure/support-form/— standalone rather than part of the PR gate, which needs AWS/Pulumi credentials and skips on forks.Agent path — the
## For agentssection of/llms.txtdocumentsPOST /api/support: the six fields, thepriorityenum, the org-name rules, and the status codes. This is the fix for a hazard the honeypot creates, not a nicety. The trap is a plain<input name="website">that anything reading raw HTML sees, so an agent filing a ticket for someone would fill it in, get a 200 with a real-looking id, and report success for a ticket that was never created. It can't be fixed inside the form — any signal that rescues a good agent also tells a bot it was caught — so agents get a different door, and the documented shape never mentions that field. The trap's label also moves from "Website" to "Leave this field empty"; naive bots fill every field regardless, so it costs nothing against them.Link cutover (partial) — footer,
help-links.html,hand-raise-section.html,/extend-trial, and the/support/redirect now point at/support/new/. The remaining Zendesk links incontent/security/,choose-edition.md, andgetting-support.mdstay put until cutover.Verified:
make lintclean,make buildrenders/support/new/, webpack bundle compiles, infratscclean, 30/30 unit tests pass.Merging this turns on a live, public
POST /api/supporton www.pulumi.com that files real Intercom tickets, and repoints/support/away from Zendesk — deliberately, so the backend and the links land together rather than leaving a form that renders but can't submit. Verify onpulumi-test.iobefore merging.Follow-ups (not in this PR): attachment upload (the Intercom spec hasn't defined where files go), the three docs pages still pointing at
support.pulumi.com, optional OAC hardening of the Function URL, and a post-deploy health check for the endpoint.Unreleased product version (optional)
n/a
Related issues (optional)
Internal Slack thread: https://pulumi.slack.com/archives/D0BRR746EAD/p1787266575569169