Skip to content

fix(plugins): storage paths can't reach Object.prototype, increment respects the key cap, proxy redirects stay on the whitelist - #319

Merged
jherforth merged 2 commits into
mainfrom
fix/plugin-storage-and-proxy
Oct 23, 2026
Merged

jherforth merged 2 commits into
mainfrom
fix/plugin-storage-and-proxy

Conversation

@jherforth

Copy link
Copy Markdown
Owner

Fixes four plugin-reachable problems from Mrramam's audit: mrramam#54, #60 and two parts of #56. These are the ones that don't depend on the plugin-isolation work (#50, #49), which is coming as its own change.

Fixes

  • Storage increment reached Object.prototype (feat: multi-tenancy #54). The increment walked the caller's dot-path with no filtering. A __proto__.x path, from POST …/increment or a manifest reaction, therefore wrote onto Object.prototype for the whole server process.
    • Paths containing __proto__, constructor or prototype are now refused with 400. The route, manifest validation and the increment function itself all check this.
    • The increment now steps through own properties only.
  • Increment skipped the 500-key cap (feat: Day/Night mode #60). PUT counted keys, increment didn't. A new key now counts against the cap inside the same transaction, and over the cap it answers 413.
  • Proxy redirects (1.3 release #56). /api/proxy checked only the first address, then followed up to five redirects anywhere. A whitelisted host could therefore send it to an address on the household's network.
    • Every hop is now checked against the whitelist in beforeRedirect. A redirect off the list answers 403, and the off-list host is never contacted.
    • Only http(s) addresses are fetched.
  • GitHub install (1.3 release #56). It fetched any download_url and echoed the upstream body on errors.
    • It now accepts only files in jherforth/HomeGlowPlugins on raw.githubusercontent.com. These are the only addresses the listing gives.
    • It follows no redirects and no longer echoes the response body.

Not in this PR

Plugin isolation, and the open CORS behind it: #50, #49 and the CORS part of #62. While plugins run same-origin and the API allows any origin, a plugin can still call every route directly. That fix changes how all plugins run, so it comes separately.

Testing

  • Server npm test: 609 pass. New tests cover each of these cases:
    • __proto__ and constructor.prototype paths are refused, and nothing is stored;
    • a reaction with an unsafe path is refused at upload;
    • an increment past the cap answers 413, while an existing key still works;
    • a redirect off the whitelist is refused, and a second server records that it was never reached;
    • a redirect between whitelisted addresses still works;
    • an ftp: address is refused;
    • four bad GitHub install addresses are refused.
  • Against the old server code, the five tests for these bugs fail, so they catch them. The allowed-redirect test passes either way, as it should.

🤖 Generated with Claude Code

…espects the key cap, proxy redirects stay on the whitelist

Reported in mrramam#54, #60 and #56.

- Storage increment walked a caller's dot-path with no filtering, so
  `__proto__.x` (from the route or a manifest reaction) wrote onto
  Object.prototype in the server process. Paths with __proto__,
  constructor or prototype segments are refused (400) by the route, by
  manifest validation and by the increment itself, which now steps only
  through own properties.
- Increment created keys without counting them, skipping the 500-key cap
  PUT enforces. A new key now counts, inside the same transaction (413).
- /api/proxy checked only the first address, then followed up to five
  redirects anywhere, so a whitelisted host could send it to an address
  on the household's network. Every hop is now checked against the
  whitelist (403 when one leads off it), and only http(s) is fetched.
- GitHub install fetched any download_url and echoed the upstream body on
  errors. It now takes only files in jherforth/HomeGlowPlugins on
  raw.githubusercontent.com (the addresses the listing gives), follows no
  redirects, and no longer echoes what came back.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@mrramam

mrramam commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

One gap in the GitHub install check: startsWith runs on the raw string, but the URL is normalized when it's fetched. So https://raw.githubusercontent.com/jherforth/HomeGlowPlugins/%2e%2e/%2e%2e/<anyone>/<repo>/main/x.html passes the check and resolves to another repo's file. It stays on raw.githubusercontent.com, so this isn't a route into the household's network. Since /api/widgets/upload is open anyway, it's a small gap, but the check promises "only files in jherforth/HomeGlowPlugins". Parsing first closes it: const u = new URL(download_url), then check u.origin and u.pathname.startsWith('/jherforth/HomeGlowPlugins/').

@jherforth jherforth self-assigned this Oct 23, 2026
@jherforth jherforth added the bug Something isn't working label Oct 23, 2026
@jherforth jherforth added this to the 1.10 milestone Oct 23, 2026
Review of #319 (Mrramam): the check ran startsWith on the raw string, but
the URL is normalized when it's fetched, so
".../HomeGlowPlugins/%2e%2e/%2e%2e/<someone>/<repo>/main/x.html" passed
and loaded another repository's file.

The address is now parsed first: https on raw.githubusercontent.com, no
user name or password, and a normalized path inside
/jherforth/HomeGlowPlugins/. Encoded dots, slashes and backslashes are
refused, so nothing can decode into a different path later. The normalized
address is the one fetched and recorded. Real listing addresses (nested
folders, %20 in a name) still pass.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@jherforth

Copy link
Copy Markdown
Owner Author

The GitHub install check could be bypassed with an encoded .. in the address, loading another repo's file. It stayed on GitHub, so it was no route into the home network.
Fixed: the address is now parsed first, and must be a file inside jherforth/HomeGlowPlugins once normalized.
Checked: his address and several variants are refused. Real store addresses, including #53's nested folders and a name with spaces, still install. CI passes on the new commit.

@jherforth
jherforth merged commit 69c4cff into main Oct 23, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in HomeGlow Kanban Oct 23, 2026
@jherforth jherforth mentioned this pull request Oct 23, 2026
jherforth added a commit that referenced this pull request Oct 23, 2026
Adds the 1.10 changelog covering everything merged since v1.9.1 (#211 to
#319): themes as installable packages and weather scenes, Home Assistant
panels and music, Routines in the core, expiring and follow-up chores,
phone notifications (browser, ntfy, the Home Assistant app), backup and
restore, wall-display touches, the plugin store's versions and categories,
and a round of security fixes. Credits to @mrramam, @adamecker and
@jasonstantler, and upgrade notes: back up first, reinstall Reef or
Starship from the theme store, the Routines plugin moves into the core by
itself, and the plugins repository's folder layout follows this release.

Versions bumped to 1.10.0 in both package.json files and their lockfiles.
The About tab reads its version from the git tag at build time, so the
v1.10 tag is what users will see; these keep local runs honest.

Co-authored-by: Claude Opus 5.5 <[email protected]>
adamecker pushed a commit to adamecker/HomeGlow that referenced this pull request Oct 24, 2026
…e 1.10 jherforth#321, plugin storage/proxy fixes jherforth#319, chore timezone display jherforth#318, CI pins jherforth#316-317, prize descriptions jherforth#315, proxmox/docker fixes jherforth#314, plugin proxy docs jherforth#313, Google OAuth escaping jherforth#312, upload typing/sandbox jherforth#311

Resolve registry conflict: keep fork renumbering (local 28 userGoogleTasks, 34 googleTasksTables); upstream's new 35 proxyWhitelistSeed and 36 prizeDescription renumbered to 41 and 42 to keep schemaIds unique and matching file names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants