Skip to content

Commit 970e6d6

Browse files
avanelsasclaude
andauthored
review pass: load-boundary hardening + codegen dedup (#64)
Route IDB autosave restore through `project-file/classify-payload` + `sanitize-doc` so a corrupted or tampered autosave can't bypass the checks the file-open path uses. Strict-reject unknown `:version` so a future schema bump fails loud instead of partially installing the wrong shape. Cancel the watcher-armed 750ms timer on file load so a stale `save-now!` can't fire after `clear-autosave!`. Surface autosave write failures as `[:ui :autosave-failed?]` with a hidden ⚠ indicator in the toolbar. Extend `unsafe-findings` with an identifier-shape scanner so attr keys, binding prop names, binding `:field`, trigger `:action-ref`, payload `:field`, field-def `:name`, and action `:name` that carry characters unsafe for codegen are refused at the load boundary — closes the JS / CLJS string-injection vectors in both export plugins. Extract `action-ref-canonical-ns` + `action-ref-alias` to `export.model` (single source of truth shared by both plugins). Route `cljs_project/emit-sub-group-child` through `em/resolve-template-source` so the legacy fallback that re-derived `stateful-host-for-template` is gone. Replace the `volatile!` in `dnd/drag/marquee-hits` with an `into` + `keep` pipeline. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 996bcd4 commit 970e6d6

14 files changed

Lines changed: 429 additions & 109 deletions

File tree

public/index.html

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,18 @@
9393
background: var(--x-color-border, rgba(127, 127, 127, 0.35));
9494
}
9595
.toolbar-icon-button span { font-size: 16px; line-height: 1; }
96+
.toolbar-autosave-warn {
97+
display: inline-flex;
98+
align-items: center;
99+
justify-content: center;
100+
width: 22px;
101+
height: 22px;
102+
margin-left: 6px;
103+
color: var(--x-color-warning, #c97a00);
104+
font-size: 16px;
105+
cursor: help;
106+
}
107+
.toolbar-autosave-warn[hidden] { display: none; }
96108
.panel {
97109
border-right: 1px solid var(--x-color-border, rgba(127, 127, 127, 0.2));
98110
padding: 12px;

src/bareforge/dnd/drag.cljs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -229,22 +229,22 @@
229229
(let [^js br (.getBoundingClientRect host)
230230
sl (.-scrollLeft host)
231231
st (.-scrollTop host)
232-
^js nodes (.querySelectorAll host "[data-bareforge-id]")
233-
out (volatile! [])]
234-
(dotimes [i (.-length nodes)]
235-
(let [^js el (.item nodes i)
236-
id (.getAttribute el "data-bareforge-id")]
237-
(when (and id (not= id "root"))
238-
(let [^js eb (.getBoundingClientRect el)
239-
left (+ (- (.-left eb) (.-left br)) sl)
240-
top (+ (- (.-top eb) (.-top br)) st)
241-
rect {:left left
242-
:top top
243-
:right (+ left (.-width eb))
244-
:bottom (+ top (.-height eb))}]
245-
(when (rects-overlap? marquee-rect rect)
246-
(vswap! out conj id))))))
247-
@out))
232+
^js nodes (.querySelectorAll host "[data-bareforge-id]")]
233+
(into []
234+
(keep (fn [i]
235+
(let [^js el (.item nodes i)
236+
id (.getAttribute el "data-bareforge-id")]
237+
(when (and id (not= id "root"))
238+
(let [^js eb (.getBoundingClientRect el)
239+
left (+ (- (.-left eb) (.-left br)) sl)
240+
top (+ (- (.-top eb) (.-top br)) st)
241+
rect {:left left
242+
:top top
243+
:right (+ left (.-width eb))
244+
:bottom (+ top (.-height eb))}]
245+
(when (rects-overlap? marquee-rect rect)
246+
id))))))
247+
(range (.-length nodes)))))
248248

249249
(defn- commit-marquee! [^js e]
250250
(let [^js host (canvas-el)

src/bareforge/doc/sanitize.cljs

Lines changed: 113 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,20 @@
22
"Pure sanitisation helpers for the two doc fields that ship raw
33
user-supplied strings into the DOM: `:inner-html` (raw SVG/HTML on
44
`:raw-html-slot?` components like x-icon) and `:attrs` URL values
5-
(`href`, `src`, …).
5+
(`href`, `src`, …). Plus an identifier-shape scanner that guards
6+
the export pipeline against doc fields that the codegen later
7+
interpolates verbatim into emitted JS / CLJS source.
68
7-
Two layers of protection:
9+
Three layers of protection:
810
911
- **Block-list scanners** (`unsafe-findings`): given a doc, return a
1012
vector of `[path reason]` entries naming each suspect site. Used at
1113
the load boundary by `storage/project-file/validate-project` to
1214
refuse a malicious payload outright with an explanatory message.
15+
Covers XSS payloads in `:inner-html` / URL attrs **and** unsafe
16+
identifier shapes in attr keys, binding keys, field / action
17+
names, and trigger action-refs (everything codegen splices into
18+
a JS or CLJS string literal).
1319
1420
- **Best-effort sanitisers** (`sanitize-svg-fragment`,
1521
`sanitize-doc`): strip the obvious payloads — `<script>` /
@@ -117,6 +123,99 @@
117123
;; with javascript-like schemes.
118124
(str/replace dangerous-url-attr-re ""))))
119125

