Skip to content

Bug 2088290 - Read the remote URL cache through diskcache-community's SafeJSONDisk - #889

Closed
avgvi wants to merge 1 commit into
mozilla:mainfrom
avgvi:fix/diskcache-safejsondisk
Closed

avgvi wants to merge 1 commit into
mozilla:mainfrom
avgvi:fix/diskcache-safejsondisk

Conversation

@avgvi

@avgvi avgvi commented Nov 3, 2026

Copy link
Copy Markdown

glean_parser depends on diskcache, whose last release, 5.6.3, is from August 2023. CVE-2025-69872 (GHSA-w8v5-vhqr-4h9v) is open against every version with no fix. Scanners report it for every glean_parser install, and upgrading isn't an option.

This moves the only user, fetch_remote_url(), to diskcache-community, a maintained, faster fork that keeps diskcache's API and file format. It's a pure-Python wheel with no dependencies, so nothing new to vendor beyond the package itself.

Change

  • pyproject.toml depends on diskcache-community>=5.8.2 instead of diskcache>=4. In uv.lock, only the diskcache entries change.
  • fetch_remote_url() opens the cache with SafeJSONDisk and on_error="miss". JSONDisk only writes JSON, but it reads through diskcache's default code, so a row rewritten as a pickle gets unpickled. SafeJSONDisk reads the same files and refuses pickles, so caches from earlier releases still work.
  • A refused entry counts as a miss, and the file is fetched again. The lookup now uses dc.get(key), because key in dc is true even for a row that can't be read.
  • Two tests in tests/test_utils.py, and a changelog entry.

Speed is about the same. On an Apple M5 Pro with Python 3.12, a 20 KB cached entry reads in 28.9 µs (29.1 µs on diskcache 5.6.3) and writes in 48.6 µs (53.6 µs). The fork's benchmarks page has more.

Testing

  • make lint passes.
  • make test: 313 passed, 25 skipped. main has 311 passed and the same 6 jsonschema RefResolver warnings.
  • The new tests fail with JSONDisk, and with the old key in dc lookup.
  • diskcache is no longer installed.
  • The script below uses a local HTTP server in place of the remote file, and the planted entry only prints a line.

On glean_parser 22.0.1 with diskcache 5.6.3:

1. first fetch
   '$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0\n' requests: 1
2. second fetch, served from the cache
   '$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0\n' requests: 1
3. tampered entry
  code from the cache directory ran in this process
   raised TypeError

With this change:

treating an unreadable entry in '…/glean_parser' as missing: UnpicklingError: SafeJSONDisk refuses a pickled value
1. first fetch
   '$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0\n' requests: 1
2. second fetch, served from the cache
   '$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0\n' requests: 1
3. tampered entry
   '$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0\n' requests: 2
Script
import http.server
import threading

import platformdirs

try:
    import diskcache_community as writer_package
except ModuleNotFoundError:
    import diskcache as writer_package  # glean_parser 22.0.1

from glean_parser import util


def planted():
    print("  code from the cache directory ran in this process")
    return "planted"


class Planted:
    def __reduce__(self):
        return (planted, ())


class Handler(http.server.SimpleHTTPRequestHandler):
    requests = 0

    def do_GET(self):
        Handler.requests += 1
        body = b"$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0\n"
        self.send_response(200)
        self.send_header("Content-Length", str(len(body)))
        self.end_headers()
        self.wfile.write(body)

    def log_message(self, *args):
        pass


server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), Handler)
threading.Thread(target=server.serve_forever, daemon=True).start()
url = "http://127.0.0.1:%d/metrics.yaml" % server.server_port

print("1. first fetch")
print("  ", repr(util.fetch_remote_url(url)), "requests:", Handler.requests)
print("2. second fetch, served from the cache")
print("  ", repr(util.fetch_remote_url(url)), "requests:", Handler.requests)

print("3. tampered entry")
cache_dir = platformdirs.user_cache_dir("glean_parser", "mozilla")
with writer_package.Cache(cache_dir) as other_writer:
    (key,) = list(other_writer)
    other_writer.set(key, Planted())
try:
    print("  ", repr(util.fetch_remote_url(url)), "requests:", Handler.requests)
except Exception as error:
    print("   raised", type(error).__name__)
server.shutdown()

The fork's docs cover moving from diskcache and what it changes for this CVE.

Pull Request checklist

  • Quality: This PR builds and tests run cleanly
    • make test runs without emitting any warnings (the 6 warnings above are the same on main)
    • make lint runs without emitting any errors
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to language binding APIs are noted explicitly

@avgvi
avgvi requested a review from a team as a code owner November 3, 2026 00:04
@avgvi
avgvi requested review from chutten and removed request for a team November 3, 2026 00:04
@badboy

badboy commented Nov 3, 2026

Copy link
Copy Markdown
Member

As mentioned in the bug, this is not what we're going with.

@badboy badboy closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants