From a7e782c3f24f617554a0cb34b78710ce5f4a8046 Mon Sep 17 00:00:00 2001 From: kevin Date: Fri, 31 Jul 2026 10:52:03 +0800 Subject: [PATCH 1/2] Harden inert snapshots and honor redirect URLs --- CHANGELOG.md | 6 + README.md | 2 +- clone/cloner.go | 59 +++++- clone/resolve_test.go | 45 +++++ docs/content/reference/release-notes.md | 5 + sanitize/sanitize.go | 241 +++++++++++++++++++++--- sanitize/sanitize_test.go | 92 +++++++-- 7 files changed, 403 insertions(+), 47 deletions(-) create mode 100644 clone/resolve_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 07f7992..2eee696 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ All notable changes to kage are recorded here. The format follows ## [Unreleased] +### Fixed + +- Saved pages no longer re-root relative URLs against the live origin via a leftover ``, and active content that escaped the script stripper is neutralized: `iframe` `srcdoc` and `data:text/html` sources, live remote frames, and HTML/SVG `object`/`embed` carriers. +- Relative links on pages that redirected are resolved against the browser's final URL (and any document ``), while the page is still written under the discovered URL so existing offline links keep working. +- Non-UTF-8 `` / Content-Type charset declarations are rewritten to `utf-8`, since kage always serialises pages as UTF-8 ([#16](https://github.com/tamnd/kage/issues/16)). + ## [0.3.9] - 2026-07-08 ### Fixed diff --git a/README.md b/README.md index 76195d2..fa478e9 100644 --- a/README.md +++ b/README.md @@ -259,7 +259,7 @@ cmd/kage/ thin main: pins the main thread, then hands off to cli.Execute cli/ the cobra command tree and flag wiring clone/ the crawl: frontier, render workers, asset workers, resume state browser/ headless Chrome control and DOM snapshotting -sanitize/ strip scripts, handlers, and javascript: URLs from the DOM +sanitize/ strip scripts, handlers, active frames, and javascript: URLs asset/ download and localise CSS, images, and fonts urlx/ the deterministic URL-to-path mapping zim/ a pure-Go ZIM reader and writer diff --git a/clone/cloner.go b/clone/cloner.go index 2940e77..4c507b6 100644 --- a/clone/cloner.go +++ b/clone/cloner.go @@ -19,6 +19,7 @@ import ( "github.com/tamnd/kage/sanitize" "github.com/tamnd/kage/urlx" "golang.org/x/net/html" + "golang.org/x/net/html/atom" "golang.org/x/time/rate" ) @@ -302,6 +303,13 @@ func (c *Cloner) processPage(ctx context.Context, j pageItem) { return } + // Resolve references against the post-redirect URL (and any ), + // but keep writing the page under the discovered URL so existing offline + // links that pointed at /old still resolve. Cross-host redirects leave the + // resolve base as the final location for relative refs; scope checks still + // use that absolute URL. + resolveBase := pageResolveBase(j.u, res.FinalURL, root) + localFile := urlx.LocalPath(c.seedHost, j.u, urlx.Page, c.cfg.Reserved) fileDir := urlx.Dir(localFile) @@ -324,7 +332,7 @@ func (c *Cloner) processPage(ctx context.Context, j pageItem) { } } - asset.RewriteHTML(root, j.u, sink) + asset.RewriteHTML(root, resolveBase, sink) sanitize.CleanTree(root, sanitize.Options{ KeepNoscript: c.cfg.KeepNoscript, MobileReadable: c.cfg.MobileReadable, @@ -354,6 +362,55 @@ func (c *Cloner) waitForCrawlDelay(ctx context.Context) bool { return c.crawlLimiter.Wait(ctx) == nil } +// pageResolveBase picks the URL against which relative references on a rendered +// page should resolve. Preference order: +// 1. A document (the live page's own base); +// 2. The browser's final URL after redirects; +// 3. The URL that was enqueued. +// +// The page is still written under the enqueued URL so offline links discovered +// as /old keep working when the server redirected /old → /new. +func pageResolveBase(enqueued *url.URL, finalURL string, root *html.Node) *url.URL { + base := enqueued + if finalURL != "" { + if u, err := url.Parse(finalURL); err == nil && u.Scheme != "" && u.Host != "" { + // Drop fragment; keep query/path as the browser shows them. + u.Fragment = "" + base = u + } + } + if href := documentBaseHref(root); href != "" { + if u, err := urlx.Normalize(base, href); err == nil { + return u + } + } + return base +} + +// documentBaseHref returns the first in document order, or "". +func documentBaseHref(root *html.Node) string { + var found string + var walk func(*html.Node) + walk = func(n *html.Node) { + if found != "" || n == nil { + return + } + if n.Type == html.ElementNode && n.DataAtom == atom.Base { + for _, a := range n.Attr { + if strings.EqualFold(a.Key, "href") && strings.TrimSpace(a.Val) != "" { + found = strings.TrimSpace(a.Val) + return + } + } + } + for c := n.FirstChild; c != nil && found == ""; c = c.NextSibling { + walk(c) + } + } + walk(root) + return found +} + // processAsset downloads one asset, rewriting CSS references on the way, and // writes it to its deterministic local path. func (c *Cloner) processAsset(ctx context.Context, j assetItem) { diff --git a/clone/resolve_test.go b/clone/resolve_test.go new file mode 100644 index 0000000..42d59dc --- /dev/null +++ b/clone/resolve_test.go @@ -0,0 +1,45 @@ +package clone + +import ( + "net/url" + "strings" + "testing" + + "golang.org/x/net/html" +) + +func TestPageResolveBaseUsesFinalURL(t *testing.T) { + enqueued, _ := url.Parse("https://ex.com/old") + root, err := html.Parse(strings.NewReader(`n`)) + if err != nil { + t.Fatal(err) + } + base := pageResolveBase(enqueued, "https://ex.com/new/", root) + if base.String() != "https://ex.com/new/" { + t.Fatalf("resolve base = %q, want final URL", base) + } +} + +func TestPageResolveBasePrefersDocumentBase(t *testing.T) { + enqueued, _ := url.Parse("https://ex.com/page") + root, err := html.Parse(strings.NewReader( + ``)) + if err != nil { + t.Fatal(err) + } + base := pageResolveBase(enqueued, "https://ex.com/page", root) + if base.String() != "https://ex.com/dir/" { + t.Fatalf("resolve base = %q, want document ", base) + } +} + +func TestDocumentBaseHref(t *testing.T) { + root, err := html.Parse(strings.NewReader( + ``)) + if err != nil { + t.Fatal(err) + } + if got := documentBaseHref(root); got != "/subdir/" { + t.Fatalf("documentBaseHref = %q, want first base", got) + } +} diff --git a/docs/content/reference/release-notes.md b/docs/content/reference/release-notes.md index a26b83a..62e01f1 100644 --- a/docs/content/reference/release-notes.md +++ b/docs/content/reference/release-notes.md @@ -6,6 +6,11 @@ weight: 40 The authoritative, commit-level history lives in [`CHANGELOG.md`](https://github.com/tamnd/kage/blob/main/CHANGELOG.md) and on the [releases page](https://github.com/tamnd/kage/releases). This page summarises each version. +## Unreleased + +- **Inert-snapshot gaps closed.** ``, `iframe` `srcdoc` / `data:text/html`, live remote frames, and HTML/SVG `object`/`embed` no longer reintroduce live or executable content. +- **Redirects resolve correctly.** Relative links use the post-redirect URL and any document ``, and non-UTF-8 charset metas are rewritten to UTF-8. + ## v0.3.9 A fix for the antivirus warning some Windows users hit when installing kage. diff --git a/sanitize/sanitize.go b/sanitize/sanitize.go index b01333f..257170e 100644 --- a/sanitize/sanitize.go +++ b/sanitize/sanitize.go @@ -2,10 +2,12 @@ // the saved page is inert: a photograph, not a program. // // It parses with golang.org/x/net/html, walks the tree, and deletes scripts, -// event handlers, javascript: URLs, downlevel IE conditional comments (which -// can smuggle a "> + + + + + +` + out, rep, err := Strip([]byte(in), Options{}) + if err != nil { + t.Fatal(err) + } + s := string(out) + if strings.Contains(s, " survived") + } + if strings.Contains(s, "srcdoc") { + t.Error("srcdoc survived") + } + if strings.Contains(strings.ToLower(s), "data:text/html") { + t.Error("data:text/html iframe survived") + } + if strings.Contains(s, "tracker.example") { + t.Error("external iframe src survived") + } + if strings.Contains(s, "widget.html") || strings.Contains(s, "player.html") { + t.Error("HTML object/embed survived") + } + // A non-script image object must survive. + if !strings.Contains(s, "logo.png") { + t.Error("image object should survive") + } + if rep.BaseTagsRemoved < 1 { + t.Errorf("BaseTagsRemoved = %d, want >= 1", rep.BaseTagsRemoved) + } + if rep.ActiveFramesRemoved < 1 { + t.Errorf("ActiveFramesRemoved = %d, want >= 1", rep.ActiveFramesRemoved) } } From d143c40581602caf1dbf33b4e4200cc452ff750c Mon Sep 17 00:00:00 2001 From: kevin Date: Fri, 31 Jul 2026 13:12:11 +0800 Subject: [PATCH 2/2] Neutralize localised .svg/.xml frame srcs and fix data: URL sniffing --- sanitize/sanitize.go | 52 ++++++++++++++++++++++++--------------- sanitize/sanitize_test.go | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 20 deletions(-) diff --git a/sanitize/sanitize.go b/sanitize/sanitize.go index 257170e..940346e 100644 --- a/sanitize/sanitize.go +++ b/sanitize/sanitize.go @@ -146,9 +146,7 @@ func clean(n *html.Node, opts Options, rep *Report) { rep.BaseTagsRemoved++ continue case atom.Iframe, atom.Frame: - if neutralizeActiveFrame(c, rep) { - // Element kept as an empty shell, or fully removed. - } + neutralizeActiveFrame(c, rep) case atom.Object, atom.Embed: // Same-domain HTML/SVG served through object/embed executes // scripts offline when localised as a raw asset. Drop active @@ -168,33 +166,30 @@ func clean(n *html.Node, opts Options, rep *Report) { // neutralizeActiveFrame strips iframe/frame carriers that would still run code // offline: srcdoc markup (not walked by the element sanitizer), data:text/html -// URLs, and remote http(s) sources left pointing at the live web. A same-site -// page/frame src is left for the asset rewriter to localise; after rewrite it -// is a static file. Returns true when the frame was fully neutralized. -func neutralizeActiveFrame(n *html.Node, rep *Report) bool { - // srcdoc holds raw HTML that never enters the element walk — drop it and - // clear the attribute so nothing executable remains. - if srcdoc := attr(n, "srcdoc"); srcdoc != "" { - setAttr(n, "srcdoc", "") +// URLs, remote http(s) sources left pointing at the live web, and localised +// raw assets that execute script inside a frame (.svg, .xml). The element +// itself stays behind as an empty, inert shell. +func neutralizeActiveFrame(n *html.Node, rep *Report) { + // srcdoc holds raw HTML that never enters the element walk — drop the + // attribute so nothing executable remains. + if attr(n, "srcdoc") != "" { removeAttr(n, "srcdoc") rep.ActiveFramesRemoved++ } src := strings.TrimSpace(attr(n, "src")) if src == "" { - return false + return } low := strings.ToLower(src) switch { case strings.HasPrefix(low, "javascript:"): removeAttr(n, "src") rep.JSURLsNeutralized++ - return true case strings.HasPrefix(low, "data:"): // data:text/html, data:image/svg+xml, etc. can carry scripts. if isActiveDataURL(low) { removeAttr(n, "src") rep.ActiveFramesRemoved++ - return true } case strings.HasPrefix(low, "http://"), strings.HasPrefix(low, "https://"), strings.HasPrefix(low, "//"): // An external live frame would phone home and run its own JS. Drop the @@ -204,9 +199,20 @@ func neutralizeActiveFrame(n *html.Node, rep *Report) bool { // is either external or was left remote intentionally. removeAttr(n, "src") rep.ActiveFramesRemoved++ - return true + default: + // A relative src is a localised file. Same-host pages (…/index.html) + // were rendered and sanitized, so they stay; but .svg/.xml took the raw + // asset path (never sanitized) and still run script inside a frame — + // drop those, mirroring the object/embed carrier policy. + path := low + if i := strings.IndexAny(path, "?#"); i >= 0 { + path = path[:i] + } + if strings.HasSuffix(path, ".svg") || strings.HasSuffix(path, ".xml") { + removeAttr(n, "src") + rep.ActiveFramesRemoved++ + } } - return false } // neutralizePlugin reports whether an object/embed should be removed entirely @@ -265,11 +271,17 @@ func isActiveDataURL(low string) bool { if !strings.HasPrefix(low, "data:") { return false } + // Only the mediatype token before the first comma decides; the payload + // (base64 or percent-encoded text) may contain these substrings by chance. + mime := low[len("data:"):] + if i := strings.IndexByte(mime, ','); i >= 0 { + mime = mime[:i] + } // data:text/html,... data:image/svg+xml,... data:application/xhtml+xml,... - return strings.Contains(low, "text/html") || - strings.Contains(low, "image/svg") || - strings.Contains(low, "xhtml") || - strings.Contains(low, "xml") + return strings.Contains(mime, "text/html") || + strings.Contains(mime, "image/svg") || + strings.Contains(mime, "xhtml") || + strings.Contains(mime, "xml") } func setAttr(n *html.Node, key, val string) { diff --git a/sanitize/sanitize_test.go b/sanitize/sanitize_test.go index a766821..70421a1 100644 --- a/sanitize/sanitize_test.go +++ b/sanitize/sanitize_test.go @@ -340,3 +340,49 @@ func TestActiveContentEscapesRemoved(t *testing.T) { t.Errorf("ActiveFramesRemoved = %d, want >= 1", rep.ActiveFramesRemoved) } } + +func TestLocalisedSVGFrameNeutralized(t *testing.T) { + // A same-host SVG takes the raw asset path — downloaded, never sanitized — + // yet a frame pointing at the localised copy would run its scripts offline. + in := `` + out, rep, err := Strip([]byte(in), Options{}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(out), "icon.svg") { + t.Errorf("localised .svg frame src survived:\n%s", out) + } + if rep.ActiveFramesRemoved < 1 { + t.Errorf("ActiveFramesRemoved = %d, want >= 1", rep.ActiveFramesRemoved) + } +} + +func TestInertDataImageFrameKept(t *testing.T) { + // Only the mediatype token decides: a base64 payload that happens to + // contain the text "xml" must not strip an inert data:image iframe. + in := `` + out, _, err := Strip([]byte(in), Options{}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), "data:image/png") { + t.Errorf("inert data:image/png frame lost its src:\n%s", out) + } +} + +func TestLocalisedInertFramesKept(t *testing.T) { + // Same-host frames localised by the rewrite pipeline stay: an image asset + // (inert in a frame) and a rendered, sanitized page. + in := `` + out, _, err := Strip([]byte(in), Options{}) + if err != nil { + t.Fatal(err) + } + s := string(out) + if !strings.Contains(s, "logo.png") { + t.Errorf("image frame should survive:\n%s", out) + } + if !strings.Contains(s, "../about/index.html") { + t.Errorf("sanitized page frame should survive:\n%s", out) + } +}