126+
;; --- identifier safety ---------------------------------------------------
127+
128+
(def ^:private safe-attr-key-re
129+
;; HTML / SVG / ARIA attribute name. Letters or `_` to start; letters,
130+
;; digits, `-`, `_`, `.`, `:` after (covers `xlink:href`, `aria-*`,
131+
;; CSS-variable-shaped attrs). Refuses anything codegen would have to
132+
;; escape — whitespace, quotes, backslash, parens, brackets, `;`, `#`,
133+
;; comment markers, etc.
134+
#"^[A-Za-z_][A-Za-z0-9_\-.:]*$")
135+
136+
(def ^:private safe-identifier-name-re
137+
;; Local-name component of a keyword (field / action / action-ref
138+
;; local) emitted into CLJS or JS source. Tighter than attr keys:
139+
;; no colons (colons inside a local keyword name would re-namespace
140+
;; it at the keyword reader; in JS strings they're a syntax hazard
141+
;; on object literals).
142+
#"^[A-Za-z][A-Za-z0-9_\-.]*$")
143+
144+
(defn- safe-attr-key? [s]
145+
(and (string? s) (boolean (re-matches safe-attr-key-re s))))
146+
147+
(defn- safe-identifier-name? [s]
148+
(and (string? s) (boolean (re-matches safe-identifier-name-re s))))
149+
150+
(defn- keyword-name-safe? [kw]
151+
(and (keyword? kw) (safe-identifier-name? (cljs.core/name kw))))
152+
153+
(defn- action-ref-safe?
154+
"True when a qualified keyword's namespace segments and local name
155+
all parse as safe identifiers. Codegen splices each piece into a
156+
JS string literal (`dispatch([\"<alias>/<name>\"])`); anything
157+
outside `safe-identifier-name-re` can break out of that string."
158+
[kw]
159+
(and (qualified-keyword? kw)
160+
(every? safe-identifier-name? (str/split (namespace kw) #"\."))
161+
(safe-identifier-name? (cljs.core/name kw))))
162+
163+
(defn- node-identifier-findings
164+
"Collect identifier-shape findings for one node. The codegen splices
165+
each of these values into emitted source code; an unsafe character
166+
in any of them produces malformed (or malicious) output."
167+
[path node]
168+
(concat
169+
;; Attribute keys — both static `:attrs` and `:bindings`.
170+
(for [[k _] (:attrs node)
171+
:when (not (safe-attr-key? k))]
172+
{:path (conj path :attrs k)
173+
:reason (str "attr key " (pr-str k)
174+
" contains characters unsafe for codegen")
175+
:preview (pr-str k)})
176+
(for [[k _] (:bindings node)
177+
:when (not (safe-attr-key? k))]
178+
{:path (conj path :bindings k)
179+
:reason (str "binding prop name " (pr-str k)
180+
" contains characters unsafe for codegen")
181+
:preview (pr-str k)})
182+
;; Binding `:field` keywords — emitted as the dispatched setter name.
183+
(for [[_ b] (:bindings node)
184+
:when (and (:field b) (not (keyword-name-safe? (:field b))))]
185+
{:path (conj path :bindings)
186+
:reason (str "binding :field " (pr-str (:field b))
187+
" contains characters unsafe for codegen")
188+
:preview (pr-str (:field b))})
189+
;; Trigger `:action-ref` qualified keywords.
190+
(for [t (:events node)
191+
:when (and (:action-ref t) (not (action-ref-safe? (:action-ref t))))]
192+
{:path (conj path :events)
193+
:reason (str "trigger :action-ref " (pr-str (:action-ref t))
194+
" contains characters unsafe for codegen")
195+
:preview (pr-str (:action-ref t))})
196+
;; Payload entries that reference fields by name.
197+
(for [t (:events node)
198+
pe (:payload t)
199+
:when (and (:field pe) (not (keyword-name-safe? (:field pe))))]
200+
{:path (conj path :events)
201+
:reason (str "trigger payload :field " (pr-str (:field pe))
202+
" contains characters unsafe for codegen")
203+
:preview (pr-str (:field pe))})
204+
;; Field-def names + action names (referenced verbatim by setter
205+
;; / sub / handler emission).
206+
(for [fd (:fields node)
207+
:when (and (:name fd) (not (keyword-name-safe? (:name fd))))]
208+
{:path (conj path :fields)
209+
:reason (str "field-def :name " (pr-str (:name fd))
210+
" contains characters unsafe for codegen")
211+
:preview (pr-str (:name fd))})
212+
(for [a (:actions node)
213+
:when (and (:name a) (not (keyword-name-safe? (:name a))))]
214+
{:path (conj path :actions)
215+
:reason (str "action :name " (pr-str (:name a))
216+
" contains characters unsafe for codegen")
217+
:preview (pr-str (:name a))})))
218+
120219
;; --- doc walker -----------------------------------------------------------
121220

122221
(defn- walk-nodes-with-path
@@ -137,8 +236,16 @@
137236

138237
(defn unsafe-findings
139238
"Walk `doc` and return a vector of `{:path :reason :preview}` maps,
140-
one per unsafe `:inner-html` or URL-typed attr value. Empty
141-
vector means the doc is clean. The load-boundary check uses
239+
one per unsafe site. Three families:
240+
241+
- `:inner-html` carrying a script / event / javascript-url payload.
242+
- URL-typed attrs (`href`, `src`, …) with a dangerous scheme.
243+
- Identifier-shaped fields the exporter splices into emitted source
244+
(attr keys, binding prop names, binding `:field`, trigger
245+
`:action-ref`, payload `:field`, field-def `:name`, action
246+
`:name`) that contain characters unsafe for codegen.
247+
248+
Empty vector means the doc is clean. The load-boundary check uses
142249
this directly: any non-empty result refuses the load."
143250
[doc]
144251
(vec
@@ -154,7 +261,8 @@
154261
:when (and (url-attr? k) (string? v) (not (safe-url? v)))]
155262
{:path (conj path :attrs k)
156263
:reason (str "attr " (pr-str k) " carries unsafe URL scheme")
157-
:preview v})))
264+
:preview v})
265+
(node-identifier-findings path node)))
158266
(walk-nodes-with-path doc))))
159267

