fix(chromium)!: isolate each conversion in its own browser context

2026-10-14 00:03:18 +01:00 · 2026-10-13 18:31:38 +02:00
parent 887727b40e
commit 7b604da4f1
6 changed files with 148 additions and 26 deletions
--- a/1
+++ b/1
@@ -66,6 +66,7 @@ CHROMIUM_ALLOW_LIST=
 CHROMIUM_DENY_LIST=^file:(?!//\/tmp/).*
 CHROMIUM_DENY_PRIVATE_IPS=false
 CHROMIUM_DENY_PUBLIC_IPS=false
+CHROMIUM_SHARED_BROWSER_CONTEXT=false
 CHROMIUM_CLEAR_CACHE=false
 CHROMIUM_CLEAR_COOKIES=false
 CHROMIUM_DISABLE_JAVASCRIPT=false
--- a/compose.yaml
+++ b/compose.yaml
@@ -60,6 +60,7 @@ services:
      - "--chromium-deny-list=${CHROMIUM_DENY_LIST}"
      - "--chromium-deny-private-ips=${CHROMIUM_DENY_PRIVATE_IPS}"
      - "--chromium-deny-public-ips=${CHROMIUM_DENY_PUBLIC_IPS}"
+      - "--chromium-shared-browser-context=${CHROMIUM_SHARED_BROWSER_CONTEXT}"
      - "--chromium-clear-cache=${CHROMIUM_CLEAR_CACHE}"
      - "--chromium-clear-cookies=${CHROMIUM_CLEAR_COOKIES}"
      - "--chromium-disable-javascript=${CHROMIUM_DISABLE_JAVASCRIPT}"
--- a/pkg/modules/chromium/browser.go
+++ b/pkg/modules/chromium/browser.go
@@ -12,10 +12,12 @@ import (
 	"time"

 	cdprotobrowser "github.com/chromedp/cdproto/browser"
+	"github.com/chromedp/cdproto/cdp"
 	"github.com/chromedp/cdproto/fetch"
 	"github.com/chromedp/cdproto/network"
 	"github.com/chromedp/cdproto/page"
 	"github.com/chromedp/cdproto/runtime"
+	"github.com/chromedp/cdproto/target"
 	"github.com/chromedp/chromedp"
 	"github.com/dlclark/regexp2/v2"
 	"github.com/shirou/gopsutil/v4/process"
@@ -62,14 +64,15 @@ type browserArguments struct {
 	hyphenDataDirPath        string

 	// Tasks specific.
-	allowList         []*regexp2.Regexp
-	denyList          []*regexp2.Regexp
-	denyPrivateIPs    bool
-	denyPublicIPs     bool
-	clearCache        bool
-	clearCookies      bool
-	clearStorage      bool
-	disableJavaScript bool
+	allowList            []*regexp2.Regexp
+	denyList             []*regexp2.Regexp
+	denyPrivateIPs       bool
+	denyPublicIPs        bool
+	sharedBrowserContext bool
+	clearCache           bool
+	clearCookies         bool
+	clearStorage         bool
+	disableJavaScript    bool
 }

 type chromiumBrowser struct {
@@ -435,6 +438,55 @@ func (b *chromiumBrowser) screenshot(ctx context.Context, logger *slog.Logger, u
 	})
 }

+// newIsolatedTarget creates a browser context and opens a blank tab in it, so
+// cookies, cache, and storage never leak to another conversion, even a
+// concurrent one. The caller must dispose the browser context.
+//
+// chromedp.WithNewBrowserContext does not work here: it opens the tab without
+// a new window, which headless Chromium rejects with "no browser is open", and
+// it leaks the browser context on that failure.
+func (b *chromiumBrowser) newIsolatedTarget(ctx context.Context, logger *slog.Logger) (cdp.BrowserContextID, target.ID, error) {
+	c := chromedp.FromContext(b.ctx)
+	if c == nil || c.Browser == nil {
+		return "", "", errors.New("browser not allocated")
+	}
+	executor := cdp.WithExecutor(ctx, c.Browser)
+
+	browserContextID, err := target.CreateBrowserContext().WithDisposeOnDetach(true).Do(executor)
+	if err != nil {
+		return "", "", fmt.Errorf("create browser context: %w", err)
+	}
+
+	targetID, err := target.CreateTarget("about:blank").
+		WithBrowserContextID(browserContextID).
+		WithNewWindow(true).
+		Do(executor)
+	if err != nil {
+		b.disposeBrowserContext(logger, browserContextID)
+		return "", "", fmt.Errorf("create target: %w", err)
+	}
+
+	return browserContextID, targetID, nil
+}
+
+// disposeBrowserContext disposes a browser context created by
+// [chromiumBrowser.newIsolatedTarget]. It uses a fresh context, as the
+// conversion's one may already be done.
+func (b *chromiumBrowser) disposeBrowserContext(logger *slog.Logger, browserContextID cdp.BrowserContextID) {
+	c := chromedp.FromContext(b.ctx)
+	if c == nil || c.Browser == nil {
+		return
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	err := target.DisposeBrowserContext(browserContextID).Do(cdp.WithExecutor(ctx, c.Browser))
+	if err != nil {
+		logger.ErrorContext(ctx, fmt.Sprintf("dispose browser context: %s", err))
+	}
+}
+
 func (b *chromiumBrowser) do(ctx context.Context, logger *slog.Logger, url string, options Options, aggregate *networkAggregate, tasks chromedp.Tasks) error {
 	if !b.isStarted.Load() {
 		return errors.New("browser not started, cannot handle tasks")
@@ -461,7 +513,18 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *slog.Logger, url strin
 	timeoutCtx, timeoutCancel := context.WithTimeout(b.ctx, time.Until(deadline))
 	defer timeoutCancel()

-	taskCtx, taskCancel := chromedp.NewContext(timeoutCtx)
+	var taskOpts []chromedp.ContextOption
+	if !b.arguments.sharedBrowserContext {
+		browserContextID, targetID, err := b.newIsolatedTarget(timeoutCtx, logger)
+		if err != nil {
+			return fmt.Errorf("create isolated browser context: %w", err)
+		}
+		// Deferred before taskCancel, so it runs after the tab is closed.
+		defer b.disposeBrowserContext(logger, browserContextID)
+		taskOpts = append(taskOpts, chromedp.WithTargetID(targetID))
+	}
+
+	taskCtx, taskCancel := chromedp.NewContext(timeoutCtx, taskOpts...)
 	defer taskCancel()

 	// Accumulate per-conversion network activity for telemetry.
--- a/pkg/modules/chromium/chromium.go
+++ b/pkg/modules/chromium/chromium.go
@@ -479,9 +479,10 @@ func (mod *Chromium) Descriptor() gotenberg.ModuleDescriptor {
 			fs.StringSlice("chromium-deny-list", []string{`^file:(?!//\/tmp/).*`}, "Set the denied URLs for Chromium using regular expressions - supports multiple values")
 			fs.Bool("chromium-deny-private-ips", false, "Reject URLs whose host resolves to a non-public IP address (loopback, RFC1918, link-local, unique-local). Enable on deployments that accept untrusted form input to mitigate SSRF against internal services")
 			fs.Bool("chromium-deny-public-ips", false, "Reject URLs whose host resolves to a public IP address. Enable on air-gapped or data-governed deployments to prevent outbound traffic from leaving a private network")
-			fs.Bool("chromium-clear-cache", false, "Clear Chromium cache between each conversion")
-			fs.Bool("chromium-clear-cookies", false, "Clear Chromium cookies between each conversion")
-			fs.Bool("chromium-clear-storage", false, "Clear Chromium local storage between each conversion (session storage is already isolated per conversion)")
+			fs.Bool("chromium-shared-browser-context", false, "Run every conversion in one shared browser context, so cookies, cache, and storage carry over between conversions. Only enable it when all callers trust each other: a conversion can then reuse cookies another one set")
+			fs.Bool("chromium-clear-cache", false, "Clear Chromium cache between each conversion - only applies with --chromium-shared-browser-context")
+			fs.Bool("chromium-clear-cookies", false, "Clear Chromium cookies between each conversion - only applies with --chromium-shared-browser-context")
+			fs.Bool("chromium-clear-storage", false, "Clear Chromium local storage between each conversion - only applies with --chromium-shared-browser-context")
 			fs.Bool("chromium-disable-javascript", false, "Disable JavaScript")
 			fs.Bool("chromium-disable-routes", false, "Disable the routes")

@@ -515,6 +516,8 @@ func (mod *Chromium) Provision(ctx *gotenberg.Context) error {
 		return errors.New("CHROMIUM_HYPHEN_DATA_DIR_PATH environment variable is not set; set it to the absolute path of the Chromium hyphenation data directory (it ships in the Gotenberg image)")
 	}

+	sharedBrowserContext := flags.MustBool("chromium-shared-browser-context")
+
 	mod.args = browserArguments{
 		binPath:                  binPath,
 		allowInsecureLocalhost:   flags.MustBool("chromium-allow-insecure-localhost"),
@@ -527,13 +530,16 @@ func (mod *Chromium) Provision(ctx *gotenberg.Context) error {
 		wsUrlReadTimeout:         flags.MustDuration("chromium-start-timeout"),
 		hyphenDataDirPath:        hyphenDataDirPath,

-		allowList:         flags.MustRegexpSlice("chromium-allow-list"),
-		denyList:          flags.MustRegexpSlice("chromium-deny-list"),
-		denyPrivateIPs:    flags.MustBool("chromium-deny-private-ips"),
-		denyPublicIPs:     flags.MustBool("chromium-deny-public-ips"),
-		clearCache:        flags.MustBool("chromium-clear-cache"),
-		clearCookies:      flags.MustBool("chromium-clear-cookies"),
-		clearStorage:      flags.MustBool("chromium-clear-storage"),
+		allowList:            flags.MustRegexpSlice("chromium-allow-list"),
+		denyList:             flags.MustRegexpSlice("chromium-deny-list"),
+		denyPrivateIPs:       flags.MustBool("chromium-deny-private-ips"),
+		denyPublicIPs:        flags.MustBool("chromium-deny-public-ips"),
+		sharedBrowserContext: sharedBrowserContext,
+		// An isolated conversion starts from an empty browser context that
+		// is disposed afterward, so there is nothing to clear.
+		clearCache:        sharedBrowserContext && flags.MustBool("chromium-clear-cache"),
+		clearCookies:      sharedBrowserContext && flags.MustBool("chromium-clear-cookies"),
+		clearStorage:      sharedBrowserContext && flags.MustBool("chromium-clear-storage"),
 		disableJavaScript: flags.MustBool("chromium-disable-javascript"),
 	}

--- a/test/integration/features/chromium_convert_url.feature
+++ b/test/integration/features/chromium_convert_url.feature
@@ -22,10 +22,9 @@ Feature: /forms/chromium/convert/url
      Page 1
      """

-  # localStorage is per-origin and shared by the long-lived browser, so without
-  # clearing it accumulates across same-origin conversions.
-  # See https://github.com/gotenberg/gotenberg/issues/919.
-  Scenario: POST /forms/chromium/convert/url (localStorage leaks without clearing)
+  # Each conversion runs in its own browser context, so localStorage never
+  # carries over.
+  Scenario: POST /forms/chromium/convert/url (localStorage isolated per conversion)
    Given I have a default Gotenberg container
    Given I have a static server
    When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s):
@@ -37,14 +36,34 @@ Feature: /forms/chromium/convert/url
      | url                       | http://host.docker.internal:%d/html/testdata/local-storage-html/index.html | field  |
      | Gotenberg-Output-Filename | foo                                                                        | header |
    Then the response status code should be 200
+    Then the "foo.pdf" PDF should have content matching "localStorageCount=1" at page 1
+
+  # With --chromium-shared-browser-context, localStorage is per-origin and
+  # shared by the long-lived browser, so without clearing it accumulates
+  # across same-origin conversions.
+  # See https://github.com/gotenberg/gotenberg/issues/919.
+  Scenario: POST /forms/chromium/convert/url (localStorage leaks in a shared browser context)
+    Given I have a Gotenberg container with the following environment variable(s):
+      | CHROMIUM_SHARED_BROWSER_CONTEXT | true |
+    Given I have a static server
+    When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s):
+      | url                       | http://host.docker.internal:%d/html/testdata/local-storage-html/index.html | field  |
+      | Gotenberg-Output-Filename | foo                                                                        | header |
+    Then the response status code should be 200
+    Then the "foo.pdf" PDF should have content matching "localStorageCount=1" at page 1
+    When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s):
+      | url                       | http://host.docker.internal:%d/html/testdata/local-storage-html/index.html | field  |
+      | Gotenberg-Output-Filename | foo                                                                        | header |
+    Then the response status code should be 200
    Then the "foo.pdf" PDF should have content matching "localStorageCount=2" at page 1

-  # With --chromium-clear-storage every conversion clears the origin's
-  # localStorage first, so the counter never carries over.
+  # With --chromium-clear-storage every conversion in a shared browser context
+  # clears the origin's localStorage first, so the counter never carries over.
  # See https://github.com/gotenberg/gotenberg/issues/919.
  Scenario: POST /forms/chromium/convert/url (Clear Storage)
    Given I have a Gotenberg container with the following environment variable(s):
-      | CHROMIUM_CLEAR_STORAGE | true |
+      | CHROMIUM_SHARED_BROWSER_CONTEXT | true |
+      | CHROMIUM_CLEAR_STORAGE          | true |
    Given I have a static server
    When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s):
      | url                       | http://host.docker.internal:%d/html/testdata/local-storage-html/index.html | field  |
@@ -210,6 +229,36 @@ Feature: /forms/chromium/convert/url
    Then the server request cookie "cookie_1" should be "foo"
    Then the server request cookie "cookie_2" should be ""

+  # A cookie set by one conversion must not reach the next one.
+  Scenario: POST /forms/chromium/convert/url (Cookies isolated per conversion)
+    Given I have a default Gotenberg container
+    Given I have a static server
+    When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s):
+      | url     | http://host.docker.internal:%d/html/testdata/page-1-html/index.html    | field |
+      | cookies | [{"name":"cookie_1","value":"foo","domain":"host.docker.internal:%d"}] | field |
+    Then the response status code should be 200
+    Then the server request cookie "cookie_1" should be "foo"
+    When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s):
+      | url | http://host.docker.internal:%d/html/testdata/page-1-html/index.html | field |
+    Then the response status code should be 200
+    Then the server request cookie "cookie_1" should be ""
+
+  # With --chromium-shared-browser-context, cookies carry over between
+  # conversions.
+  Scenario: POST /forms/chromium/convert/url (Cookies in a shared browser context)
+    Given I have a Gotenberg container with the following environment variable(s):
+      | CHROMIUM_SHARED_BROWSER_CONTEXT | true |
+    Given I have a static server
+    When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s):
+      | url     | http://host.docker.internal:%d/html/testdata/page-1-html/index.html    | field |
+      | cookies | [{"name":"cookie_1","value":"foo","domain":"host.docker.internal:%d"}] | field |
+    Then the response status code should be 200
+    Then the server request cookie "cookie_1" should be "foo"
+    When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s):
+      | url | http://host.docker.internal:%d/html/testdata/page-1-html/index.html | field |
+    Then the response status code should be 200
+    Then the server request cookie "cookie_1" should be "foo"
+
  # See https://github.com/gotenberg/gotenberg/issues/1130.
  Scenario: POST /forms/chromium/convert/url (case-insensitive sameSite)
    Given I have a default Gotenberg container
--- a/test/integration/features/debug.feature
+++ b/test/integration/features/debug.feature
@@ -98,6 +98,7 @@ Feature: /debug
          "chromium-max-queue-size": "0",
          "chromium-proxy-server": "",
          "chromium-restart-after": "100",
+          "chromium-shared-browser-context": "false",
          "chromium-start-timeout": "20s",
          "gotenberg-build-debug-data": "true",
          "gotenberg-graceful-shutdown-duration": "30s",
@@ -237,6 +238,7 @@ Feature: /debug
          "chromium-max-concurrency": "6",
          "chromium-proxy-server": "",
          "chromium-restart-after": "100",
+          "chromium-shared-browser-context": "false",
          "chromium-start-timeout": "20s",
          "gotenberg-build-debug-data": "true",
          "gotenberg-graceful-shutdown-duration": "30s",