harden: adversarial-review findings on #136 + audit archive + honeypot integration test - #137
Merged
Merged
Conversation
mdheller
added a commit
that referenced
this pull request
Jul 30, 2026
…pstream product names Regression from PR #104 (my packaging cleanup a few days ago): I wrote 'LibreWolf-mirror fork of Firefox 150' in the deb description. verify-linux- packaging.sh forbids upstream product names ('LibreWolf|librewolf|Libre Wolf') to keep distribution branding coherent — a deb packager should read BearBrowser, not the base name. Turned the linux-packaging CI check red on main (and every PR since #104) — noticed only because #137 and #138 both have new YAML that queued a fresh CI run and I ran gh pr checks. Verified locally after fix: 'BearBrowser Linux packaging verified' Firefox is not banned (Mozilla's MPL requires attribution, and it's the engine name — plainly true and useful). Only the LibreWolf product name was the issue.
mdheller
added a commit
that referenced
this pull request
Jul 30, 2026
…ld (#138) * feat(rs-mirror): zero-trust review — we do not relay Mozilla blind Michael: 'zero trust dude we don't trust mozilla, we review the upstream and harden where possible too.' He is right and my earlier framing was still too trusting: a valid signature proves only that MOZILLA SIGNED IT, not that the content is benign. A compelled or compromised upstream ships signed records. review.mjs reads what we are about to serve and FAILS CLOSED on: a collection with no signature, a URL in signed content the collection has no business carrying, active-content markers (javascript:/<script/eval(), a new or vanished collection, or a >25% swing in record count — the shape a poisoned security list would take. A SHA256 baseline makes silent drift in already-vouched-for data visible on the next run. Made it collection-aware after the first pass produced noise — blunt rules get ignored, which is worse than no rule. ct-logs IS a list of log endpoints, intermediates embeds CPS URIs in certificate data, plugins/addons rows carry a user-facing 'why was this blocked' link. Those are inventoried; everywhere else a URL is a finding. (Same false-positive trap as calling plugins=5 a leak earlier — verify what upstream legitimately contains before alarming.) REAL findings from the first review of Mozilla PROD data: - Mozilla ships PLACEHOLDER urls in production CT data: https://ct.example.com/bogus/ and .../bogus/ipng/ - plugin-blocklist rows carry CLEARTEXT http:// advisory links (oracle, adobe, microsoft, divx, mozilla blog) Neither is fetched in normal operation, but we now SEE them instead of assuming, and the baseline diff surfaces the next change automatically. * fix(3 gaps): cockpit startup race + sovereign update channel + signing scaffold Closes the three items I flagged after v150.0.3. All three tested to the extent possible without a running build; each is behind an existing gate so a build without the corresponding piece is unaffected. 1. Cockpit startup race → cockpit-waiter.html. Was: new-tab opened resource://bearbrowser-cockpit/index.html immediately, but the gate at :8080 needed a beat to bind — cockpit UI hit ECONNREFUSED and looked broken. Now: new-tab points at a tiny waiter page in bearstart that polls http://127.0.0.1:8080/health with 250ms backoff and location.replace()s to the cockpit once the gate answers. 20s deadline with an honest 'the backend never bound' message if the cockpit was not assembled. Added to the packager stager and the verify-package.sh gate so a cockpit-carrying build without a waiter fails BEFORE ship. 2. Sovereign update channel → publish-latest-json.sh + weekly check in autoconfig + release-triggered workflow. Was: we killed aus5 correctly (leaks %OS_VERSION%/%BUILD_TARGET%/%LOCALE% per install) but that left NO channel to tell users a new version exists. Now: every release publishes a small latest.json to /releases/latest/ download/latest.json (idempotent). Autoconfig fetches it ONCE PER WEEK (pref-tracked timestamp), robustly parses semver-ish (rc suffixes → base version), and fires the observer 'bearbrowser-update-available' if newer. Header hygiene: credentials:'omit', referrerPolicy:'no-referrer', no cookies to GitHub. One request per user per week; no fingerprint-bearing template. Version parser tested inline: 5/5 cases pass. 3. Signing pipeline scaffolded (dispatch workflows, NO-OPS UNTIL SECRETS EXIST). Two new workflows: sign-and-notarize-macos.yml and sign-windows.yml. Both guard on their secrets and emit a ::warning:: without failing when Michael hasn't set them yet. When he does: macOS needs 6: APPLE_CODESIGN_IDENTITY, APPLE_ID, APPLE_APP_PASSWORD, APPLE_TEAM_ID, APPLE_P12_BASE64, APPLE_P12_PASSWORD Windows needs 2: WINDOWS_CODESIGN_PFX_BASE64, WINDOWS_CODESIGN_PFX_PASSWORD Both use ephemeral keychains/cert files that never persist on the runner. Both overwrite the ORIGINAL asset filename via --clobber so brew tap and winget URLs auto-pick up the signed version. macOS also regenerates SHA256SUMS.txt on the release so checksums stay honest. This is prep, not a signing event — it turns 'week of flailing' into ~10 minutes when the paid certs arrive. Not merging without Copilot review — draft. * fix: drop 'LibreWolf' from deb/control — linux-packaging check bans upstream product names Regression from PR #104 (my packaging cleanup a few days ago): I wrote 'LibreWolf-mirror fork of Firefox 150' in the deb description. verify-linux- packaging.sh forbids upstream product names ('LibreWolf|librewolf|Libre Wolf') to keep distribution branding coherent — a deb packager should read BearBrowser, not the base name. Turned the linux-packaging CI check red on main (and every PR since #104) — noticed only because #137 and #138 both have new YAML that queued a fresh CI run and I ran gh pr checks. Verified locally after fix: 'BearBrowser Linux packaging verified' Firefox is not banned (Mozilla's MPL requires attribution, and it's the engine name — plainly true and useful). Only the LibreWolf product name was the issue.
…honeypot integration test Ran my own adversarial pass on #136 before waiting for external review, since Michael pointed out I've been merging without one. Five real findings, plus the audit-archive work he asked for. All in one PR, Copilot requested before merge. 1. BearTrapMonitor SINK_UNSAFE: dead code + weak matcher. Was: /aus\d+\.mozilla\.org|addons\.mozilla\.org|systemAddon/i The 'systemAddon' clause is a Firefox pref-name fragment; it never matches a HOSTNAME. Removed. Also anchored the alternation with ^…$ so a lookalike subdomain (evil.addons.mozilla.org) cannot bypass. Whole-host match now. 2. Newtab rip: WARN when 0/N modules delete. The rip was silent-success on any upstream rename — exactly the class of drift Michael's ASK-WHAT-CALLS-THIS memory warns about. Now emits an ::warning:: if the dir exists but every target file was missing (someone renamed AdsFeed → AdsFeedV2 and we shipped it). 3. Runtime audit: split out of 'Package — tarball', explicit distro guard, check binary exists, extend timeout 40s → 90s (cold Firefox on CI runs past 40), and hard-fail (not || true) if xvfb install fails. The inline block muddied fedora's flow (apt-get in a fedora container is nonsense, swallowed by || true). Standalone step is auditable. 4. Cockpit toolchain: bun and pnpm were fetched from an unpinned installer — the moving-tag trap the memory warns about. Pinned bun=1.1.42, pnpm=9.15.4. Update deliberately when bumping, not by accident. 5. VENDOR HONEYPOT INTEGRATION TEST (the promised #2 from tomorrow's plan): The runtime audit now flips bearbrowser.honeypot.vendor.sink=true and, after the browser starts, curls /honeypot to verify a vendor-sink record exists for detectportal.firefox.com. Without this, 'we honeypot them, we don't just block them' was another 26/26-node-tests claim. Warning-not-fail on absence for the first run so a transient sidecar issue does not block the release loop — will tighten to fail-closed once we see it pass twice. 6. Audit report archived as CI artifact PASS OR FAIL — 60-day retention. Enables the same baseline-diff discipline the RS-mirror review uses; slow drift in a surface we forgot to check surfaces here first. Same shape on the DMG lane (verify-package.log). #3 from the plan. Not tested end-to-end against a real build (needs the pending nightly). Every mechanical claim above (regex fix, WARN emission, YAML validity, pin syntax) was verified locally.
mdheller
force-pushed
the
hardening-review-findings
branch
from
July 30, 2026 04:55
decae04 to
920c587
Compare
mdheller
marked this pull request as ready for review
July 30, 2026 04:57
There was a problem hiding this comment.
Pull request overview
This PR hardens the BearBrowser overlay based on adversarial review findings from #136, and strengthens CI auditing by pinning toolchain versions and archiving runtime/package verification outputs for later diffing.
Changes:
- Tighten vendor honeypot safety logic by anchoring
SINK_UNSAFEhost matching to whole-host values. - Improve “newtab rip” drift visibility by emitting a GitHub Actions warning when 0/N target modules are found while the directory still exists.
- Refactor Linux nightly runtime audit into an Ubuntu-only step with longer timeout and artifact archiving; pin bun/pnpm versions; archive DMG package verification output.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| settings/actors/BearTrapMonitor.sys.mjs | Anchors SINK_UNSAFE to whole-host matches to avoid unintended regex behavior in honeypot mode. |
| scripts/bearbrowser-patches.py | Adds a CI-visible warning when upstream drift causes the newtab module ripper to match zero targets. |
| .github/workflows/nightly-linux.yml | Pins bun/pnpm, splits runtime audit into an Ubuntu-only step, adds honeypot integration check, and archives audit outputs. |
| .github/workflows/nightly-dmg.yml | Archives verify-package.sh output (via tee) for audit trail / debugging. |
| [ -f scripts/audit/surface-report.json ] && break | ||
| sleep 1 | ||
| done | ||
| pkill -TERM -P $BB_PID 2>/dev/null || true |
Comment on lines
+323
to
+332
| node -e ' | ||
| const j = JSON.parse(require("fs").readFileSync("/tmp/honeypot.json","utf8")); | ||
| const sinks = (j.detections || []).filter(r => r.kind === "vendor-sink"); | ||
| if (sinks.length === 0) { | ||
| console.error("::warning::vendor honeypot did NOT record any sink — sidecar reachable, no BearTrap redirects"); | ||
| console.error(" ring size:", (j.detections || []).length, "kinds:", [...new Set((j.detections||[]).map(r=>r.kind))].join(",")); | ||
| } else { | ||
| console.log("VENDOR HONEYPOT VERIFIED: " + sinks.length + " sink record(s), dests: " + | ||
| [...new Set(sinks.map(r=>r.dest))].slice(0,5).join(", ")); | ||
| }' |
| # bugs: BearBlocker shipped with ZERO filter lists because | ||
| # FINAL_TARGET_FILES does not survive `mach package` — and nothing checked. | ||
| run: bash scripts/verify-package.sh "build/nightly/BearBrowser.app/Contents/Resources" | ||
| run: bash scripts/verify-package.sh "build/nightly/BearBrowser.app/Contents/Resources" | tee /tmp/verify-package-dmg.log |
Comment on lines
+113
to
+114
| BUN_VERSION=1.1.42 | ||
| curl -fsSL "https://bun.sh/install" | BUN_VERSION="bun-v${BUN_VERSION}" bash |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Not merging without external review. Adversarial pass on my own #136 (5 findings), bundled with audit-archive + honeypot-integration-test.
Findings from reviewing my own code
SINK_UNSAFEweak + dead code —systemAddonnever matched a hostname; alternation not anchored, soevil.addons.mozilla.orgcould slip. Fixed to^…$whole-host.::warning::when dir is present but 0/N targets found.if: matrix.distro == 'ubuntu', hard-fail on xvfb install, verify binary exists, 40s→90s.bearbrowser.honeypot.vendor.sink=trueand asserts/honeypotshows a sink record for detectportal.Untested end-to-end pending nightly. Every mechanical claim verified locally. Draft; requesting Copilot.