160268
(defn- sanitize-node-attrs

src/bareforge/export/cljs_project.cljs

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -176,24 +176,11 @@
176176
(def ^:private collect-trigger-payload-fields em/collect-trigger-payload-fields)
177177
(def ^:private collect-trigger-action-refs em/collect-trigger-action-refs)
178178

179-
(defn- action-ref->alias
180-
"Turn an action-ref qualified keyword like :app.cart.events/add-to-cart
181-
into the require alias string: \"cart.events\". Matches the
182-
dot-notation alias convention used elsewhere in the generator.
183-
184-
The group segment is passed through `actions/name->ns-segment`
185-
so an action-ref that was committed before the
186-
`bareforge.doc.actions/action-ref` canonicalisation fix (e.g.
187-
`:app.Dashboard.events/tick` in an older doc) still emits a
188-
lowercase require — `[app.dashboard.events :as dashboard.events]`
189-
matching the file at `src/app/dashboard/events.cljs`."
190-
[ref]
191-
(let [ns (namespace ref)
192-
first-dot (.indexOf ns ".")
193-
last-dot (.lastIndexOf ns ".")
194-
group-seg (subs ns (inc first-dot) last-dot)
195-
suffix (subs ns last-dot)] ;; ".events" or ".subs"
196-
(str (actions/name->ns-segment group-seg) suffix)))
179+
(def ^:private action-ref->alias
180+
"Shared action-ref → require-alias helper. Lives in the export
181+
model so the cljs and vanilla-js plugins agree byte-for-byte on
182+
the canonical form (see `em/action-ref-canonical-ns` for why)."
183+
em/action-ref-alias)
197184

198185
;; --- :write / :read-write binding → DOM event handler ---------------------
199186
;;
@@ -390,32 +377,36 @@
390377
template sub-group emits its iteration form; later encounters
391378
return nil so a parent with N seed-backed clones still emits
392379
one iteration. Singleton sub-groups return the plain
393-
`(<ns>.views/<ns>)` call as a `:raw` value."
380+
`(<ns>.views/<ns>)` call as a `:raw` value.
381+
382+
Template-source resolution routes through
383+
`em/resolve-template-source` — single source of truth shared with
384+
the vanilla-js plugin, so both targets pick the same source for
385+
every instance."
394386
[ctx child gname tpl? rendered-tpls]
395387
(let [{:keys [doc all-groups field-owner-ns-map]} ctx]
396388
(cond
397389
(and tpl? (contains? rendered-tpls gname))
398390
[nil rendered-tpls]
399391

400392
tpl?
401-
(let [;; Fall back to the (single) collection field that
402-
;; points at this template when the instance has no
403-
;; explicit :source-field / :source-sub set. Lets the
404-
;; user declare a collection + name the template
405-
;; without also having to manually wire the
406-
;; 'Rendered from' source in the inspector.
407-
fallback (when (and (nil? (:source-sub child))
408-
(nil? (:source-field child)))
409-
(stateful-host-for-template
410-
doc all-groups gname))
411-
src-field (or (:source-field child)
412-
(when fallback (keyword (:field-name fallback))))
413-
field-ns (or (get field-owner-ns-map (:source-field child))
414-
(when fallback (:ns-name fallback)))]
415-
[(collection-iteration-form gname
416-
(:source-sub child)
417-
src-field
418-
field-ns)
393+
(let [src (em/resolve-template-source
394+
(assoc child :ns-name gname)
395+
doc all-groups field-owner-ns-map)
396+
;; Map the resolver's :kind back onto the three values
397+
;; `collection-iteration-form` already accepts. :source-sub
398+
;; carries the explicit sub keyword; :source-field carries
399+
;; an explicit field + owner; :auto-host is the implicit
400+
;; fallback where the field-name is a string we re-key.
401+
[source-sub source-field field-owner-ns]
402+
(case (and src (:kind src))
403+
:source-sub [(:sub src) nil nil]
404+
:source-field [nil (:field src) (:owner-ns src)]
405+
:auto-host [nil
406+
(keyword (:field-name src))
407+
(:owner-ns src)]
408+
[nil nil nil])]
409+
[(collection-iteration-form gname source-sub source-field field-owner-ns)
419410
(conj rendered-tpls gname)])
420411

421412
:else

src/bareforge/export/model.cljs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,39 @@
6464
ns-name)]
6565
[pick (conj seen pick)]))
6666

67+
;; --- action-ref canonicalisation -----------------------------------------
68+
69+
(defn action-ref-canonical-ns
70+
"Canonicalise an action-ref qualified keyword's namespace so the
71+
group segment matches the rest of the generator. An action-ref
72+
committed before the canonicalisation fix can carry the raw
73+
user-typed name (`:app.Dashboard.events/tick`); every other
74+
generator path (file paths, ns forms, db aliases) uses
75+
`name->ns-segment` to lowercase / dash-collapse it. Without this
76+
pass, dispatch fires on a key the registry doesn't know.
77+
78+
Single source of truth for both plugins (`cljs_project` and
79+
`vanilla_js`). Returns the full canonical namespace string —
80+
e.g. `\"app.dashboard.events\"`."
81+
[ref]
82+
(let [ns (namespace ref)
83+
first-dot (.indexOf ns ".")
84+
last-dot (.lastIndexOf ns ".")
85+
app-pref (subs ns 0 (inc first-dot))
86+
grp-seg (subs ns (inc first-dot) last-dot)
87+
suffix (subs ns last-dot)]
88+
(str app-pref (name->ns-segment grp-seg) suffix)))
89+
90+
(defn action-ref-alias
91+
"The require-alias string for an action-ref qualified keyword:
92+
`action-ref-canonical-ns` with the leading `app.` prefix stripped.
93+
Used by the CLJS plugin as the `[<canonical> :as <alias>]` form.
94+
95+
`:app.Dashboard.events/tick` → `\"dashboard.events\"`."
96+
[ref]
97+
(let [ns (action-ref-canonical-ns ref)]
98+
(subs ns (inc (.indexOf ns ".")))))
99+
67100
;; --- group detection -----------------------------------------------------
68101

69102
(defn- named-node?

src/bareforge/export/vanilla_js/codegen.cljs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -562,18 +562,9 @@
562562
[trigger {:keys [template-record-sym template-field-syms]
563563
:as ctx}]
564564
(let [aref (:action-ref trigger)
565-
;; Canonicalise the action-ref's group segment: an older doc
566-
;; can carry `:app.Dashboard.events/tick` (raw user-typed
567-
;; name) while the registry id was built from the lowercased
568-
;; `:ns-name` — without this, `dispatch` fires on a key the
569-
;; registry doesn't know.
570-
alias (let [ns (namespace aref)
571-
first-dot (.indexOf ns ".")
572-
last-dot (.lastIndexOf ns ".")
573-
app-pref (subs ns 0 (inc first-dot))
574-
grp (subs ns (inc first-dot) last-dot)
575-
suffix (subs ns last-dot)]
576-
(str app-pref (actions/name->ns-segment grp) suffix))
565+
;; Shared with the CLJS plugin via em/action-ref-canonical-ns,
566+
;; so the two emitters dispatch on identical alias strings.
567+
alias (em/action-ref-canonical-ns aref)
577568
ename (cljs.core/name aref)
578569
payload (:payload trigger)
579570
args (cond

0 commit comments

Comments
 (0)