v0.7.47: seo, import/export api, trigger task sizes - #6005
Merged
Conversation
…#5996) The H1 opened with "Sim is the", spending its first three words on a brand token already carried by the title tag, the meta description, and the hero's sr-only summary. Lead with the terms people search instead. Relaxes the landing H1 rule in both places that asserted the brand had to appear in the heading itself; the sr-only summary still opens by naming Sim, so entity consistency for answer engines is unchanged.
…rs (#5994) * chore(deps): fix OTel version split, drop dead deps, declare emcn peers - Pin @opentelemetry/{resources,sdk-metrics,sdk-trace-base,sdk-trace-node} to exact 2.7.1 so they match sdk-node's pins instead of floating to 2.8.0. The carets meant app code built spans with 2.8.0 and passed them into NodeSDK from 2.7.1, which only worked by duck-typing. - Declare the 13 packages @sim/emcn imports but never declared, as peers mirrored into devDeps. @radix-ui/react-dismissable-layer had no declaration anywhere in the repo and resolved only transitively. - Remove ffmpeg-static: its binary downloads via postinstall, but it is not in trustedDependencies and Docker installs with --ignore-scripts, so the accessSync branch never succeeded and both call sites always fell through to system ffmpeg. - Remove critters + experimental.optimizeCss: Next only loads critters from the Pages Router renderer, and apps/sim is App Router only. - Make simstudio-ts-sdk zero-dependency by dropping node-fetch for native fetch; engines >=18. - Remove unused @vercel/og and postgres from docs, dotenv/inquirer/listr2 from the CLI, and yaml from the root. - Move @aws-sdk/client-appconfig from the root to apps/sim, its only consumer. - Delete the apps/sim overrides block; Bun only honors top-level overrides. - Bump free-email-domains 1.2.25 -> 1.9.70 (4,779 -> 13,059 domains). - Validate object/array variables with JSON.parse instead of JSON5, matching what the executor actually parses. - Swap the changelog GitHub icon off lucide to GithubOutlineIcon, matching the navbar chip on the same page. - Unify @types/node on 24.2.1 and lucide-react on ^0.511.0; bump chalk to 5 and image-size to 2. * fix(deps): complete the OTel pin, restore SDK error detail, revert email list Follow-ups from an independent audit of the previous commit. - Pin @opentelemetry/sdk-node and the three otlp-http exporters to exact 0.217.0. Pinning only their four dependents was self-reversing: sdk-node 0.219.0 requires core 2.8.0 exactly, so the next update would have silently rebuilt the split this PR removes. - Declare @opentelemetry/core (2.7.1). It is imported by lib/copilot/request/go/propagation.ts but resolved only by hoisting, and it is the OTel package with the most version churn in the tree. - Pin @radix-ui/react-dismissable-layer to exact 1.1.13 in @sim/emcn. All five transitive parents pin it exactly; a caret would fork a second copy on 1.1.14, which is the duplicate-context bug the declaration prevents. - Surface error.cause in simstudio-ts-sdk. Native fetch reports network failures as a bare "fetch failed" and puts the reason on cause, so every DNS/TLS/refused error was reaching callers with no diagnostic content. - Revert free-email-domains to 1.2.25. Upstream now merges the free-domain list with two disposable-email blocklists, so 1.9.70 classifies real organization domains as free — UK charities, some companies and universities, and the JP/KR ISP domains APAC SMBs use for business mail. The demo form blocks submission on that check, so a false positive costs the booking entirely. Worth doing deliberately, not inside a deps change. - Lower packages/cli engines to >=18. chalk 5 and commander 11 both accept >=16 and the source uses no Node 20 API, so >=20 only produced EBADENGINE for Node 18 users. --------- Co-authored-by: Waleed Latif <waleed@simstudio.ai>
) * fix(logs): match copilot log-grep patterns literally, never as a regex `grepSpans` compiled a caller-supplied pattern with a bare `new RegExp`, whose only guard was a `catch` for syntax errors. Trace text is attacker-influenced — a workflow can emit arbitrarily long uniform runs into its own block outputs — and matching runs synchronously on the shared event loop, so an authenticated caller could choose both the pattern and the input and stall every request on the instance. Patterns are now matched as an escaped, case-insensitive literal. Screening was implemented first and abandoned, because each rule only excludes the shapes someone thought to enumerate: - `safe-regex2` (already used by the guardrails validator) documents itself as having false negatives. It screens star height only, so it passes `(a|a)*b`, measured >61s on V8 at 30 characters. - Rejecting quantified groups on top of that catches `(a|a)*b`, and every attack `safe-regex2` catches, but still passes `a*a*b` — adjacent quantifiers over overlapping character sets — measured 213s on JSC and 132s on V8 over a 10k-character run, well inside what a block output can hold. An escaped literal is linear on every engine, needs no dependency, and does not depend on which runtime serves the request. `safe-regex2` stays a dependency for its existing guardrails/PII callers; it is simply not relied on here. `query_logs` loses regex matching. Its catalog entry describes `pattern` as "greps" rather than promising regex, but it is generated from a contract in another repository and cannot be updated here, so a `patternNotice` is returned whenever a pattern contains regex syntax — the caller is told the pattern was taken literally instead of reading zero matches as "not in the trace". Two bounds are kept as backstops: a cumulative match-time budget charging only time inside `test`/`exec` (never the blob-store reads the scan awaits between matches, which would truncate slow-but-legitimate greps under load), and a total scanned-character cap. * improvement(logs): only flag genuine regex intent, document the truncated invariant Review follow-ups on the literal log-grep. The `patternNotice` fired on any regex metacharacter, which includes `.` — so ordinary literal searches (`example.com`, `file.pdf`, `v1.2.3`, `block_1.output`) were told their pattern "was not interpreted as a regex", inviting the agent to retry a search that had in fact worked. Measured 10 false notices across 19 realistic literal searches. `REGEX_INTENT` now keys on actual regex intent — escape classes, character class, group, alternation, a leading/trailing anchor, or a repetition quantifier — which drops that to 0/19 while still flagging all 10 regex attempts tested. `truncated` gains a TSDoc block stating its invariant: it reports that trace was left unread, not that a budget was reached. Every point that skips work goes through `done()`, which sets it, so a budget exhausted by the final match correctly reports `false`. Raised by Cursor Bugbot; behavior is right, the contract was just implicit. Also drops `snippetAround`'s `maxChars` parameter, which every caller passed from the state object it already receives. * fix(logs): match log-grep patterns with RE2 instead of dropping regex support Restores full regex support on copilot log-grep, which the previous commits in this PR had removed to close the ReDoS. Deleting the capability was the wrong trade: RE2JS is a port of RE2 that matches in time linear in the input, so no pattern can blow up regardless of the text, and callers keep their regexes. Measured through the real compile path, against a 100k-character adversarial input — 10x the run that took 213s on JSC / 132s on V8: (a+)+$ 13.0ms (\w+\s?)*$ 10.7ms (a|a)*b 6.7ms (x+x+)+y 4.0ms a*a*b 8.1ms ^(\d+)*$ 0.0ms Two escape hatches keep this honest: - A pattern with no regex metacharacter takes the built-in engine. Semantics are identical when there is nothing to interpret, and RE2JS costs ~100x more per byte (~25ms/MB), so the common plain-text search stays native. - Lookaround and backreferences are not in RE2. RE2JS rejects them at compile time, so those fall back to a literal and return a `patternNotice` naming the constructs — a narrow, reported gap rather than a silent behavior change. The match-time and scanned-character budgets stop being formalities: RE2JS's throughput is what they now bound, not backtracking. Also collapses the old test-then-exec double scan — `compilePattern` returns a single `find` returning a match index, which `recordIfMatch` passes straight to `snippetAround`, so each field is scanned once instead of twice. re2js@2.8.6 is MIT, pure JavaScript (no native addon, so nothing changes for the oven/bun image), server-only, and published 2026-07-05 — clearing the 7-day bunfig minimumReleaseAge gate without an exclusion. * fix(security): route every caller-supplied regex through the linear-time engine Extends the RE2 fix to the three other places that compiled a caller-supplied pattern and ran it on the shared event loop, behind one shared helper — `lib/core/security/linear-regex.ts` — rather than four local copies. - `lib/copilot/vfs/operations.ts` — copilot VFS grep took a caller pattern over caller-supplied file content with no screen at all. - `lib/guardrails/validate_regex.ts` (`validateRegex`) — a guardrail rule's pattern matched against LLM output, screened only by `safe-regex2`. - `lib/chunkers/regex-chunker.ts` — the KB chunker's split pattern. This one screened for catastrophic backtracking by running the pattern against six probes including `'a'.repeat(10000)` and rejecting anything over 50ms — but it measured elapsed time *after* the match returned, so the screen was the denial of service it existed to prevent (`a*a*b` on that probe: 213s). The probe is deleted rather than repaired. Deliberately unchanged: `validateRegexPattern`. Those patterns are persisted for Presidio, a separate service where a slow pattern times out instead of stalling this loop, and Presidio's Python engine accepts constructs RE2 does not — so narrowing it to the RE2 subset would reject patterns that work. Its TSDoc now says plainly that its `safe-regex2` screen is a courtesy, not a ReDoS defense, and points in-process callers at `compileLinearRegex`. `safe-regex2` therefore stays a dependency, used only there. Lookaround is the standard way to split while keeping the delimiter (`(?=#\s)`) and RE2 cannot represent it, so the chunker keeps the built-in engine for those patterns — the only remaining path that can backtrack, bounded by the existing 500-character cap. RE2JS `split` was verified identical to `String.split` across six representative chunking patterns. Adds 27 tests for the shared helper (every catastrophic pattern asserted linear over a 50k-character input) plus regression tests at each site, plus the first tests for `validateRegex`. Also removes the now-duplicated `escapeRegExp` and literal-finder locals from `log-views.ts`. * fix(security): drop safe-regex2, keep lookaround splits on the linear engine Audit follow-ups, plus the Cursor round-4 finding. Removes `safe-regex2` entirely. Its last caller was `validateRegexPattern`, the PII custom-pattern boundary, where it was pure cost: it screens star height alone and passes `(a|a)*b` and `a*a*b`, while rejecting patterns that work — lookbehind and optional groups could not be saved as custom PII rules at all. Nor could any screen be sound there: those patterns execute in Presidio's Python engine, which backtracks on shapes RE2 accepts, so RE2-representability says nothing about their runtime. Validation is now syntax-only and the dependency is gone. Cursor flagged the chunker's lookaround fallback as reintroducing backtracking, which was correct. The fallback is removed. Verified first that no bounded probe can replace it: at a probe size small enough to survive an exponential pattern (24 chars), `(?=a*a*b)` completes in 0.0ms and passes, because polynomial blowup only shows on long input — small probes miss it and large probes hang, the same trap as the original guard. Instead `compileLookaroundSplit` puts the two idioms that actually need lookaround onto RE2: splitting on `(?=X)` is slicing at every match start of X, and on `(?<=X)` at every match end, neither of which needs the assertion. Both `(?=#\s)` and the pre-existing `(?<=</section>)` case keep working, now in linear time; anything else RE2 cannot represent is rejected with an actionable message rather than run on the backtracking engine. Also from the audit: - `literalRegex` recompiled its `RegExp` on every call to dodge `lastIndex` state from the `g` flag. VFS grep calls it per line, so that was a compile per line. It now keeps one non-global instance for `test`/`find` and a global one for `split`. - `validateRegex` and VFS grep log a warning when a pattern degrades. A guardrail rule that used lookaround now fails closed, which reads as the guardrail tripping on every input, and VFS grep's return shape has nowhere to report that a pattern was taken literally — both need to be findable in logs. Measured: RE2 costs 6-13x native on per-line matching (20k-line, 1.66MB file greps in 7ms), against ~100x on single multi-megabyte strings. * test(pii): drop the custom-pattern editor's backtracking-screen assertion CI caught this; my local runs only covered `lib/**` and missed the `.tsx` counterpart of removing `safe-regex2`. The editor asserted that `(a+)+$` renders "potentially unsafe". That screen is gone, so the assertion is inverted: the test now pins that `(a+)+$`, lookbehind and optional groups all render cleanly, which is the point of the removal — they are valid Presidio patterns that could not be saved before. Syntactically invalid patterns still surface "Invalid regex". * fix(chunkers): support lookaround with an affix, and keep JS whitespace semantics Three independent reviewers with no shared context audited the RE2 migration. Two found the same high-severity regression, and one found the divergence that silently rewrites stored data. Both are fixed here. **Lookaround with an affix threw.** `compileLookaroundSplit` only accepted a pattern that was *entirely* one assertion, so `(?<=\.)\s+` — split after the period — and `(?<=[.!?])\s+(?=[A-Z])` — the textbook sentence splitter — fell through to the chunker's `throw`. Those are persisted KB configs, and the compile runs in the constructor, so an existing knowledge base would stop re-ingesting with a hard error. The TSDoc claiming the bare assertions "cover the reason a split pattern reaches for lookaround" was simply wrong. Fixed generally rather than by adding another special case: splitting never needs the assertion, only the span the delimiter consumes, so `(?<=X)Y(?=Z)` compiles to `(?:X)(Y)(?:Z)` and group 1's span is exactly what `String.split` removes. One rule now covers every combination, and the four shapes above match `String.prototype.split` output exactly. **`\s` silently lost 14 codepoints.** RE2's `\s` is ASCII-only; ECMAScript's also covers `\v`, NBSP, U+2028/9, U+202F, U+3000, U+FEFF and the rest of `\p{Zs}`. A reviewer measured 9 of 35 realistic pattern/document pairs chunking differently through the real `RegexChunker` — NBSP from PDF/DOCX/HTML extraction, U+3000 in CJK, U+202F in French text. That rewrites stored chunk boundaries and embeddings for a document that never changed, with nothing thrown and nothing logged. `translateToRe2` now rewrites `\s`/`\S` to the verified-equivalent RE2 class, and `\uXXXX` to `\x{XXXX}` (RE2 rejects the former outright, turning a non-ASCII delimiter into a hard failure). Also from the audit: - `truncated`'s new TSDoc asserted an invariant the code violates — reaching `maxMatches` sets it even when nothing was left unread. Reworded to what it actually means. - The chunker's rejection message blamed "backreferences and lookaround" for repeat-count and `\uXXXX` rejections too. - `escapeRegExp` was a byte-identical copy of `executor/constants.ts:472` and exported for nobody; now module-private. - The guardrails wand prompt told the LLM to emit "valid JavaScript regex", which reliably produces `(?=.*\d)`-style patterns that now fail every check. - The PII editor's TSDoc still described the removed backtracking screen. - VFS grep's TSDoc omitted that invalid patterns now match literally. Test gaps the audit exposed: the match-time budget's only mechanism was unguarded (deleting the accumulation left every test green) — now covered by a mutation-verified test; the escape test survived dropping `.` from the class; the split-parity test dodged the one real divergence; and two test names described the built-in engine that no longer runs. * fix(security): correct three defects in the lookaround split decomposition A fourth reviewer, given no context beyond "these scanners are new and unreviewed", differential-tested them against the built-in engine over ~62k comparisons and found three real defects in code added an hour earlier. All were reachable through RegexChunker with plausible patterns, and all silently produced different chunks rather than failing. 1. **Top-level alternation was reshaped into a different pattern.** `(?<=\.)\s+|\n\n` means `((?<=\.)\s+)|(\n\n)`, but decomposing it produced `(?:\.)(\s+|\n\n)` — demanding the period before *both* branches. It could lose boundaries (`"Heading\n\nAlpha beta.\nGamma delta."` split in 2, not 3) and invent them. No grouping recovers the original meaning, so patterns whose middle alternates at the top level are now declined outright, which also removes an asymmetry: `\n\n|(?<=\.)\s` already threw. 2. **A capturing group inside the lookbehind stole group 1.** `(?<=(a))b` cut at the assertion instead of the delimiter, and where the group did not participate `start(1)` was -1, so `find` and `test` contradicted each other. The middle is now captured by name, immune to numbering. 3. **Consuming the assertion text swallowed every other boundary.** The compiled form matched `behind + middle + ahead`, so any boundary whose lookahead doubles as the next one's lookbehind was skipped: `(?<=\w)\s+(?=[A-Z])` over `"A B C D"` split 2 of 3 gaps. Each iteration now resumes at the end of the delimiter rather than the end of the match. This was documented as affecting only self-overlapping delimiters; that was wrong and far too narrow. The reviewer confirmed `translateToRe2` and `closingParen` clean across exhaustive escape-state, character-class, and paren-matching probes, and that nothing added is superlinear. One divergence remains and is now documented rather than overstated: when delimiters themselves overlap (a single-character middle whose matches abut, as in `(?<=\w).(?=\w)`), a match starting behind the cursor is dropped instead of emitting the empty segment the built-in engine produces. Splitters that consume whitespace or punctuation between tokens do not overlap and match exactly. * test(security): pin engine parity with a differential suite Every defect in this module has been a silent semantic divergence rather than a crash, and each was found by comparing engines over a corpus in a throwaway script. That comparison now lives in the suite: ~700 pattern/document pairs plus match/index/ignore-case parity, checked against the built-in engine on every run, with the accepted divergences enumerated and individually pinned. It earned its place immediately by falsifying my own documentation. The "self-overlapping delimiter" caveat was stale — fixing the boundary-advance in the previous commit also made `(?=#{1,6}\s)`, `(?=aa)` and `(?=##)` exact, so the entry asserting they still diverge failed. One real divergence remains and is now stated precisely: a lookbehind whose body self-overlaps (`(?<=aa)` over `aaaaa`). Making that exact means restarting the scan one character past each match start, which is quadratic on a multi-megabyte document and forfeits the linear guarantee — so it is a deliberate trade, not an oversight. Mutation-verified against the three bugs that actually escaped review: disabling the `\s` translation fails 12 tests, changing the boundary-advance fails 2, and removing the top-level-alternation guard fails 3. That last one initially passed, because the corpus had no alternation pattern in it — the gap is closed and the mutation now fails as it should.
…5995) * fix(files): serve rendered documents instead of generator source Generated docs (docx/pptx/pdf/xlsx) store their generation source as the primary file; the rendered binary lives in a separate content-addressed artifact store. resolveServableDocBytes is the chokepoint that swaps one for the other, and the serve route, single-file download and ~50 tool routes all go through it. Archive compression and the public v1 download read raw bytes instead, so a generated document arrived as source text under a .docx name and Word reported it as corrupt. Both now resolve through the servable reader. Adds fetchServableWorkspaceFileBuffer beside the raw reader so the record to UserFile mapping lives in one place, preserving storageContext; the raw reader's doc comment now says it returns generation source, so the next call site has to choose deliberately. v1 also has to send the resolved content type rather than the record's source MIME, and returns a retryable 409 rather than a 500 when an artifact is still compiling. docNotReadyMessage centralizes the 409 copy, and isRenderableDocumentName is shared so the read path and its callers agree on which extensions can expand. * perf(files): stream workspace archives instead of buffering them The bulk download route materialized every selected file before writing a byte, so peak memory tracked the size of the selection. Ordinary files are now appended as lazy streams: each opens its storage read only when the archiver reaches it, so one entry is resident at a time rather than the whole archive. Generated documents still resolve to buffers first. They are the only entries whose bytes decide anything, and every status this route returns comes from them — once the first byte is written the status code is committed, so those decisions have to happen before the archive starts. The per-entry allowance and byte budget therefore govern documents only. archiver processes appended entries through a sequential queue; lazystream defers each storage read until that entry's turn, since handing the archiver an open stream per entry would hold more connections than the storage client pools. nodeReadableToWebStream moves out of input-validation.server.ts into a shared util rather than being written twice: Readable.toWeb throws ERR_INVALID_STATE when a consumer cancels while the source is still flowing, which is exactly what a cancelled download does. Trade: a storage read failing mid-archive truncates the response rather than returning 500, since the status is already sent. Documents cannot hit this. The table export route already behaves this way. * improvement(files): surface archive download errors in place Bulk and folder downloads navigated to the API route, so any rejection replaced the Files page with the raw JSON error body. Single-file download already fetched and saved the blob, so the two paths had diverged. That was survivable when the only failures were "too many files" and "too large"; resolving rendered documents adds a 409 for a still-compiling artifact, which is reachable in normal use. Both archive paths now fetch the zip and show the server's message as a toast — the route writes that copy for a person, so it is worth surfacing rather than discarding. Single-file download shows its error too instead of only logging it. * improvement(files): simplify the archive route and stop buffering real uploads Four review passes over the branch converged on the same points. Routing every office extension through the buffered path held an entire selection of genuinely uploaded documents in memory — for a check the resolver settles on the first few magic bytes. Only files stored under a generator-source content type need resolving; a real .docx serves what is stored and now streams like anything else, which is what the change was for. With resolution sequential, the AbortController cancelled nothing (its signal never reached the storage read), the success-path over-limit branch was unreachable, and the cancellation guard in the catch could not fire. A plain loop that returns at the point of failure replaces the outcome record, the sentinel, the fan-out helper at limit 1, and four post-hoc scans. ZIP_MATERIALIZE_CONCURRENCY was dead, and the aggregate over-limit message reported a byte count that was by construction under the limit. lazystream and its types are gone: Readable.from over an async generator defers the open the same way and propagates source errors natively, so the PassThrough relay went with them. The client uses requestRaw with the existing binary contract instead of a hand-built query string and a re-typed error extractor, and both archive call sites share one callback. The render headroom moves beside isRenderableDocumentName, the servable reader stops minting a request id per file, and the v1 download returns a view rather than a second full copy. * improvement(files): keep the lazy entry stream in byte mode Readable.from defaults to object mode; these chunks are bytes headed for an archive, so the mode is now explicit. Also makes the fire-and-forget bulk download call consistent with its sibling. * improvement(files): correct the servable reader's 409 note Batch callers build the response from docNotReadyMessage rather than through docNotReadyResponse, so the doc comment now describes the outcome instead of naming one of the two helpers. * improvement(files): correct two comments that described the wrong behavior The resolution loop's comment claimed at most one rendered document is resident. It is not: every resolved buffer is held until the archive is assembled, bounded by the request's remaining budget. The loop resolves one at a time, it does not release one at a time. RENDERED_DOCUMENT_HEADROOM_BYTES was documented as bounding the expansion beyond the declared size, but it is passed straight through as maxBytes and is an absolute ceiling on the rendered document — renamed to MAX_RENDERED_DOCUMENT_BYTES so the name matches. Download failures also carry a fallback message, so a transport error surfaces something better than a bare "Failed to fetch". * fix(files): put streamed files and rendered documents on one byte budget The budget only counted rendered documents, so streamed entries never reserved their declared bytes. A mixed selection could clear the up-front declared-size gate and still ship close to two full limits: ordinary files up to the cap, plus documents drawing a fresh cap of their own. Streamed entries ship exactly what they declared, so their share is known before anything is read and is now reserved up front; documents draw from what is left. The budget-exhausted rejection also quoted declared sizes, which for a selection of small sources reads as a tiny total exceeding the limit. That case has no knowable byte count — the documents have not been rendered — so it now says what happened without inventing a number. * fix(files): attach the download anchor and handle a finalize rejection A detached anchor's click() works in current browsers, but every other download helper in the app attaches first, and a silent no-op on this path would look exactly like a download that never started. Not worth the ambiguity for two DOM operations. archive.finalize() was fired with void, so a failure after the response had started became an unhandled rejection on top of the stream error event that already fails the response. It is caught and logged now. * fix(files): guard the archive download against concurrent clicks Navigating to the route meant a second click just re-navigated. Fetching the archive instead means each click starts another download that holds the whole zip in tab memory, and the Download button gave no sign anything was happening. The button now reflects the in-flight download alongside the existing move and delete states. The ref guard is there as well as the state because two clicks in the same tick would both pass a state check. * fix(files): resolve every generated-document source type, not four of five The archive keyed off a locally enumerated set of source content types that omitted text/x-pdflibjs — the isolated-vm PDF generator. Those files failed the check and streamed their generator source under a .pdf name, which is the exact corruption this change exists to fix. A canonical five-member set already existed in the file viewer; enumerating a fourth copy was the mistake, not the missing string. The set moves to file-utils beside the other file-type predicates, the viewer imports it, and the route asks isGeneratedDocumentSourceType rather than carrying its own list. A parameterized test pins all five, so adding a generator without extending the set fails. * test(files): validate the produced archive with a real zip reader Every existing test mocks the storage stream and reads the result back with the same JS library that wrote it, which cannot catch the failure this route exists to fix: an archive a real zip reader will not open. This drives 5 MB of non-repeating bytes from an fs stream through archiver, the lazy entry generator and the Node-to-web bridge, then checks the output with the operating system's unzip and compares the extracted bytes. Skips rather than fails where unzip is unavailable. * fix(files): bound the markdown export's embedded-asset bundling The embed list is produced by scanning the document body, so its length and the bytes behind it are whatever the author put there. Every referenced asset was downloaded at once with no concurrency bound, no per-file cap and no total cap, so one request could materialize an unbounded number of unbounded files. Its sibling bulk-download route already had a count cap and a byte cap. Metadata is now resolved first, which bounds the download from declared sizes before a byte is read and moves the authorization check off the download path. Assets are then fetched with bounded concurrency and a per-file cap; a single unreadable or oversized asset drops out of the bundle rather than failing the export, which is what the previous allSettled did. * fix(files): mount rendered documents into the sandbox, not generator source Mounting a generated document handed the sandbox its generator source under a .docx name, so a python-docx or openpyxl script failed on a file that looked fine and the agent debugged code that was correct. Both branches were wrong, not just the buffered one: with cloud storage the mount presigns record.key, which is the raw source object, so swapping the buffered read alone would have fixed only local storage. Source-backed documents now always take the servable path — they are bounded by the render ceiling, so routing them through the web process instead of presigning is affordable. The text/binary decision also keyed off record.type, which for these files is text/x-docxjs, so a resolved binary would have been decoded as UTF-8. It reads the resolved content type now. * chore(deps): regenerate the lockfile against staging's dependency rework Staging reworked the OTel split, dropped dead deps and declared emcn peers, so the lockfile is regenerated on top of that rather than carrying a stale merge. The archive dependency is the only addition; lazystream stays as archiver's transitive dep now that it is no longer declared directly. Regenerated incrementally rather than from scratch: a clean resolve fails the bunfig age gate because minimumReleaseAgeExcludes lists only the darwin and linux-gnu @next/swc binaries, not the musl and windows variants that a full re-resolve also pulls. * fix(files): budget sandbox mounts on rendered bytes, not declared source size A source-backed document declares the size of its generator, not of the document, so the per-file and aggregate mount pre-checks were reading a number unrelated to what was about to be mounted — tiny sources cleared the guard and only afterwards was the running total updated with the real length. Those pre-checks now apply only to files that serve what they declare. A document is capped by the read instead, at the smaller of the per-file limit and the budget left, and a size rejection is reported in the same terms as the other mount limits. Same invariant the archive route already had to learn: declared size is not a bound once bytes can be rendered. * test(files): cover the two surfaces added without tests The markdown export route had no test file at all, and the sandbox mount's source-backed branch was exercised by nothing — both were changed in this PR and one of them shipped a defect a reviewer caught within minutes. Export: the embed-count rejection, the declared-bytes rejection landing before any asset leaves storage, the per-asset download cap, an unreadable asset dropping out rather than failing the export, and an unauthorized asset never being read. Mount: a generated document never presigning its raw key even on cloud storage, its rendered bytes mounting as base64 rather than being utf-8 decoded off the source MIME, and the budget rejecting on rendered length. * fix(files): stop the export caps from rejecting documents that used to work The caps I picked would have failed exports that previously succeeded: a screenshot-heavy document can hold more than 100 embeds, and 100 embeds of unoptimised PNGs can exceed 100 MB. Adding a limit to bound memory is right; setting it below real usage turns a memory risk into a broken feature. The byte ceiling now matches the bulk-download route, so the two export surfaces reject at the same size rather than at two invented ones. The count is only a guard on the metadata lookups that run before the byte check, so it moves well above any hand-authored document instead of sitting where a legitimate one could reach it.
…API (#5999) * feat(api): add workflow export and import endpoints to the public v1 API Adds GET /api/v1/workflows/[id]/export and POST /api/v1/workflows/import. The export envelope is accepted verbatim by import, so workflows round-trip between workspaces over the public API. Unlike the admin export, the public export is secret-sanitized: stored credentials and password fields are redacted while {{ENV_VAR}} references and block positions are preserved. Import regenerates block, edge, loop and parallel ids and de-duplicates the workflow name against the target folder. Also moves parseWorkflowVariables out of the admin types module into lib/workflows/variables/parse.ts so the public route does not import from the admin namespace. * fix(api): make workflow import atomic and clarify what export redacts Writes the imported graph and its variables in a single transaction and deletes the shell workflow row on any failure, so a caller that receives an error is never left with a partially imported workflow. Previously a throw from the variables update returned 500 while leaving the workflow behind with an empty variables map. Also narrows the export route's sanitization claim: workflow variables are emitted as stored, matching GET /api/v1/workflows/[id] and the in-app export. They are plaintext configuration readable at the same permission level this route requires; secrets belong in environment variables, which travel as unresolved references. * fix(api): close import defects found in audit and share one write pipeline Security: - Escape block names before interpolating them into a RegExp in updateValueReferences. Names reach it straight from imported workflow JSON and normalizeWorkflowBlockName preserves regex metacharacters, so a name like `a*a*a*a*b` compiled to a catastrophically backtracking pattern. A sub-kilobyte body blocked the event loop for 50s and grew exponentially. Also skip rename-to-itself, which is the entire map on the import path, so the scan no longer runs at all there. - Validate folder ownership before folder lock state, so a locked folder in another workspace can no longer be distinguished from a missing one. Correctness: - Gate the imported graph on workflowStateSchema, the same schema the canonical PUT /api/workflows/[id]/state path enforces. Without it a valid 201 could persist a block field of the wrong type, which then threw on every subsequent read and left a workflow nothing could open. - Guard the compensating delete so a failed rollback logs the orphaned id instead of vanishing into a generic 500. - Validate variable `type` against the enum and build the record on a null-prototype object, so a `__proto__` key no longer silently drops the variable. - Bound payload-derived names and descriptions to the same limits the contract declares for the explicit overrides. - Return the description as stored rather than coercing '' to null, matching GET /api/v1/workflows/[id]. Shared code, so the two write paths cannot drift: - Extract prepareWorkflowStateForPersistence and use it from both PUT /api/workflows/[id]/state and the v1 import route: agent-tool sanitization, block backfill, dangling-edge removal, and loop/parallel recomputation now have one implementation. - Persist inline custom tools on import, which the canonical path already did. - Move variable normalization into lib/workflows/variables and repoint the admin importer at it, removing the last duplicate. Docs: - OpenAPI: oneOf -> anyOf on the import body. WorkflowExport matches any object, so every valid object payload matched two branches and failed validation under any spec-driven validator. Document 423 and the loss of workspace-scoped bindings on export. Tests: prepare-state unit tests and a real export -> import round trip with no mocks of the sanitizer or parser, covering loop/parallel children and the regex-metacharacter payload. * fix(api): cap import names inside the bound and align the three import paths - `truncate` appends its suffix after slicing, so capping at the contract limit produced 203/2003-character values — past the very bound the cap exists to enforce, and into the headroom reserved for dedup suffixes. Reserve the ellipsis inside the limit. - Match `extractWorkflowName`'s candidate order (state.metadata.name before workflow.name) and trim, so the v1 API and the in-app importer resolve the same name for the same payload. Previously a hand-authored payload carrying both could yield two different names. - Run the admin importer through prepareWorkflowStateForPersistence too. It was writing raw parsed state, so a dangling edge tripped the workflow_edges foreign key and a block missing its backfilled columns could land unopenable — the same class this PR just closed on the v1 path.
The workspace importer guarded its variable write on `Array.isArray`, but `parseWorkflowVariables` — what the matching workspace export emits — returns the record form. Every variable was silently dropped, so exporting a workspace and importing it back lost all of them. It now uses the shared `normalizeImportedVariables`, which accepts both shapes. Also runs the workspace importer through `prepareWorkflowStateForPersistence`, the pipeline the editor, the v1 import API and the single-workflow admin import already share. Without it this route wrote raw parsed state, so a dangling edge tripped the `workflow_edges` foreign key and failed the restore, and blocks missing their backfilled columns could land unopenable. Repoints `persistImportedWorkflow` at the same helper, removing the last inline copy — all four import paths now normalize variables identically. Adds the first tests for these helpers, including the export -> import round trip that pins the record form the bug dropped.
…5979) * improvement(tables): announce locks on open instead of a header chip Drop the lock entry from the table header actions. A locked table now announces itself once on open via an info toast carrying the Lock settings action for admins; the breadcrumb dropdown remains the permanent entry point. * fix(tables): wait for permissions before announcing table locks The lock announcement fires once per table, so a canAdmin that is still false because permissions have not resolved permanently drops the toast's Lock settings action. Arm the one-shot on the permissions decision rather than on table resolution. * fix(tables): keep the lock announcement from being swept on navigation The toast provider clears route-scoped toasts in a pathname effect that runs after child effects, so an announcement fired on a warm-cache navigation was added and removed in the same commit. Opt these toasts out of the sweep and dismiss them on unmount so they still don't trail the user. * fix(tables): drop the lock notice when its action stops being valid A toast's action is captured at creation, so a viewer whose admin access is revoked while the announcement is on screen kept a Lock settings button that opened nothing. Dismiss the notice when the action is no longer available. * fix(tables): only drop the lock notice when access is actually lost The dismiss effect treated "has no access" the same as "just lost access", so on a warm-cache mount — where the announcement and the dismiss both run in the mount commit — a non-admin's notice was torn down the moment it was created. Dismiss on the true-to-false transition only. * fix(tables): clear the lock notice when switching tables Switching tables reuses this component rather than remounting it, and the notice is exempt from the provider's route sweep, so a locked table's announcement stayed on screen over the next table — where its action would have opened that table's lock settings instead. Key the cleanup on tableId. * fix(emcn): sweep route-scoped toasts during render, not after child effects The provider cleared route-scoped toasts from a pathname effect. Effects run child-first, so it also swept toasts the newly rendered route had just raised: any toast added from a child's mount effect — which is what happens whenever that child's data is already cached — was appended and filtered out in the same commit, before it painted. Move the sweep into render, where it runs before children render, so only toasts predating the navigation are cleared. Timers and heights were already reconciled by effects keyed off `toasts`, so this stays a pure state update. Drops the persistAcrossRoutes workaround the table lock notice needed to dodge the bug; its tableId-keyed cleanup stays, covering embedded swaps that change the prop without a route change. * fix(tables): reset the announce latch when leaving a table The tableId cleanup dismissed the notice on departure but left the latch holding that table's id, so returning before another table announced — a quick there-and-back, or a second table that never loaded — found the latch matching and stayed silent. Leaving ends the visit, so clear the latch with it.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Import paths are aligned so admin and workspace bulk import use shared Files and downloads now serve rendered artifacts for generated documents (v1 file API, copilot mounts, tool compress, markdown zip exports) instead of generator source, with 409 when still compiling. Workspace bulk zip download is rebuilt to stream via User-supplied regex moves to a linear-time RE2 path for copilot/VFS grep, log grep, and regex chunking; catastrophic-backtracking probes and Smaller changes: landing hero H1 leads with “AI workspace” keywords (docs/rules updated), table locks announce via an on-open info toast instead of a header chip, changelog uses Reviewed by Cursor Bugbot for commit e5ae445. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e5ae445. Configure here.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Greptile SummaryAdds public v1 workflow export/import, hardens admin/in-app import via shared prepare-state and variable normalization, streams workspace zip downloads with rendered-doc resolution, and moves user-pattern matching onto RE2-backed linear regex, plus SEO/deps/toast polish.
Confidence Score: 3/5Not safe to merge until admin import persists variables atomically with the graph (or rolls back the workflow row on variables write failure). Admin single and bulk import can leave a fully saved workflow with empty variables while returning failure after the new variables path runs; public import already avoids this. Filename/content-type mismatch on rendered downloads is a lesser follow-up. Files Needing Attention: apps/sim/app/api/v1/admin/workflows/import/route.ts, apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts, apps/sim/app/api/v1/files/[fileId]/route.ts
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/v1/workflows/import/route.ts | New public import with write authz, body size cap, schema gate, transactional graph+variables persist, and custom-tool side effects. |
| apps/sim/app/api/v1/workflows/[id]/export/route.ts | New public export with read authz, sanitizeForExport, edge projection, and audit logging. |
| apps/sim/app/api/v1/admin/workflows/import/route.ts | Shares prepare-state and variable normalization but still writes variables outside a transaction without rollback. |
| apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts | Fixes record-form variable drop on bulk restore but keeps the same non-atomic variables write pattern. |
| apps/sim/app/api/workspaces/[id]/files/download/route.ts | Streams zip via archiver; pre-renders generated docs; ordinary files stream without runtime byte caps beyond declared size. |
| apps/sim/lib/core/security/linear-regex.ts | RE2JS-based matcher with limited lookaround split recovery; call sites fail closed or fall back to literal search. |
| apps/sim/lib/workflows/persistence/prepare-state.ts | Shared server write normalization for blocks, edges, loops, and parallels used by editor state PUT and imports. |
Sequence Diagram
sequenceDiagram
participant Client
participant Export as GET /api/v1/workflows/{id}/export
participant Import as POST /api/v1/workflows/import
participant Authz as validateWorkspaceAccess
participant DB as Postgres
Client->>Export: API key + workflow id
Export->>Authz: read on workspace
Export->>DB: load normalized state + variables
Export-->>Client: sanitized export envelope
Client->>Import: workspaceId + envelope
Import->>Authz: write on workspace
Import->>Import: parseWorkflowJson, normalize vars, prepare state
Import->>DB: create workflow + txn(save graph, vars)
Import-->>Client: 201 ImportedWorkflow
Comments Outside Diff (1)
-
apps/sim/app/api/v1/admin/workflows/import/route.ts, line 121-137 (link)Non-atomic admin variable write
When admin single or bulk import saves the graph successfully and the following variables UPDATE throws (or the process dies between the two writes), the handler returns failure while leaving the workflow row with empty
variablesand a fully persisted graph. Retries create duplicates and bulk restore reports failure while partial rows remain; the public v1 import already writes graph and variables in one transaction.
Reviews (1): Last reviewed commit: "improvement(tables): announce locks on o..." | Re-trigger Greptile
6 tasks
* chore(trigger): upsize instances * chore(trigger): upsize tasks
icecrasher321
changed the title
* fix(files): count the document body against the export limit The 250 MB export cap measured only the embedded assets' declared sizes. The markdown body was downloaded with no limit and never counted, so a large document with modest attachments cleared the check and still produced a zip well over the stated limit, materialized whole in memory. The body is the largest single entry in most bundles, so excluding it left the limit unenforced against the item most able to exceed it. It is now capped on read and counted alongside its assets, and the message names both. Follow-up to #5995, which introduced the asset caps without extending them to the body. * test(v1): cover the file download's rendered-bytes behavior The route had no tests, and #5995 changed what it serves: rendered bytes, the resolved content type rather than the record's source MIME, Content-Length from the rendered length, and a retryable 409 while an artifact compiles. One test pins the filename/content-type relationship. A review flagged the download as naming a rendered file with a source extension, but the renderer picks its output format from the file name — getE2BDocFormat and COMPILABLE_FORMATS both key on it — so a .docx renders to a docx and the two cannot disagree. The test makes that argument executable rather than a comment. * fix(files): report an oversized document body as a size rejection Capping the body read meant an oversized document threw PayloadSizeLimitError, which nothing caught, so the caller got a generic 500 — hiding the very limit message the cap was added to produce. It now returns the same 400 as the bundle check, naming the export limit. * chore(files): drop the unreachable export asset-count cap extractEmbeddedFileRefs stops collecting at MAX_EMBEDDED_IMAGES, so the list this route receives is already bounded before it arrives and the count check could never fire. Its test only passed because it mocked the extractor, so it asserted a branch production cannot reach. The byte ceilings are the real bound and stay. A comment records where the count is actually enforced, so the next reader does not add a second one.
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.
Uh oh!
There was an error while loading. Please reload this page.