feat(gnoweb): change built-in playground and run views into features by jeronimoalbi · Pull Request #5774 · gnolang/gno

jeronimoalbi · 2026-06-04T08:02:59Z

No description provided.

Gno2D2 · 2026-06-04T08:03:35Z

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):

IGNORE the bot requirements for this PR (force green CI check)

✅ Automated Checks (for Contributors):

No automated checks match this pull request.

☑️ Contributor Actions:

Fix any issues flagged by automated checks.
Follow the Contributor Checklist to ensure your PR is ready for review.
- Add new tests, or document why they are unnecessary.
- Provide clear examples/screenshots, if necessary.
- Update documentation, if required.
- Ensure no breaking changes, or include BREAKING CHANGE notes.
- Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

Debug

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)
If
🟢 Condition met
└── 🟢 On every pull request
Can be checked by

Any user with comment edit permission

codecov · 2026-06-04T08:23:11Z

Codecov Report

❌ Patch coverage is 75.19084% with 65 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
gno.land/pkg/gnoweb/feature/playground/handler.go	81.81%	16 Missing and 6 partials ⚠️
...no.land/pkg/gnoweb/feature/playground/ratelimit.go	72.09%	8 Missing and 4 partials ⚠️
gno.land/pkg/gnoweb/feature/run/component.go	0.00%	10 Missing ⚠️
gno.land/pkg/gnoweb/feature/run/handler.go	0.00%	7 Missing ⚠️
gno.land/pkg/gnoweb/handler_http.go	82.75%	5 Missing ⚠️
gno.land/pkg/gnoweb/feature/run/template.go	42.85%	2 Missing and 2 partials ⚠️
gno.land/pkg/gnoweb/feature/playground/template.go	60.00%	1 Missing and 1 partial ⚠️
gno.land/pkg/gnoweb/feature/run/view.go	0.00%	2 Missing ⚠️
gno.land/pkg/gnoweb/components/template.go	50.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

jeronimoalbi · 2026-06-04T09:37:56Z

-	case gnourl.IsRealm(), gnourl.IsPure(), gnourl.IsUser(), gnourl.IsPackageFork():
-		return h.GetPackageView(ctx, gnourl, indexData)


The IsPackageFork() check is redundant, forking is done within realms or pure packages so switch already matches. The GetPackageView() method already checks if web query arguments indicate that it's a fork and if so it renders the playground in fork mode.

jeronimoalbi · 2026-06-04T12:37:20Z

-// IsPackageFork checks if the URL represents a package fork.
-func (gnoURL GnoURL) IsPackageFork() bool {
-	return gnoURL.WebQuery.Has("fork")
-}


Removed because is not being used right now

alexiscolin

Solid refactor. The per-feature layout is clean, decoupling each feature from the parent gnoweb via a locally-declared ClientAdapter is the right move for testability, and the Makefile tricks (vpath, dup-basename guard) are nice. Thanks also for the inline notes on IsPackageFork: that clears up my two switch / url.go questions before I had to ask them.

Two things I'd want pinned down before merge I suppose:

GetForkView no longer returns an error page when ListFiles fails: it falls back to the default playground while still stamping ForkFrom, so the page will read "forked from X" with the default snippet inside. That feels like an undocumented UX change; flagging it inline.
Codecov shows feature/run/{handler.go, component.go, view.go} at 0%. One small test would close all three.

The rest is mostly small stuff plus a handful of cheap security wins (decompression-bomb cap, MaxBytesReader on eval, X-Forwarded-For trust assumption, missing rate-limit on /funcs). None are blockers: just feels worth landing them while we're already in this code.

alexiscolin · 2026-06-05T05:37:26Z

 		Renderer: cfg.Renderer,
 		Aliases:  cfg.Aliases,
 		Logger:   logger,
+		Playground: playground.New(playground.Deps{


The featureClientAdapter / playground.ClientAdapter link is only validated at the playground.New(...) call site today, which is easy to miss when iterating on either interface. A static assertion next to the type would fail-fast on drift: var _ playground.ClientAdapter = (*featureClientAdapter)(nil) no?

Doc says "for custom features" (plural) but only playground actually uses it: run declares no client. Probably worth tightening to "for the playground feature, drops the FileMeta return" unless you plan to share it with future features.

Changed in ca89afc

Changed client adapter name to make sense for current code. The previous name was early optimization in case other feature like block explorer requires the same adapter.

alexiscolin · 2026-06-05T05:52:53Z

+// New validates required deps and returns a Handler.
+func New(deps Deps) *Handler {
+	if deps.Client == nil {
+		panic("playground.New: Client is required")


Validation is asymmetric here no? Client panics if nil, but empty Domain / Remote / ChainId slide through silently even though they all end up rendered into the page. Not necessarily wrong (maybe empty defaults are legitimate in dev?), just felt inconsistent enough to flag. Either validate them all or drop a one-liner saying empties are intentional.

Done in 38a9c78 and also for run feature in d99869d

alexiscolin · 2026-06-05T05:53:53Z

+	if initial != "" {
+		if decoded, err := base64.StdEncoding.DecodeString(initial); err == nil {
+			// Decompress code when given as DEFLATE compressed data format (RFC 1951)
+			if u.Query.Has("z") {


io.ReadAll(zr) on user-controlled compressed input is a textbook decompression bomb vector. A small base64 with a high compression ratio expands to arbitrary RAM. An io.LimitReader(zr, 1<<20) (or whatever the playground's reasonable code-size ceiling is) may shut that down?

zr.Close() isn't deferred. Currently safe because the only path through the if-block reaches it, but the next edit that adds an early return inside the if will silently leak. defer zr.Close() right after flate.NewReader(...) could be a free upgrade.

Fixed in 53f6185

alexiscolin · 2026-06-05T05:54:59Z

+func (h *Handler) GetForkView(ctx context.Context, u *weburl.GnoURL) (int, *components.View) {
+	pkgPath := u.Path
+	files, err := h.deps.Client.ListFiles(ctx, pkgPath)
+	if err != nil {


This changed behavior on me and I'm not sure it's intentional. Old code returned GetClientErrorStatusPage here. New code falls back to the default playground but still sets ForkFrom: path.Join(h.deps.Domain, pkgPath), so the page will display "forked from gno.land/r/foo" while actually showing the default Render snippet: which seems misleading.

If the friendlier fallback is intentional, two options that feel cleaner:

leave ForkFrom empty in this branch (silently degrade to a fresh playground), or

add a ForkFailed field so the template can show an honest "couldn't fetch source, starting fresh" hint.

Could also be I'm reading the template wrong, let me know.

It was provably done for simplicity in the PoC, but now it makes sense to return an error.

Done in 870cc7a

alexiscolin · 2026-06-05T06:03:48Z

+}
+
+func newRateLimiter(burstSize int, refillInterval time.Duration) *rateLimiter {
+	rl := &rateLimiter{


pruneLoop is started here with no way to stop it. Fine in prod (one Handler per process) but every test that calls New(...) leaks a goroutine, and there's no escape hatch. Same as the old code, flagging because the new feature layout could plausibly spawn multiple Handler instances down the line. A context.Context taken by New (or a Close() method) may fix it cleanly.

alexiscolin · 2026-06-05T06:04:54Z

+// tokens; one token is added every refillInterval.
+type rateLimiter struct {
+	mu             sync.Mutex
+	buckets        map[string]*rateBucket


The bucket map grows linearly with unique IPs and is only trimmed every minute. Under a flood from distinct (or spoofed: see the XFF comment) IPs it can balloon to IPsPerSecond × 60 entries before the first prune sweep. A hard cap that evicts oldest entries above some threshold (say 100k) would put a ceiling on the blast radius. Pre-existing, so not blocking, but maybe cheap to add?

alexiscolin · 2026-06-05T06:06:01Z

+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
+}


This is the security issue I'd most like resolved before merge: X-Forwarded-For is trusted unconditionally. If gnoweb is ever deployed without a reverse proxy in front (or with one that doesn't strip incoming XFF), an attacker can rotate the spoofed IP per request and bypass rate-limiting entirely.

Two ways out I guess:

document the deployment assumption (gnoweb requires a trusted XFF-terminating proxy), or

make XFF trust opt-in via config, default off -> use RemoteAddr.

The second is strictly safer. The first is fine if you're confident about the deployment story: worth at least a line in a doc?

alexiscolin · 2026-06-05T06:07:22Z

+func mustParse(name string, paths ...string) *template.Template {
+	t, err := template.New(name).ParseFS(templateFS, paths...)
+	if err != nil {
+		panic("playground: parse " + paths[0] + ": " + err.Error())
+	}
+	return t
+}


Two unrelated observations:

The name parameter looks dead to me: template.New(name) creates the container, but ExecuteTemplate in the component looks up "renderPage" defined by {{ define "renderPage" }} inside the file, not the container's name. If I'm right, you can drop the param?

This file and feature/run/template.go are 90% identical. At two features it's fine, but if feature/* becomes the pattern, hoisting this into a tiny shared helper (e.g. feature/internal/featuretmpl) would pay off?

Indeed, name is no needed within the current features context.

Regarding the pattern, we do have some differences, run feature uses other templates so the parsing there have extra steps. Most features might end up being the same though.

Removed in 9682008

alexiscolin · 2026-06-05T06:10:01Z

@@ -1,5 +1,5 @@
-import { CodeEditor, isDarkMode } from "./code-editor.js";
-import { BaseController } from "./controller.js";
+import { CodeEditor, isDarkMode } from "../../../frontend/js/code-editor.js";


../../../frontend/js/code-editor.js: three dot-dots. Brittle to any future move of feature/. A TS path alias ("@gnoweb/js/*" in tsconfig.json) should clean it up. I should do the same for the state explorer.

Done in 7466039

jefft0 · 2026-06-05T07:37:09Z

When the Actions page does a non-crossing query, it uses fetch from JavaScript:

gno/gno.land/pkg/gnoweb/frontend/js/controller-action-function.ts

Line 232 in fdce3c1

const response = await fetch(url);

This causes problems with security filters. It can be fixed by updating to also use the new api/eval endpoint. After merging this PR, we should do that fix.

jeronimoalbi · 2026-06-05T15:28:02Z

Addressing review comments

It limits the files fetched asynchronically, limits the total size of the compressed playground code and forked package size, to avoid memory exhaustion.

This alias replaces the "../../.." imports within the features. It also adds a minimal `tsconfig.json` to avoid issues with IDEs, otherwise imports will show an error.

jeronimoalbi · 2026-06-08T12:07:32Z

@alexiscolin I think we should address the rest of the comments in different issues to make it easier to review. Most of the issues are actually not really from this PR but from previous code, from pre-existing or "new" files splitted from these.

I'm thinking that this PR is deviating from just moving run and playground to features into a more complex one that solves other pre-existing issues 🤔

WDYT?

feat: refactor playground and run views into features

1c303a9

jeronimoalbi requested review from alexiscolin and gfanton June 4, 2026 08:02

jeronimoalbi self-assigned this Jun 4, 2026

github-project-automation Bot added this to 🧙‍♂️Gno.land development Jun 4, 2026

github-project-automation Bot moved this to Triage in 🧙‍♂️Gno.land development Jun 4, 2026

github-actions Bot added 📦 ⛰️ gno.land Issues or PRs gno.land package related 🌍 gnoweb Issues & PRs related to gnoweb and render labels Jun 4, 2026

jeronimoalbi added a/ux User experience, product, marketing community, developer experience team and removed 📦 ⛰️ gno.land Issues or PRs gno.land package related 🌍 gnoweb Issues & PRs related to gnoweb and render labels Jun 4, 2026

jeronimoalbi changed the title ~~feat(gnoweb): refactor built-in playground and run views into features~~ feat(gnoweb): change built-in playground and run views into features Jun 4, 2026

jeronimoalbi commented Jun 4, 2026

View reviewed changes

jeronimoalbi marked this pull request as ready for review June 4, 2026 12:37

jeronimoalbi added 2 commits June 4, 2026 14:38

feat: change run feature to use component's copy button template

75baeb3

chore: fix docs

9f30703

github-actions Bot added 📦 ⛰️ gno.land Issues or PRs gno.land package related 🌍 gnoweb Issues & PRs related to gnoweb and render labels Jun 4, 2026

jeronimoalbi mentioned this pull request Jun 4, 2026

feat(gnoweb): built-in playground(2) #5421

Draft

13 tasks

alexiscolin reviewed Jun 5, 2026

View reviewed changes

jeronimoalbi added 5 commits June 5, 2026 11:15

chore: rename playground's feature client adapter

ca89afc

chore: fix typos

223d59b

feat: check playground feat dependency values

38a9c78

feat: check run feat dependency values

d99869d

feat: fix decompression bomb attack vector in playground feature

53f6185

jeronimoalbi marked this pull request as draft June 5, 2026 15:27

jeronimoalbi added 5 commits June 5, 2026 17:28

ci: make egnerate

56dbb30

feat: change fork view to return error page on files read failure

870cc7a

feat: change fork view to fetch files async and limit size

167d64c

It limits the files fetched asynchronically, limits the total size of the compressed playground code and forked package size, to avoid memory exhaustion.

chore: remove template name when parsing

9682008

feat: add @gnoweb/js alias for TS feature imports

7466039

This alias replaces the "../../.." imports within the features. It also adds a minimal `tsconfig.json` to avoid issues with IDEs, otherwise imports will show an error.

Merge branch 'playground2' into feat/gnoweb-playground-feature

99dcec3

jeronimoalbi marked this pull request as ready for review June 9, 2026 09:58

		case gnourl.IsRealm(), gnourl.IsPure(), gnourl.IsUser(), gnourl.IsPackageFork():
		return h.GetPackageView(ctx, gnourl, indexData)

Conversation

jeronimoalbi commented Jun 4, 2026

Uh oh!

Gno2D2 commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛠 PR Checks Summary

Manual Checks (for Reviewers):

✅ Automated Checks (for Contributors):

☑️ Contributor Actions:

☑️ Reviewer Actions:

📚 Resources:

If

Can be checked by

Uh oh!

codecov Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jeronimoalbi Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexiscolin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jefft0 commented Jun 5, 2026

Uh oh!

jeronimoalbi commented Jun 5, 2026

Uh oh!

jeronimoalbi commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Gno2D2 commented Jun 4, 2026 •

edited

Loading

codecov Bot commented Jun 4, 2026 •

edited

Loading

jeronimoalbi Jun 4, 2026 •

edited

Loading