feat(gnoweb): built-in playground(2)#5421
Conversation
github-project-automation
Bot
added this to
💪 Bounties & Worx and 🧙♂️Gno.land development
github-actions
Bot
added
📦 ⛰️ gno.land
🛠 PR Checks Summary🔴 Changes related to gnoweb must be reviewed by its codeowners Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🔴 Changes related to gnoweb must be reviewed by its codeowners ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
alexiscolin
added
the
a/ux
moul
changed the title
- pre-allocate `funcs` slice in BuildEvalFuncs (prealloc lint) - fix biome formatting in controller-eval.ts and controller-playground.ts
…code - Add TestHTTPHandler_PlaygroundPage, TestHTTPHandler_EvalView, TestHTTPHandler_ForkView - Add TestHandlerPlaygroundEval and TestHandlerPlaygroundFuncs - Remove unused playgroundViewParams wrapper and ui/playground_editor template
Addresses milestone 0 (M0) from #5549 --------- Co-authored-by: Alexis Colin <alexis@jaunebleu.co>
This is an initial functional basic implementation for Playground scrollable tabs which should be improved in the future. It is required to be able to edit many files, for example when forking code.
Playground can share code though a URL. Code can be large, so before being shared it's compressed. Also adds a length limit check as a safeguard because code can get too big to be shared using a URL.
Addresses milestone 2 (M2) from #5549
Addresses milestone 1 (M1) from #5549 The "Edit" link opens the file being edited in the built-in playground within the package/realm fork mode.
…5638) Adds support for downloading multiple playground files as ".tar", or as a single ".gno" file when only one file is available. Downloading multiple files is handy in case users want to use `gnokey` to run or deploy the code written in the playground.
Done to bring output into view when the view is only the editor.
alexiscolin
left a comment
alexiscolin
left a comment
There was a problem hiding this comment.
Thanks for pushing this: the direction (Go-native, single binary, kill-WASM) is great @jeronimoalbi
I have not tested and checked the design and front-end right now in this first review but:
Main thing I'd like to discuss: I'd like to see the playground land as a self-contained module under pkg/gnoweb/feature/playground/, mirroring what the state-explorer refactor takes in #5649. Right now, gnoweb core is not light as it should be: so this would be a transitional shape, already cleaned and ready, toward a feature framework like featureapi/ leaf package + capability interfaces. I'll open an issue with the full design ASAP so we have a proper place to discuss it.
A few blockers to address before merge:
- Security hardening on the new public endpoints
- Goroutine leak + HTTP-200-on-error in
handler_playground.go - Front (few stuff I saw):
prompt()for filename, Tab keyboard trap in run view
And this question worth a spike: again, since we aim to keep gnoweb lightweight with minimal dependencies and JavaScript:
309 KB CM6 bundle vs ~12 KB CodeJar + a tokenizer emitting chroma classes: would reuse the existing
/_chroma/style.cssand unify the source view / editor theme.
| func clientIP(r *http.Request) string { | ||
| if xff := r.Header.Get("X-Forwarded-For"); xff != "" { | ||
| if ip, _, err := net.SplitHostPort(strings.TrimSpace(strings.SplitN(xff, ",", 2)[0])); err == nil { | ||
| return ip | ||
| } | ||
| return strings.TrimSpace(strings.SplitN(xff, ",", 2)[0]) | ||
| } | ||
| ip, _, err := net.SplitHostPort(r.RemoteAddr) | ||
| if err != nil { | ||
| return r.RemoteAddr | ||
| } | ||
| return ip | ||
| } |
There was a problem hiding this comment.
X-Forwarded-For is honored without any trusted-proxy gate. Any client can send X-Forwarded-For: 1.2.3.4 rotated per request to force each one into a fresh bucket: the 10/3s budget becomes infinite.
If gnoweb runs behind a known proxy (nginx, cloudflare (to come asap)), gate XFF on RemoteAddr being in a configured CIDR allowlist. If not, fall back to RemoteAddr only.
| func clientIP(r *http.Request) string { | |
| if xff := r.Header.Get("X-Forwarded-For"); xff != "" { | |
| if ip, _, err := net.SplitHostPort(strings.TrimSpace(strings.SplitN(xff, ",", 2)[0])); err == nil { | |
| return ip | |
| } | |
| return strings.TrimSpace(strings.SplitN(xff, ",", 2)[0]) | |
| } | |
| ip, _, err := net.SplitHostPort(r.RemoteAddr) | |
| if err != nil { | |
| return r.RemoteAddr | |
| } | |
| return ip | |
| } | |
| // clientIP returns the request's source IP. X-Forwarded-For is honored only | |
| // when RemoteAddr is inside one of the trusted CIDRs (empty default = trust nothing). | |
| func clientIP(r *http.Request, trustedProxies []*net.IPNet) string { | |
| host, _, err := net.SplitHostPort(r.RemoteAddr) | |
| if err != nil { | |
| host = r.RemoteAddr | |
| } | |
| if !isFromTrustedProxy(host, trustedProxies) { | |
| return host | |
| } | |
| if xff := r.Header.Get("X-Forwarded-For"); xff != "" { | |
| first := strings.TrimSpace(strings.SplitN(xff, ",", 2)[0]) | |
| if ip := net.ParseIP(first); ip != nil { | |
| return ip.String() | |
| } | |
| } | |
| return host | |
| } |
| decoded, err := base64.StdEncoding.DecodeString(initialCode) | ||
| if err == nil { | ||
| if gnourl.Query.Has("z") { | ||
| r := flate.NewReader(bytes.NewReader(decoded)) | ||
| if plain, err := io.ReadAll(r); err == nil { | ||
| initialCode = string(plain) | ||
| } | ||
|
|
||
| r.Close() | ||
| } else { | ||
| initialCode = string(decoded) | ||
| } | ||
| } |
There was a problem hiding this comment.
flate.NewReader + io.ReadAll with no size cap. A ~5 KB compressed payload of zeros decompresses to >1 GB -> OOM in a single GET. The URL is public, shareable, cacheable thus trivially weaponizable.
Also: the io.ReadAll error (L794) and r.Close() error (L798) are swallowed silently.
| decoded, err := base64.StdEncoding.DecodeString(initialCode) | |
| if err == nil { | |
| if gnourl.Query.Has("z") { | |
| r := flate.NewReader(bytes.NewReader(decoded)) | |
| if plain, err := io.ReadAll(r); err == nil { | |
| initialCode = string(plain) | |
| } | |
| r.Close() | |
| } else { | |
| initialCode = string(decoded) | |
| } | |
| } | |
| const maxPlaygroundCodeBytes = 256 << 10 // 256 KiB | |
| if gnourl.Query.Has("z") { | |
| r := flate.NewReader(bytes.NewReader(decoded)) | |
| defer r.Close() | |
| plain, err := io.ReadAll(io.LimitReader(r, maxPlaygroundCodeBytes+1)) | |
| if err != nil { | |
| h.Logger.Warn("playground deflate read failed", "error", err) | |
| } else if len(plain) > maxPlaygroundCodeBytes { | |
| h.Logger.Warn("playground code exceeds size limit", | |
| "size", len(plain), "limit", maxPlaygroundCodeBytes) | |
| } else { | |
| initialCode = string(plain) | |
| } | |
| } else { | |
| initialCode = string(decoded) | |
| } |
| } | ||
|
|
||
| var req evalRequest | ||
| if err := json.NewDecoder(r.Body).Decode(&req); err != nil { |
There was a problem hiding this comment.
json.NewDecoder(r.Body).Decode(&req) with no http.MaxBytesReader. Combined with no length cap on req.Expression (L174-185), an attacker can POST GBs of JSON or 10 MB-long expressions, each forwarded to the node via qeval. I got the same issue on the State Explorer Front End refactor I did...
| func handlerPlaygroundFuncs(logger *slog.Logger, cli ClientAdapter) http.Handler { | ||
| h := &playgroundAPIHandler{ | ||
| logger: logger, | ||
| client: cli, | ||
| } | ||
| return http.HandlerFunc(h.serveFuncs) | ||
| } |
There was a problem hiding this comment.
handlerPlaygroundFuncs builds playgroundAPIHandler with limiter: nil (L138 omits it). The handler calls Client.Doc(): a full node RPC. Spam this endpoint means saturate the node. Same amplification vector the eval limiter was meant to close, on a sibling endpoint the same JS controller calls on every package switch.
| } | ||
|
|
||
| // GetForkView loads all source files from a package and redirects to playground with the code. | ||
| func (h *HTTPHandler) GetForkView(ctx context.Context, gnourl *weburl.GnoURL) (int, *components.View) { |
There was a problem hiding this comment.
ListFiles + serial loop calling Client.File() per .gno file. No file count cap, no byte cap, no request timeout. A package with many files = many serial RPCs + multi-MB string concat in one request. Also slow (N × RTT) for legitimate packages with 5-10 files.
| h.logger.Debug("playground eval result", "took", took, "error", err) | ||
|
|
||
| if err != nil { | ||
| writeJSON(w, http.StatusOK, evalResponse{Error: err.Error()}) |
There was a problem hiding this comment.
Returning 200 with {"error": ...} for backend RPC failures breaks HTTP semantics, monitoring (you can't grep 5xx), and fetch().ok on the frontend.
|
|
||
| jdoc, err := h.client.Doc(r.Context(), pkgPath) | ||
| if err != nil { | ||
| writeJSON(w, http.StatusOK, map[string]string{"error": err.Error()}) |
There was a problem hiding this comment.
Returning 200 with {"error": ...} for backend RPC failures breaks HTTP semantics, monitoring (you can't grep 5xx), and fetch().ok on the frontend. Same as above
| assetsHandler := cacheAssetHandler(AssetHandler()) | ||
| mux.Handle(assetsBase, http.StripPrefix(assetsBase, assetsHandler)) | ||
|
|
||
| // Handle playground API endpoints |
There was a problem hiding this comment.
These two lines wire playground endpoints directly into the root mux. I'd like to suggest reshaping the playground as a self-contained module under pkg/gnoweb/feature/playground/, matching what the state-explorer
refactor takes in #5649. Same shape, applied to playground:
pkg/gnoweb/feature/playground/
├── feature.go // entry / Register(mux, deps)
├── handler.go // GetPlaygroundView, GetForkView, GetRunView (today in handler_http.go:779-868)
├── handler_api.go // serveEval, serveFuncs (today in handler_playground.go)
├── ratelimit.go // playgroundRateLimiter (today in handler_playground.go)
├── validate.go // pkg_path sanitization, base64+deflate decode helpers
├── client.go // local subset interface (Eval, Doc, ListFiles, File)
├── templates/playground.html, run.html
├── frontend/controller-playground.ts, controller-run.ts, controller-action-eval.ts
├── frontend/playground.css // own file, not 594 lines appended to 06-blocks.css (now 3163 lines)
└── *_test.go
This is a transitional shape (same as I did for #5649 for state-explorer this week). The final form is a feature framework: a featureapi/ leaf package with capability interfaces (QueryHandler for ?eval/?fork/?run, RouteProvider for //play + //api/*, LayoutContributor for the Eval/Fork header nav, Middlewarer for rate limiting). Playground would actually be the densest user of the framework: it implements all four. I'll open an issue with the full design ASAP so we have a proper place to discuss it but it covers anti-amplification, dispatch order, frontend build pipeline, and explains why this intermediate module shape is the right next step.
Why it matters specifically here, beyond cleanup IMO:
- Anti-amplification is baked into the framework: each interaction = one fetch = at most one upstream RPC. Today's
GetForkViewdoing N serial RPCs is exactly the kind of thing the framework rules out. Better to move toward that shape now than retrofit later. - Per-feature middleware is where rate limiting / body caps /
pkg_pathvalidation naturally live: instead of handler_playground.go reimplementing security primitives. - Eval method stays out of the root
ClientAdapter(today added at client.go): local subset interface avoids growing the root interface per feature. - Templates colocated with handler: easier to grep, to test and to scope, easier to check and delete the whole feature as a unit.
There was a problem hiding this comment.
Moved playground and run functionality into features in PR #5774 using patterns from #5649. This is a first step, before the final form you mention is defined.
Maybe other pending review changes could be addressed after merging it, because some file had to be splitted into new files. Addressing review changed could be simpler that way 🤔
…5674) The new built-in Playground changes include a run view where Gno code for run TXs can be written. This PR changes the textarea for a CodeMirror editor, the same used by the Playground, same configuration. The `gnokey` commands copy buttons were moved into "inline" copy icons to follow Gnoweb patterns. Related to #5421
6 participants
Summary
Exploration PR: add playground capabilities directly into gnoweb instead of a separate standalone app (replacing the need for gnostudio/studio).
Context: The current play.gno.land is a complex React+WASM app in a separate repo. This PR explores integrating interactive features directly into gnoweb as a Go-native, single-binary solution.
Related:
New features
/_/play— Scratch pad editor with multi-file tabs, keyboard shortcuts (Ctrl+Enter = run), share via URL?evalon any realm page — Expression evaluator with live qeval against the node, quick-call buttons for all read-only functions, history with re-run?forkon any package/realm — Loads all .gno source files into the playground editorPOST /_/api/eval— JSON API for expression evaluationGET /_/api/funcs?path=...— JSON API for listing package functionsArchitecture
vm/qevalRPC queriesgno.land/pkg/gnoweb/— same binary, same deployWhat's NOT done yet (follow-up work)
gno run/gno test/gno fmtDesign principles
gnowebembeds everythingTest plan
?evalon a live realm?forkloads source correctly/_/playscratch pad