From 211e3cfc8e91cfad10fde3330d10624623c97ce6 Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Sun, 16 Aug 2026 14:52:45 +0530 Subject: [PATCH 1/5] feat(browserext): add browser extension inventory scan Inventories installed browser extensions across Chrome, Edge, Brave, Chromium and Firefox, reporting per-browser coverage alongside the findings so the backend can tell an empty browser from an unreadable one. Coverage is membership-complete: only scanned and partial browsers carry an authoritative extension list, while failed and not_present ship none. Chromium profiles are read from Local State plus Secure Preferences and Preferences; enabled state comes from the disable_reasons bitmask. Gecko profiles come from profiles.ini and extensions.json, carrying the signature state instead of the store fields. Extension name and locale are attribute reads, so a missing or unreadable manifest reduces the finding and marks the browser partial rather than discarding it. Adds a no-follow mode to safepath: an openat chain with O_NOFOLLOW on unix, and an unfollowed open verified against the kernel's final path on Windows. All reads are byte-capped and the phase carries its own deadline. Per-browser and per-profile caps bound one browser; the total finding cap and the deadline end the payload. Behind the browser_extensions_scan feature gate, off by default. --- internal/detector/browserext/catalog.go | 217 ++++++ internal/detector/browserext/chromium.go | 703 ++++++++++++++++++ internal/detector/browserext/chromium_test.go | 668 +++++++++++++++++ internal/detector/browserext/detector.go | 584 +++++++++++++++ internal/detector/browserext/detector_test.go | 658 ++++++++++++++++ .../detector/browserext/fifo_other_test.go | 10 + .../detector/browserext/fifo_unix_test.go | 10 + internal/detector/browserext/gecko.go | 435 +++++++++++ internal/detector/browserext/gecko_test.go | 476 ++++++++++++ internal/detector/browserext/session_other.go | 8 + .../detector/browserext/session_windows.go | 21 + internal/featuregate/featuregate.go | 5 + internal/model/browserext.go | 243 ++++++ internal/model/browserext_golden_test.go | 257 +++++++ internal/model/model.go | 6 + internal/model/scanresult_jsonshape_test.go | 15 + .../browser_extension_scan_golden.json | 224 ++++++ internal/output/html.go | 28 +- internal/output/pretty.go | 58 ++ internal/output/pretty_test.go | 73 ++ internal/safepath/open_unix.go | 19 +- internal/safepath/open_windows.go | 22 +- internal/safepath/safepath.go | 67 +- internal/safepath/safepath_test.go | 75 +- internal/scan/scanner.go | 18 + internal/telemetry/phase_deadline.go | 31 +- internal/telemetry/telemetry.go | 33 + tests/test_smoke_go.sh | 9 + 28 files changed, 4942 insertions(+), 31 deletions(-) create mode 100644 internal/detector/browserext/catalog.go create mode 100644 internal/detector/browserext/chromium.go create mode 100644 internal/detector/browserext/chromium_test.go create mode 100644 internal/detector/browserext/detector.go create mode 100644 internal/detector/browserext/detector_test.go create mode 100644 internal/detector/browserext/fifo_other_test.go create mode 100644 internal/detector/browserext/fifo_unix_test.go create mode 100644 internal/detector/browserext/gecko.go create mode 100644 internal/detector/browserext/gecko_test.go create mode 100644 internal/detector/browserext/session_other.go create mode 100644 internal/detector/browserext/session_windows.go create mode 100644 internal/model/browserext.go create mode 100644 internal/model/browserext_golden_test.go create mode 100644 internal/model/testdata/browser_extension_scan_golden.json diff --git a/internal/detector/browserext/catalog.go b/internal/detector/browserext/catalog.go new file mode 100644 index 00000000..839039fd --- /dev/null +++ b/internal/detector/browserext/catalog.go @@ -0,0 +1,217 @@ +// Package browserext inventories the extensions installed in a developer's +// browsers. +// +// The unit of collection is a browser: one set of data directories with its own +// parser family and its own coverage status. What a browser's status answers is +// deliberately narrow — is the set of extension identities reported for it +// complete? — because the state this feeds is stored per extension, and only a +// complete list can authorise deleting the rows it leaves out. Everything else +// (a name that would not resolve, a permission list too long to ship whole) is an +// attribute problem and degrades the browser without casting doubt on membership. +// +// Three properties hold for every read and are what keep the phase bounded, +// consent-safe and honest. Every location is an exact path under the target +// user's home rather than a tree to walk. Every read has a byte cap and goes +// through descriptors that follow no symlink, so a link planted anywhere on the +// path refuses instead of redirecting a privileged read. And nothing executes: no +// browser is launched, no store is called, no helper binary runs, and the +// browsers' SQLite stores — cookies, history, passwords — are never opened. +// +// Adding a browser of a family already here is a table row. A new family is a +// parser, by definition. +package browserext + +import ( + "path/filepath" + "time" + + "github.com/step-security/dev-machine-guard/internal/model" +) + +// catalogVersion is the revision of the browser list below, travelling with +// every scan so a reader can tell a list narrower than it expects from a browser +// that ran and found nothing. A string because a revision is an identifier: +// nothing compares two arithmetically. +const catalogVersion = "1" + +// Browser identifiers. Stable strings: coverage entries key on them and fleet +// views group by them, so renaming one splits that browser's history in two. +const ( + browserChrome = "chrome" + browserEdge = "edge" + browserBrave = "brave" + browserFirefox = "firefox" +) + +// engine names the browser family, which decides which parser runs and which +// engine-specific fields a finding carries. It is a classification and never an +// identity: Chrome, Edge and Brave are three browsers sharing one family, and +// grouping is always per (browser, extension). It stays inside this package — +// the value is a pure function of the browser id, so a reader derives it from +// its own copy of the catalog rather than being sent a second opinion. +type engine int + +const ( + engineChromium engine = iota + engineGecko +) + +// browserSpec is one browser's whole definition: an id, a family, and the data +// directory candidates to try per platform. +// +// Candidate paths are relative to the target user's home and are joined onto it, +// never read from the environment. $HOME, %LOCALAPPDATA% and %APPDATA% describe +// whichever account the agent process runs as — under an unattended deploy that +// is a service account — so a path built from them would scan the wrong home and +// report every browser missing. That answer is authoritative to a reader, which +// would then delete the device's real inventory. +// +// The consequence is documented rather than hidden: a browser launched with a +// custom data directory, or one whose config root moved with $XDG_CONFIG_HOME, +// stores its state outside every candidate here and reports as not present. That +// is honestly "no default directory exists", and the only rows it can retire are +// rows a scan of those same default directories wrote. +type browserSpec struct { + ID string + Engine engine + + // Slash-separated, home-relative. Multiple candidates per platform are + // normal: a native install and a snap or flatpak of the same browser both + // count, and both are scanned. + Darwin []string + Windows []string + Linux []string +} + +// catalog is the browser list: the head of the desktop share distribution. A +// browser covered by a row costs no parser work; one that is not covered is +// absent from the payload entirely, which is what "not attempted" means to a +// reader. +// +// A candidate marked unconfirmed comes from vendor documentation rather than from +// a machine running that packaging. The cost of a wrong one is bounded: a +// directory that does not exist reports the browser as not present, which is the +// same answer as not carrying the candidate at all. +// +// Iteration order is the order below and matters: caps cut at browser +// boundaries, so a deterministic order is what makes two runs over an unchanged +// machine produce the same payload. +var catalog = []browserSpec{ + { + ID: browserChrome, + Engine: engineChromium, + Darwin: []string{"Library/Application Support/Google/Chrome"}, + Windows: []string{"AppData/Local/Google/Chrome/User Data"}, + Linux: []string{".config/google-chrome"}, + }, + { + ID: browserEdge, + Engine: engineChromium, + Darwin: []string{"Library/Application Support/Microsoft Edge"}, + Windows: []string{"AppData/Local/Microsoft/Edge/User Data"}, + Linux: []string{ + ".config/microsoft-edge", + ".var/app/com.microsoft.Edge/config/microsoft-edge", // flatpak, unconfirmed + }, + }, + { + ID: browserBrave, + Engine: engineChromium, + Darwin: []string{"Library/Application Support/BraveSoftware/Brave-Browser"}, + Windows: []string{"AppData/Local/BraveSoftware/Brave-Browser/User Data"}, + // The snap packaging is absent deliberately. Its data directory sits under + // a `current` link that snapd repoints on every revision, and a path + // through a link is refused rather than followed — so carrying it would + // report a permanent failure, and take a native install alongside it down + // with it. Covering that packaging means enumerating revisions, not adding + // a row. + Linux: []string{ + ".config/BraveSoftware/Brave-Browser", + ".var/app/com.brave.Browser/config/BraveSoftware/Brave-Browser", // flatpak, unconfirmed + }, + }, + { + // One id covers every channel: release, beta, Nightly, ESR and Developer + // Edition share this directory and separate themselves by profile. The + // profile-name suffix hints at the channel but is not reliable identity, + // so the channel is not reported. + ID: browserFirefox, + Engine: engineGecko, + Darwin: []string{"Library/Application Support/Firefox"}, + Windows: []string{"AppData/Roaming/Mozilla/Firefox"}, + Linux: []string{ + ".mozilla/firefox", + "snap/firefox/common/.mozilla/firefox", + ".var/app/org.mozilla.firefox/.mozilla/firefox", + }, + }, +} + +// roots returns this browser's data directory candidates on one platform, as +// absolute paths under home. +func (s browserSpec) roots(platform, home string) []string { + var rel []string + switch platform { + case model.PlatformDarwin: + rel = s.Darwin + case model.PlatformWindows: + rel = s.Windows + case model.PlatformLinux: + rel = s.Linux + } + if home == "" || len(rel) == 0 { + return nil + } + out := make([]string, 0, len(rel)) + for _, r := range rel { + out = append(out, filepath.Join(home, filepath.FromSlash(r))) + } + return out +} + +// List caps. Every one of them cuts at a browser boundary rather than inside a +// browser's extension list, because a reported browser's membership is taken as +// complete: a list silently cut in half would retire the extensions below the cut. +const ( + maxProfilesPerBrowser = 20 // counted across all of a browser's candidates + maxExtensionsPerBrowser = 500 + maxExtensionsTotal = 2000 + maxPermissionsPerFinding = 64 // API permissions and host patterns each +) + +// String caps in BYTES, not runes: the wire and the reader's caps are +// byte-denominated, so the producer's have to be too. Truncation backs up to a +// rune boundary so a capped string is still valid UTF-8. +const ( + maxNameBytes = 256 + maxVersionBytes = 64 + maxExtensionIDBytes = 256 // gecko ids are e-mail or UUID shaped; chromium is 32 + maxPermissionBytes = 256 // per entry +) + +// File read caps. Validated against the descriptor the bytes come from, and a +// read of one byte past the cap means the file outgrew it — that refuses rather +// than handing the parser a prefix, because a truncated JSON document either +// fails to parse for the wrong reason or, worse, parses. +const ( + maxLocalStateBytes = 4 << 20 + maxPrefsBytes = 10 << 20 // heavy profiles reach megabytes + maxManifestBytes = 1 << 20 + maxMessagesBytes = 512 << 10 + maxExtensionsJSONBytes = 10 << 20 + maxProfilesINIBytes = 256 << 10 +) + +// Directory listing bounds. The directories below are exactly the ones a local +// process can fill, so an overlong listing is refused rather than measured. +const ( + maxRootEntries = 512 + maxProfileEntries = 512 + maxVersionEntries = 64 +) + +// browserExtPhaseBudget is the detector's own deadline, well inside the phase +// budget the orchestrator applies. It bounds the work this phase chooses to do; +// it cannot interrupt a blocked syscall, which is why no read is allowed to be +// able to block in the first place. +const browserExtPhaseBudget = 60 * time.Second diff --git a/internal/detector/browserext/chromium.go b/internal/detector/browserext/chromium.go new file mode 100644 index 00000000..6e2e4c86 --- /dev/null +++ b/internal/detector/browserext/chromium.go @@ -0,0 +1,703 @@ +package browserext + +import ( + "context" + "encoding/json" + "path/filepath" + "slices" + "sort" + "strings" + + "github.com/step-security/dev-machine-guard/internal/model" +) + +// The Chromium family: one parser for Chrome, Edge and Brave, whose data +// directories have the same shape on every desktop platform — a `Local State` +// file naming the profiles, and one directory per profile holding that profile's +// preferences. + +// Update servers, matched by prefix because a real update URL carries query +// parameters. The URL itself never leaves the machine: a self-hosted update +// server's hostname is internal infrastructure, so only the label ships. +const ( + chromeWebStoreUpdateURL = "https://clients2.google.com/service/update2/crx" + edgeAddonsUpdateURL = "https://edge.microsoft.com/extensionwebstorebase/v1/crx" +) + +// Install locations, as the browser records them. Component extensions are the +// browser's own parts and are not reported at all; every other value is, including +// the ones the browser cannot classify. +const ( + locationInternal = 1 + locationExternalPref = 2 + locationExternalRegistry = 3 + locationUnpacked = 4 + locationComponent = 5 + locationExternalPrefDownload = 6 + locationExternalPolicyDownload = 7 + locationCommandLine = 8 + locationExternalPolicy = 9 + locationExternalComponent = 10 +) + +// Disable reasons worth naming, as the browser's own enumeration numbers them. +// It records a set of them, and only these three change the answer: a set holding +// the user's own action was the user's doing, one holding a policy reason was the +// administrator's, and any other non-empty set means the browser disabled the +// extension for a reason of its own — which is the case worth surfacing, since a +// store takedown lands here. Bits are appended to that enumeration rather than +// renumbered, so an unrecognised value deliberately reads as "the browser did it" +// rather than as a value to interpret. +const ( + disableReasonUserAction = 1 << 0 + disableReasonPolicyUpdateRequire = 1 << 13 + disableReasonBlockedByPolicy = 1 << 15 +) + +// scanChromiumRoot reads one Chromium-family data directory and reports whether +// it held an installation. +func (d *Detector) scanChromiumRoot(ctx context.Context, scan *scanState, root string, b *browserScan) bool { + data, missing, reason := scan.readState(filepath.Join(root, "Local State"), maxLocalStateBytes) + if reason != "" { + b.fail(reason) + return true + } + if missing { + return d.classifyChromiumRoot(scan, root, b) + } + + profiles, ok := parseProfileDirs(data) + if !ok { + // The profile list is what makes the extension list complete, so an + // unreadable one means membership is unknowable: an unknown profile can + // hold extensions. Guessing the layout instead — globbing for `Profile *`, + // or reading `Default` alone — would ship a partial list under a status + // that reads as complete. + b.fail(model.BrowserExtReasonParseError) + return true + } + for _, profile := range profiles { + if ctx.Err() != nil { + b.failPayload(model.BrowserExtReasonTimedOut, model.BrowserExtTruncatedDeadline) + return true + } + if b.profiles >= maxProfilesPerBrowser { + // An unscanned profile can hide extensions, so a bounded profile list + // is a membership question and fails the browser rather than + // degrading it. + b.failBounded(model.BrowserExtReasonCapped, model.BrowserExtTruncatedFindingCap) + return true + } + b.profiles++ + d.scanChromiumProfile(scan, root, profile, b) + if b.failure != "" { + return true + } + } + return true +} + +// classifyChromiumRoot decides what a data directory with no `Local State` is. A +// directory can exist while the browser never has — installers leave one behind +// holding nothing but an empty native-messaging folder — and calling that a +// failure would paint a permanent red row for a browser nobody installed, on +// every scan, which is how a coverage list stops being read. +// +// It reads directory entries only; no file is opened. Reporting the directory as +// absent is authoritative, and safely so: the only rows it can retire are rows a +// previous scan of this same empty directory wrote, and that scan cannot have +// found anything either. +func (d *Detector) classifyChromiumRoot(scan *scanState, root string, b *browserScan) bool { + names, missing, reason := scan.listNames(root, maxRootEntries) + if reason != "" { + b.fail(reason) + return true + } + if missing { + return false + } + for _, name := range names { + if name == "Default" || name == "Secure Preferences" || name == "Preferences" || + strings.HasPrefix(name, "Profile ") { + // A profile is here but the file naming the profiles is not: this is + // a broken installation, not an absent one. + b.fail(model.BrowserExtReasonParseError) + return true + } + } + return false +} + +// parseProfileDirs returns the profile directory names from `Local State`. +// +// The values beside those names carry the profile's display label, the signed-in +// account's name and its e-mail address. They are decoded as raw bytes and never +// looked at: the directory basename is the only part this reads, and even that +// stays inside the detector. +func parseProfileDirs(data []byte) ([]string, bool) { + var state struct { + Profile struct { + InfoCache map[string]json.RawMessage `json:"info_cache"` + } `json:"profile"` + } + if err := json.Unmarshal(data, &state); err != nil { + return nil, false + } + if len(state.Profile.InfoCache) == 0 { + // A data directory that names no profile tells us nothing about which + // profiles exist, which is not the same as telling us there are none. + return nil, false + } + names := make([]string, 0, len(state.Profile.InfoCache)) + for name := range state.Profile.InfoCache { + if !isDirName(name) { + // A key that is not a directory basename would move the read + // somewhere else entirely. The file is not trustworthy, so nothing + // derived from it is. + return nil, false + } + names = append(names, name) + } + sort.Strings(names) + return names, true +} + +// isDirName reports whether name can be one directory entry: not empty, not a +// traversal, and carrying no separator of either flavour. +func isDirName(name string) bool { + if name == "" || name == "." || name == ".." { + return false + } + return !strings.ContainsAny(name, `/\`) +} + +// hasParentComponent reports whether a relative path steps above its base. A +// component test rather than a substring one: a version directory is free to +// contain dots, and "1..0" walks nowhere. +func hasParentComponent(path string) bool { + for _, c := range strings.FieldsFunc(path, func(r rune) bool { return r == '/' || r == '\\' }) { + if c == ".." { + return true + } + } + return false +} + +// scanChromiumProfile reads one profile's extension records. +// +// `Secure Preferences` is read first on every platform, because that is where the +// extension map lives on all of them — the belief that Linux keeps it in plain +// `Preferences` is wrong: what differs per platform is whether the browser +// enforces the file's integrity, not where it writes it. Plain `Preferences` fills +// in ids the first file does not carry, which split preference tracking makes +// possible. The integrity fields beside them are skipped: this reads, and the +// browser's own tamper detection is not its business. +func (d *Detector) scanChromiumProfile(scan *scanState, root, profile string, b *browserScan) { + dir := filepath.Join(root, profile) + settings := map[string]json.RawMessage{} + for _, name := range []string{"Secure Preferences", "Preferences"} { + data, missing, reason := scan.readState(filepath.Join(dir, name), maxPrefsBytes) + if reason != "" { + b.fail(reason) + return + } + if missing { + continue + } + entries, ok := parseExtensionSettings(data) + if !ok { + b.fail(model.BrowserExtReasonParseError) + return + } + for id, raw := range entries { + if _, seen := settings[id]; !seen { + settings[id] = raw + } + } + } + + ids := make([]string, 0, len(settings)) + for id := range settings { + ids = append(ids, id) + } + sort.Strings(ids) + + for _, id := range ids { + occ, ok := d.chromiumOccurrence(scan, dir, id, settings[id], b) + if !ok { + continue + } + // The data directory and profile together order the occurrences of one + // extension seen in several places. + occ.sortKey = dir + if !b.add(id, occ) { + return + } + } +} + +// parseExtensionSettings returns the per-extension map from a preferences file. +// Values stay raw so one corrupt record cannot cost the whole file. +func parseExtensionSettings(data []byte) (map[string]json.RawMessage, bool) { + var prefs struct { + Extensions struct { + Settings map[string]json.RawMessage `json:"settings"` + } `json:"extensions"` + } + if err := json.Unmarshal(data, &prefs); err != nil { + return nil, false + } + return prefs.Extensions.Settings, true +} + +// chromiumEntry is one record under the extension map. Every field is optional: +// the file is written by a browser whose shape changes between releases, and a +// missing field degrades one attribute rather than the record. +type chromiumEntry struct { + Location *int `json:"location"` + DisableReasons json.RawMessage `json:"disable_reasons"` + // The pre-list form of the same fact, read only when disable_reasons is + // absent: current browsers do not write it. + State *int `json:"state"` + // Relative to the profile's own Extensions directory for a store install, + // absolute for an unpacked one. An absolute path is never opened and never + // ships. + Path string `json:"path"` + Manifest *chromiumManifest `json:"manifest"` + FromWebstore *bool `json:"from_webstore"` + WasInstalledByDefault bool `json:"was_installed_by_default"` + WasInstalledByOEM bool `json:"was_installed_by_oem"` + Granted *chromiumPermissions `json:"granted_permissions"` + RuntimeGranted *chromiumPermissions `json:"runtime_granted_permissions"` + CWSInfo *chromiumCWSInfo `json:"cws-info"` +} + +// chromiumManifest is the copy of the extension's manifest the browser keeps +// inside its own preferences. It covers almost every extension, which is what +// keeps this phase to two file reads per profile. +type chromiumManifest struct { + Name string `json:"name"` + Version string `json:"version"` + DefaultLocale string `json:"default_locale"` + UpdateURL string `json:"update_url"` + // Presence alone is the test: a manifest with either key is a theme or a + // legacy packaged app rather than an extension. + Theme json.RawMessage `json:"theme"` + App json.RawMessage `json:"app"` +} + +// chromiumPermissions is one grant record. The granted sets are preferred over +// what the manifest declared, because a declaration is a request and these are +// what the browser actually handed over. +type chromiumPermissions struct { + API []string `json:"api"` + ExplicitHost []string `json:"explicit_host"` + ScriptableHost []string `json:"scriptable_host"` +} + +// chromiumCWSInfo is the browser's cached belief about the store's listing. It is +// the highest-value pair in the record: an extension the store has pulled while +// the machine still runs it, with its permissions granted, is exactly the shape of +// a compromised extension, and nothing else on disk says so. +type chromiumCWSInfo struct { + IsLive *bool `json:"is-live"` + ViolationType *int `json:"violation-type"` +} + +// chromiumOccurrence turns one preference record into one occurrence, or reports +// that the record does not describe an installed extension. +func (d *Detector) chromiumOccurrence(scan *scanState, profileDir, id string, raw json.RawMessage, b *browserScan) (occurrence, bool) { + if !chromiumIDShape(id) { + // The browser generates every extension id — store, sideload, policy and + // unpacked alike — as thirty-two letters from the first sixteen of the + // alphabet, so a key of another shape cannot name an extension and the + // browser's own loader could not load it. A classification filter, not a + // degradation: what this key names is not in the extension set at all. + return occurrence{}, false + } + + var e chromiumEntry + if err := json.Unmarshal(raw, &e); err != nil { + // The map key is the identity, so a corrupt record costs the metadata + // and not the extension. Reported with what survived, which keeps + // membership complete while the browser goes partial. + b.degrade(model.BrowserExtReasonManifestUnavailable) + return reducedOccurrence(), true + } + if e.Manifest == nil && e.Path == "" && e.Location == nil { + // Bookkeeping residue: an update-ping or allowlist stub with nothing the + // browser could load. It is not listed as an extension by the browser + // either, so reporting it would invent one — and, having no manifest, it + // would also degrade the browser on every scan for ever. Any one of the + // three fields present means there was something real to record, and the + // reduced-finding path below covers it. + return occurrence{}, false + } + + source, reported := installSource(e.Location) + if !reported { + // A part of the browser itself rather than something installed into it. + return occurrence{}, false + } + + manifest := e.Manifest + // Only a store install's path is relative, which is what makes it safe to + // resolve: it lands inside the browser's own tree. An unpacked extension's + // path is an arbitrary user location — this never opens one, and never ships + // it either, so nothing about it is read beyond what the preferences recorded. + extDir := "" + if e.Path != "" && !filepath.IsAbs(e.Path) && !hasParentComponent(e.Path) { + extDir = filepath.Join(profileDir, "Extensions", filepath.FromSlash(e.Path)) + } + if manifest == nil && extDir != "" { + data, missing, reason := scan.readState(filepath.Join(extDir, "manifest.json"), maxManifestBytes) + if reason != "" { + // Membership came from the preference map, which has already been read + // whole. Nothing this file could have said would add or remove an + // extension, so a refusal costs the metadata and the browser goes + // partial rather than losing a list that is known complete. + b.degrade(reason) + } + if !missing && reason == "" { + var m chromiumManifest + if json.Unmarshal(data, &m) == nil { + manifest = &m + } + } + } + if manifest != nil && (len(manifest.Theme) > 0 || len(manifest.App) > 0) { + // A theme or a legacy packaged app. Neither runs code against pages, so + // neither is what this inventory is about. + return occurrence{}, false + } + + name, version := "", "" + if manifest == nil { + b.degrade(model.BrowserExtReasonManifestUnavailable) + } else { + name = d.resolveExtensionName(scan, extDir, manifest, b) + version = manifest.Version + } + + state, disabledBy := chromiumEnabledState(e) + listing, violation := storeDisposition(e.CWSInfo) + perms, hosts, capped := permissionLists(e.Granted, e.RuntimeGranted) + if capped { + b.degrade(model.BrowserExtReasonCapped) + } + return occurrence{ + enabled: state, + disabledBy: disabledBy, + block: model.BrowserExtensionFinding{ + Name: capBytes(name, maxNameBytes), + Version: capBytes(version, maxVersionBytes), + EnabledState: state, + StoreListing: listing, + StoreViolation: violation, + InstallSource: source, + Store: chromiumStore(manifest, e), + Preinstalled: e.WasInstalledByDefault || e.WasInstalledByOEM, + Permissions: perms, + HostPermissions: hosts, + }, + }, true +} + +// reducedOccurrence is what a record whose value could not be read produces: an +// identity, and everything else spelled as unknown rather than left out. A reader +// requires the two store fields on this family, and "cannot tell" is a value. +func reducedOccurrence() occurrence { + return occurrence{ + enabled: model.BrowserExtStateUnknown, + block: model.BrowserExtensionFinding{ + EnabledState: model.BrowserExtStateUnknown, + StoreListing: model.BrowserExtStoreListingUnknown, + StoreViolation: model.BrowserExtStoreViolationUnknown, + InstallSource: model.BrowserExtInstallUnknown, + Store: model.BrowserExtStoreUnknown, + Permissions: []string{}, + HostPermissions: []string{}, + }, + } +} + +// chromiumIDShape reports whether id has the shape this family generates: exactly +// thirty-two characters from a through p. +func chromiumIDShape(id string) bool { + if len(id) != 32 { + return false + } + for i := 0; i < len(id); i++ { + if id[i] < 'a' || id[i] > 'p' { + return false + } + } + return true +} + +// installSource maps a recorded location to how the extension got there, +// reporting false for the values that are the browser's own components. +func installSource(location *int) (string, bool) { + if location == nil { + return model.BrowserExtInstallUnknown, true + } + switch *location { + case locationInternal: + return model.BrowserExtInstallUser, true + case locationExternalPref, locationExternalPrefDownload: + return model.BrowserExtInstallSideload, true + case locationExternalRegistry: + return model.BrowserExtInstallRegistry, true + case locationUnpacked, locationCommandLine: + // The highest-signal rows in the whole inventory, and never excluded. + return model.BrowserExtInstallUnpacked, true + case locationExternalPolicyDownload, locationExternalPolicy: + // The installed state is evidence enough of a policy install; the policy + // sources themselves are not read, which keeps this phase to file reads + // inside one home. + return model.BrowserExtInstallPolicy, true + case locationComponent, locationExternalComponent: + return "", false + default: + return model.BrowserExtInstallUnknown, true + } +} + +// chromiumEnabledState derives whether the extension runs, and who stopped it. +// +// Enabled is the empty disable-reason set — not a flag, and not the absence of +// the record. The reason set names the actor rather than the cause: two very +// different browser decisions carry the same value, which is why the store +// disposition is read separately. +func chromiumEnabledState(e chromiumEntry) (state, disabledBy string) { + if len(e.DisableReasons) == 0 { + if e.State != nil { + // A profile old enough to predate the reason set. Last resort: it + // says whether, not why. + if *e.State == 1 { + return model.BrowserExtEnabled, "" + } + return model.BrowserExtDisabled, model.BrowserExtDisabledByUnknown + } + return model.BrowserExtEnabled, "" + } + reasons, ok := parseDisableReasons(e.DisableReasons) + if !ok { + // The field is there and unreadable, so enabled and disabled are + // indistinguishable. Saying either would be a guess a console would + // display as fact. + return model.BrowserExtStateUnknown, "" + } + if len(reasons) == 0 { + return model.BrowserExtEnabled, "" + } + for _, r := range reasons { + if r == disableReasonUserAction { + return model.BrowserExtDisabled, model.BrowserExtDisabledByUser + } + } + for _, r := range reasons { + if r == disableReasonBlockedByPolicy || r == disableReasonPolicyUpdateRequire { + return model.BrowserExtDisabled, model.BrowserExtDisabledByPolicy + } + } + return model.BrowserExtDisabled, model.BrowserExtDisabledByBrowser +} + +// parseDisableReasons reads both shapes in the wild: current browsers write a +// list of values, older profiles a single combined bitmask. +func parseDisableReasons(raw json.RawMessage) ([]int, bool) { + var list []int + if json.Unmarshal(raw, &list) == nil { + return list, true + } + var mask int + if json.Unmarshal(raw, &mask) != nil || mask < 0 { + return nil, false + } + var out []int + for bit := 1; bit > 0 && bit <= mask; bit <<= 1 { + if mask&bit != 0 { + out = append(out, bit) + } + } + return out, true +} + +// storeDisposition reads the cached store listing. An absent record is unknown +// and never listed: inferring that the store still carries an extension from the +// absence of a record would turn a missing answer into a reassuring one. +// +// It is a cached belief, refreshed on the browser's update ping, so a browser that +// has not run since a takedown still says listed. Delisted is a strong positive +// and listed a weak negative, which is a distinction the copy in front of a +// customer has to keep. +func storeDisposition(info *chromiumCWSInfo) (listing, violation string) { + listing, violation = model.BrowserExtStoreListingUnknown, model.BrowserExtStoreViolationUnknown + if info == nil { + return listing, violation + } + if info.IsLive != nil { + if *info.IsLive { + listing = model.BrowserExtStoreListingListed + } else { + listing = model.BrowserExtStoreListingDelisted + } + } + if info.ViolationType != nil { + if *info.ViolationType == 0 { + violation = model.BrowserExtStoreViolationNone + } else { + violation = model.BrowserExtStoreViolationFlagged + } + } + return listing, violation +} + +// chromiumStore attributes the extension to a store, as a label and never a URL. +// A Brave install carries the Chrome Web Store's own update URL because Brave +// proxies that store, which is the right answer from where the code came from. +func chromiumStore(manifest *chromiumManifest, e chromiumEntry) string { + url := "" + if manifest != nil { + url = strings.TrimSpace(manifest.UpdateURL) + } + switch { + case url == "": + if e.FromWebstore != nil && *e.FromWebstore { + return model.BrowserExtStoreChromeWebStore + } + return model.BrowserExtStoreUnknown + case strings.HasPrefix(url, chromeWebStoreUpdateURL): + return model.BrowserExtStoreChromeWebStore + case strings.HasPrefix(url, edgeAddonsUpdateURL): + return model.BrowserExtStoreEdgeAddons + default: + return model.BrowserExtStoreSelfHosted + } +} + +// resolveExtensionName resolves a localized name to the string a person would +// see. Store installs only, on the same grounds as the manifest read: the message +// table sits inside the browser's own tree. +func (d *Detector) resolveExtensionName(scan *scanState, extDir string, manifest *chromiumManifest, b *browserScan) string { + key, localized := messageKey(manifest.Name) + if !localized || extDir == "" { + return manifest.Name + } + for _, locale := range localeCandidates(manifest.DefaultLocale) { + data, missing, reason := scan.readState( + filepath.Join(extDir, "_locales", locale, "messages.json"), maxMessagesBytes) + if reason != "" { + // A name is an attribute. The identity is already known, so an + // unreadable message table degrades the browser and leaves the + // placeholder standing. + b.degrade(reason) + return manifest.Name + } + if missing { + continue + } + if value := lookupMessage(data, key); value != "" { + return value + } + } + // Nothing resolved it. The placeholder is still the closest thing to a name + // this extension has, and an empty one would read as metadata never recorded. + return manifest.Name +} + +// messageKey extracts the message name from a localized manifest value. +func messageKey(name string) (string, bool) { + if !strings.HasPrefix(name, "__MSG_") || !strings.HasSuffix(name, "__") || len(name) <= len("__MSG___") { + return "", false + } + return name[len("__MSG_") : len(name)-len("__")], true +} + +// localeCandidates returns the message tables to try, in order. The declared +// default locale is read rather than the machine's, so the resolved name is the +// same on every machine holding the same extension. Directory names use +// underscores rather than the hyphen of a language tag. +func localeCandidates(defaultLocale string) []string { + candidates := make([]string, 0, 3) + for _, locale := range []string{strings.ReplaceAll(defaultLocale, "-", "_"), "en_US", "en"} { + if locale == "" || !isDirName(locale) { + continue + } + if !slices.Contains(candidates, locale) { + candidates = append(candidates, locale) + } + } + return candidates +} + +// lookupMessage returns one message's text. Keys are compared case-insensitively, +// which is what the browser itself does on both sides of the lookup. +func lookupMessage(data []byte, key string) string { + var table map[string]struct { + Message string `json:"message"` + } + if err := json.Unmarshal(data, &table); err != nil { + return "" + } + want := strings.ToLower(key) + for name, entry := range table { + if strings.ToLower(name) == want { + return entry.Message + } + } + return "" +} + +// permissionLists reduces the grant records to the two wire lists. +// +// Both grant paths are unioned, because either alone misreports what the +// extension can reach: the up-front set keeps patterns the user has since +// withheld, and the runtime set is the only home for what they granted on +// demand. The union is the honest upper bound of effective access. +func permissionLists(granted, runtime *chromiumPermissions) (perms, hosts []string, capped bool) { + var api, host []string + for _, set := range []*chromiumPermissions{granted, runtime} { + if set == nil { + continue + } + api = append(api, set.API...) + host = append(host, set.ExplicitHost...) + host = append(host, set.ScriptableHost...) + } + perms, apiCapped := capPermissionList(api) + hosts, hostCapped := capPermissionList(host) + return perms, hosts, apiCapped || hostCapped +} + +// capPermissionList sorts, deduplicates and bounds one permission list, reporting +// whether anything was left out. +// +// An over-long entry is dropped rather than shortened. These strings are matched +// and not read — a permission is compared for equality and a host pattern is a +// match expression — so a shortened one is a different grant, and showing whoever +// is auditing a permission the extension never held is worse than showing them +// one fewer. +func capPermissionList(in []string) ([]string, bool) { + out := make([]string, 0, len(in)) + capped := false + seen := map[string]bool{} + for _, entry := range in { + if entry == "" || seen[entry] { + continue + } + seen[entry] = true + if len(entry) > maxPermissionBytes { + capped = true + continue + } + out = append(out, entry) + } + sort.Strings(out) + if len(out) > maxPermissionsPerFinding { + out = out[:maxPermissionsPerFinding] + capped = true + } + return out, capped +} diff --git a/internal/detector/browserext/chromium_test.go b/internal/detector/browserext/chromium_test.go new file mode 100644 index 00000000..6aa5621b --- /dev/null +++ b/internal/detector/browserext/chromium_test.go @@ -0,0 +1,668 @@ +package browserext + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/step-security/dev-machine-guard/internal/model" +) + +// Extension ids of the shape this family generates: thirty-two characters from a +// through p. +const ( + idA = "abcdefghijklmnopabcdefghijklmnop" + idB = "bcdefghijklmnopabcdefghijklmnopa" + idC = "cdefghijklmnopabcdefghijklmnopab" +) + +// manySettings builds n extension records, for the tests that push a bound. The +// counter is spelled in the sixteen letters this family's ids are made of, so +// every one of them passes the shape gate. +func manySettings(n int) string { + const letters = "abcdefghijklmnop" + entries := make([]string, 0, n) + for i := range n { + var spelled strings.Builder + for _, digit := range fmt.Sprintf("%04d", i) { + spelled.WriteByte(letters[digit-'0']) + } + id := strings.Repeat("a", 28) + spelled.String() + entries = append(entries, `"`+id+`": {"location": 1, "manifest": {"name": "Example", "version": "1.0"}}`) + } + return strings.Join(entries, ",") +} + +// chromeFinding runs one Chrome profile and returns its single finding plus the +// browser's coverage entry, which is what most of the parsing rules are stated in +// terms of. +func chromeFinding(t *testing.T, settings string) (model.BrowserExtensionFinding, model.BrowserCoverage) { + t.Helper() + home := tempHome(t) + localState(t, chromeRoot(home), "Default") + securePrefs(t, chromeRoot(home), "Default", settings) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserChrome) + if len(got) != 1 { + t.Fatalf("findings = %d, want exactly one", len(got)) + } + return got[0], coverageFor(t, info, browserChrome) +} + +// TestChromium_PrefsResidenceAndMerge pins where the extension map is read from. +// It lives in the integrity-tracked file on every platform — the belief that Linux +// keeps it in the plain one is wrong, and reading the plain file first would report +// a stale record over the live one. +func TestChromium_PrefsResidenceAndMerge(t *testing.T) { + home := tempHome(t) + root := chromeRoot(home) + localState(t, root, "Default") + securePrefs(t, root, "Default", `"`+idA+`": { + "location": 1, "manifest": {"name": "From Secure", "version": "1.0"} + }`) + writeFile(t, filepath.Join(root, "Default", "Preferences"), `{"extensions": {"settings": { + "`+idA+`": {"location": 1, "manifest": {"name": "From Plain", "version": "9.9"}}, + "`+idB+`": {"location": 1, "manifest": {"name": "Only In Plain", "version": "2.0"}} + }}}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserChrome) + if len(got) != 2 { + t.Fatalf("findings = %d, want the union of both files", len(got)) + } + byID := map[string]model.BrowserExtensionFinding{} + for _, f := range got { + byID[f.ExtensionID] = f + } + if name := byID[idA].Name; name != "From Secure" { + t.Errorf("name = %q, want the integrity-tracked file to win", name) + } + if name := byID[idB].Name; name != "Only In Plain" { + t.Errorf("name = %q, want the plain file to fill an id the other lacks", name) + } +} + +// TestChromium_EnabledState covers every shape the disable record takes in the +// wild, and what each one says about who turned the extension off. Enabled is the +// empty set — not a flag, and not the record's absence. +func TestChromium_EnabledState(t *testing.T) { + tests := []struct { + name string + record string + state string + disabledBy string + }{ + { + name: "no disable record is enabled", + record: `"location": 1`, + state: model.BrowserExtEnabled, + }, + { + name: "empty list is enabled", + record: `"location": 1, "disable_reasons": []`, + state: model.BrowserExtEnabled, + }, + { + name: "the user's own action", + record: `"location": 1, "disable_reasons": [1]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByUser, + }, + { + // The browser's own decision, which is where a store takedown lands. + name: "a reason of the browser's own", + record: `"location": 1, "disable_reasons": [512]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByBrowser, + }, + { + name: "an administrator's policy holding an update back", + record: `"location": 1, "disable_reasons": [8192]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByPolicy, + }, + { + // The other policy bit, and the one an administrator reaches for to + // ban an extension outright. + name: "an administrator's policy blocking it outright", + record: `"location": 1, "disable_reasons": [32768]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByPolicy, + }, + { + // The user's action wins the cause when a set carries several. + name: "the user's action alongside another reason", + record: `"location": 1, "disable_reasons": [1, 512]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByUser, + }, + { + name: "the older combined bitmask", + record: `"location": 1, "disable_reasons": 513`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByUser, + }, + { + name: "the bitmask reading zero is enabled", + record: `"location": 1, "disable_reasons": 0`, + state: model.BrowserExtEnabled, + }, + { + name: "the pre-list flag, read only when nothing else says", + record: `"location": 1, "state": 1`, + state: model.BrowserExtEnabled, + }, + { + name: "the pre-list flag saying disabled", + record: `"location": 1, "state": 0`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByUnknown, + }, + { + // Present and unreadable: enabled and disabled are indistinguishable, + // and saying either would be a guess displayed as a fact. + name: "an unreadable disable record", + record: `"location": 1, "disable_reasons": "wat"`, + state: model.BrowserExtStateUnknown, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": {`+tc.record+`, "manifest": {"name": "Example", "version": "1.0"}}`) + if got.EnabledState != tc.state { + t.Errorf("enabled_state = %q, want %q", got.EnabledState, tc.state) + } + if got.DisabledBy != tc.disabledBy { + t.Errorf("disabled_by = %q, want %q", got.DisabledBy, tc.disabledBy) + } + }) + } +} + +// TestChromium_InstallSource maps every recorded location. The values the browser +// uses for its own components are the only ones that produce no row at all. +func TestChromium_InstallSource(t *testing.T) { + tests := []struct { + name string + location string + want string + }{ + {name: "an ordinary install", location: "1", want: model.BrowserExtInstallUser}, + {name: "a sideload through external preferences", location: "2", want: model.BrowserExtInstallSideload}, + {name: "a downloaded sideload", location: "6", want: model.BrowserExtInstallSideload}, + {name: "a registry sideload", location: "3", want: model.BrowserExtInstallRegistry}, + {name: "developer mode", location: "4", want: model.BrowserExtInstallUnpacked}, + {name: "a command-line load", location: "8", want: model.BrowserExtInstallUnpacked}, + {name: "an administrator install", location: "7", want: model.BrowserExtInstallPolicy}, + {name: "an administrator install, other form", location: "9", want: model.BrowserExtInstallPolicy}, + {name: "a value this build does not know", location: "42", want: model.BrowserExtInstallUnknown}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": {"location": `+tc.location+ + `, "manifest": {"name": "Example", "version": "1.0"}}`) + if got.InstallSource != tc.want { + t.Errorf("install_source = %q, want %q", got.InstallSource, tc.want) + } + }) + } +} + +// TestChromium_ExcludedAndNonMemberEntries covers everything that is in the +// preference map and not an installed extension. None of them may degrade the +// browser: they are classification, not damage — and a bookkeeping stub that +// degraded it would do so on every scan for ever. +func TestChromium_ExcludedAndNonMemberEntries(t *testing.T) { + tests := []struct { + name string + id string + record string + reasons string + }{ + { + name: "the browser's own component", + id: idA, + record: `"location": 5, "manifest": {"name": "Component", "version": "1.0"}`, + }, + { + name: "the browser's own external component", + id: idA, + record: `"location": 10, "manifest": {"name": "Component", "version": "1.0"}`, + }, + { + name: "a theme", + id: idA, + record: `"location": 1, "manifest": {"name": "Dark", "version": "1.0", "theme": {"colors": {}}}`, + }, + { + name: "a legacy packaged app", + id: idA, + record: `"location": 1, "manifest": {"name": "Notes", "version": "1.0", "app": {"launch": {}}}`, + }, + { + // Bookkeeping residue the browser's own loader cannot load. Reporting + // it would invent an extension and, having no manifest, would pin the + // browser to a degraded status permanently. + name: "an update-ping stub", + id: idA, + record: `"active_bit": true, "allowlist": {"state": 1}, "lastpingday": "13300000000000000"`, + }, + { + name: "a key that cannot be an extension id", + id: "not-an-extension-id", + record: `"location": 1, "manifest": {"name": "Example", "version": "1.0"}`, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := tempHome(t) + localState(t, chromeRoot(home), "Default") + securePrefs(t, chromeRoot(home), "Default", `"`+tc.id+`": {`+tc.record+`}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + if got := findingsFor(info, browserChrome); len(got) != 0 { + t.Errorf("findings = %d (%q), want none", len(got), got[0].Name) + } + got := coverageFor(t, info, browserChrome) + if got.Status != model.BrowserCoverageScanned || got.ReasonCode != "" { + t.Errorf("status = %q/%q, want the browser scanned and undegraded", got.Status, got.ReasonCode) + } + }) + } +} + +// TestChromium_CorruptRecordKeepsItsIdentity is the difference between the two +// families: here the map key is the identity, so a record whose value cannot be +// read still reports the extension. Dropping it under a status that claims a +// complete list would retire that extension's stored row. +func TestChromium_CorruptRecordKeepsItsIdentity(t *testing.T) { + got, coverage := chromeFinding(t, `"`+idA+`": "this is not a record"`) + + if got.ExtensionID != idA { + t.Errorf("extension_id = %q, want the map key", got.ExtensionID) + } + if got.Name != "" || got.Version != "" { + t.Errorf("name/version = %q/%q, want both empty on a record that could not be read", got.Name, got.Version) + } + if got.EnabledState != model.BrowserExtStateUnknown { + t.Errorf("enabled_state = %q, want unknown", got.EnabledState) + } + // A reader requires both store fields on this family, and "cannot tell" is a + // value rather than an omission. + if got.StoreListing != model.BrowserExtStoreListingUnknown || got.StoreViolation != model.BrowserExtStoreViolationUnknown { + t.Errorf("store fields = %q/%q, want both unknown", got.StoreListing, got.StoreViolation) + } + if coverage.Status != model.BrowserCoveragePartial || coverage.ReasonCode != model.BrowserExtReasonManifestUnavailable { + t.Errorf("status = %q/%q, want partial and the missing metadata named", + coverage.Status, coverage.ReasonCode) + } +} + +// TestChromium_StoreDisposition covers the pair this feature exists for: an +// extension the store has pulled while the machine still runs it. Absent, the +// answer is unknown and never listed — inferring that the store still carries it +// would turn a missing answer into a reassuring one. +func TestChromium_StoreDisposition(t *testing.T) { + tests := []struct { + name string + record string + listing string + violation string + state string + }{ + { + name: "listed and clean", + record: `"cws-info": {"is-live": true, "violation-type": 0}`, + listing: model.BrowserExtStoreListingListed, + violation: model.BrowserExtStoreViolationNone, + state: model.BrowserExtEnabled, + }, + { + name: "pulled for a policy violation", + record: `"disable_reasons": [512], "cws-info": {"is-live": false, "violation-type": 2}`, + listing: model.BrowserExtStoreListingDelisted, + violation: model.BrowserExtStoreViolationFlagged, + state: model.BrowserExtDisabled, + }, + { + // The worst case, and the only field that finds it: no longer in the + // store, still running, still holding its permissions. + name: "delisted and still enabled", + record: `"cws-info": {"is-live": false, "violation-type": 0}`, + listing: model.BrowserExtStoreListingDelisted, + violation: model.BrowserExtStoreViolationNone, + state: model.BrowserExtEnabled, + }, + { + name: "no store record at all", + record: `"location": 1`, + listing: model.BrowserExtStoreListingUnknown, + violation: model.BrowserExtStoreViolationUnknown, + state: model.BrowserExtEnabled, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": {"location": 1, `+tc.record+ + `, "manifest": {"name": "Example", "version": "1.0"}}`) + if got.StoreListing != tc.listing || got.StoreViolation != tc.violation { + t.Errorf("store fields = %q/%q, want %q/%q", + got.StoreListing, got.StoreViolation, tc.listing, tc.violation) + } + if got.EnabledState != tc.state { + t.Errorf("enabled_state = %q, want %q: the listing is independent of it", got.EnabledState, tc.state) + } + }) + } +} + +// TestChromium_StoreAttribution reduces the update server to a label. The URL +// itself never ships: a self-hosted one names internal infrastructure. +func TestChromium_StoreAttribution(t *testing.T) { + tests := []struct { + name string + record string + want string + }{ + { + name: "the public store's update server", + record: `"manifest": {"name": "Example", "update_url": "` + chromeWebStoreUpdateURL + `"}`, + want: model.BrowserExtStoreChromeWebStore, + }, + { + name: "the other vendor's store", + record: `"manifest": {"name": "Example", "update_url": "` + edgeAddonsUpdateURL + `?prod=edgechromium"}`, + want: model.BrowserExtStoreEdgeAddons, + }, + { + name: "somebody's own server", + record: `"manifest": {"name": "Example", "update_url": "https://updates.example.internal/crx"}`, + want: model.BrowserExtStoreSelfHosted, + }, + { + name: "no update server, but the store flag", + record: `"from_webstore": true, "manifest": {"name": "Example"}`, + want: model.BrowserExtStoreChromeWebStore, + }, + { + name: "nothing to attribute it by", + record: `"manifest": {"name": "Example"}`, + want: model.BrowserExtStoreUnknown, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": {"location": 1, `+tc.record+`}`) + if got.Store != tc.want { + t.Errorf("store = %q, want %q", got.Store, tc.want) + } + if strings.Contains(got.Store, "://") { + t.Errorf("store = %q, want a label rather than a URL", got.Store) + } + }) + } +} + +// TestChromium_Permissions covers the union that makes the list honest. Either +// grant record alone misreports what the extension can reach: the up-front set +// keeps patterns the user has since withheld, and the on-demand set is the only +// home for what they granted later. +func TestChromium_Permissions(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example", "version": "1.0"}, + "granted_permissions": { + "api": ["storage", "tabs"], + "explicit_host": ["https://example.internal/*"] + }, + "runtime_granted_permissions": { + "api": ["storage", "cookies"], + "scriptable_host": ["https://other.internal/*"] + } + }`) + + wantPerms := []string{"cookies", "storage", "tabs"} + if strings.Join(got.Permissions, ",") != strings.Join(wantPerms, ",") { + t.Errorf("permissions = %v, want %v deduplicated and sorted", got.Permissions, wantPerms) + } + wantHosts := []string{"https://example.internal/*", "https://other.internal/*"} + if strings.Join(got.HostPermissions, ",") != strings.Join(wantHosts, ",") { + t.Errorf("host_permissions = %v, want %v", got.HostPermissions, wantHosts) + } +} + +// TestChromium_OverlongFieldsByClass pins the difference between a string a person +// reads and a string something matches. A name is shortened; a permission is +// dropped, because a shortened grant is a different grant and showing an auditor a +// permission the extension never held is worse than showing one fewer. +func TestChromium_OverlongFieldsByClass(t *testing.T) { + t.Run("a name is shortened and the browser stays clean", func(t *testing.T) { + long := strings.Repeat("N", maxNameBytes+50) + got, coverage := chromeFinding(t, `"`+idA+`": {"location": 1, "manifest": {"name": "`+long+`", "version": "1.0"}}`) + if len(got.Name) > maxNameBytes { + t.Errorf("name is %d bytes, want at most %d", len(got.Name), maxNameBytes) + } + if coverage.Status != model.BrowserCoverageScanned { + t.Errorf("status = %q, want no status change for a display field", coverage.Status) + } + }) + + t.Run("an overlong host pattern is absent, not shortened", func(t *testing.T) { + long := "https://" + strings.Repeat("h", maxPermissionBytes) + ".internal/*" + got, coverage := chromeFinding(t, `"`+idA+`": { + "location": 1, "manifest": {"name": "Example", "version": "1.0"}, + "granted_permissions": {"explicit_host": ["`+long+`", "https://kept.internal/*"]} + }`) + want := []string{"https://kept.internal/*"} + if strings.Join(got.HostPermissions, ",") != strings.Join(want, ",") { + t.Errorf("host_permissions = %v, want exactly %v — a shortened pattern must never ship", + got.HostPermissions, want) + } + if coverage.Status != model.BrowserCoveragePartial || coverage.ReasonCode != model.BrowserExtReasonCapped { + t.Errorf("status = %q/%q, want partial and capped", coverage.Status, coverage.ReasonCode) + } + }) +} + +// TestChromium_ManifestFallbackAndLocalizedName covers the two reads beyond the +// preferences: the manifest on disk when the preference copy is missing, and the +// message table a localized name resolves through. +func TestChromium_ManifestFallbackAndLocalizedName(t *testing.T) { + t.Run("the manifest on disk fills in for a store install", func(t *testing.T) { + home := tempHome(t) + root := chromeRoot(home) + localState(t, root, "Default") + securePrefs(t, root, "Default", `"`+idA+`": {"location": 1, "path": "`+idA+`/1.2.3_0"}`) + writeFile(t, filepath.Join(root, "Default", "Extensions", idA, "1.2.3_0", "manifest.json"), + `{"name": "Example From Disk", "version": "1.2.3"}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserChrome) + if len(got) != 1 { + t.Fatalf("findings = %d, want one", len(got)) + } + if got[0].Name != "Example From Disk" || got[0].Version != "1.2.3" { + t.Errorf("name/version = %q/%q, want the values from the manifest on disk", got[0].Name, got[0].Version) + } + if c := coverageFor(t, info, browserChrome); c.Status != model.BrowserCoverageScanned { + t.Errorf("status = %q/%q, want scanned: nothing was missing", c.Status, c.ReasonCode) + } + }) + + t.Run("a localized name resolves through the declared locale", func(t *testing.T) { + home := tempHome(t) + root := chromeRoot(home) + localState(t, root, "Default") + securePrefs(t, root, "Default", `"`+idA+`": { + "location": 1, "path": "`+idA+`/1.0_0", + "manifest": {"name": "__MSG_extName__", "version": "1.0", "default_locale": "en-GB"} + }`) + // Directory names use underscores rather than a language tag's hyphen, + // and the browser compares message names without case. + writeFile(t, filepath.Join(root, "Default", "Extensions", idA, "1.0_0", "_locales", "en_GB", "messages.json"), + `{"EXTNAME": {"message": "Example Localized"}}`) + + got := findingsFor(scanHome(t, home), browserChrome) + if len(got) != 1 { + t.Fatalf("findings = %d, want one", len(got)) + } + if got[0].Name != "Example Localized" { + t.Errorf("name = %q, want the resolved message", got[0].Name) + } + }) + + t.Run("an unresolvable name keeps its placeholder", func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": {"location": 1, "path": "`+idA+`/1.0_0", + "manifest": {"name": "__MSG_extName__", "version": "1.0", "default_locale": "en"}}`) + if got.Name != "__MSG_extName__" { + t.Errorf("name = %q, want the placeholder kept rather than an empty name", got.Name) + } + }) + + // Both reads describe an extension the preference map has already listed. + // Neither can add or remove one, so a refusal costs the attribute and leaves + // membership — and every other extension in the browser — standing. + t.Run("a refused attribute read degrades rather than failing the browser", func(t *testing.T) { + tests := []struct { + name string + entry string + // Planted where the read expects a state file: something of that name + // which is not one. The read refuses it rather than describing + // whatever it is. + plant []string + }{ + { + name: "the manifest on disk", + entry: `"` + idA + `": {"location": 1, "path": "` + idA + `/1.0_0"}`, + plant: []string{"manifest.json", "anything"}, + }, + { + name: "the message table behind a localized name", + entry: `"` + idA + `": {"location": 1, "path": "` + idA + `/1.0_0", + "manifest": {"name": "__MSG_extName__", "version": "1.0", "default_locale": "en"}}`, + plant: []string{"_locales", "en", "messages.json", "anything"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := tempHome(t) + root := chromeRoot(home) + localState(t, root, "Default") + securePrefs(t, root, "Default", tc.entry) + mkdir(t, filepath.Join(append([]string{root, "Default", "Extensions", idA, "1.0_0"}, tc.plant...)...)) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserChrome) + if len(got) != 1 || got[0].ExtensionID != idA { + t.Fatalf("findings = %d, want the listed extension still reported", len(got)) + } + if c := coverageFor(t, info, browserChrome); c.Status != model.BrowserCoveragePartial { + t.Errorf("status = %q/%q, want partial: membership is still complete", c.Status, c.ReasonCode) + } + }) + } + }) +} + +// TestChromium_UnpackedContentIsNeverOpened is the rule with no exception: an +// unpacked extension's directory is an arbitrary user location, so its absolute +// path is never resolved and never read. The manifest planted there would supply +// a name if anything opened it. +func TestChromium_UnpackedContentIsNeverOpened(t *testing.T) { + home := tempHome(t) + unpacked := filepath.Join(home, "projects", "client-work", "ext") + writeFile(t, filepath.Join(unpacked, "manifest.json"), `{"name": "Example Never Read", "version": "9.9"}`) + + localState(t, chromeRoot(home), "Default") + securePrefs(t, chromeRoot(home), "Default", + `"`+idA+`": {"location": 4, "path": "`+filepath.ToSlash(unpacked)+`"}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserChrome) + if len(got) != 1 { + t.Fatalf("findings = %d, want the extension reported with what the preferences held", len(got)) + } + if got[0].Name != "" { + t.Errorf("name = %q, want none: the extension's own directory must never be opened", got[0].Name) + } + if got[0].InstallSource != model.BrowserExtInstallUnpacked { + t.Errorf("install_source = %q, want unpacked — the whole signal this row carries", got[0].InstallSource) + } + // No filesystem path of any kind reaches the wire, and this is the path that + // could have carried a client's name. + raw := marshalFindings(t, info) + if strings.Contains(raw, "client-work") { + t.Errorf("payload carries a filesystem path: %s", raw) + } + c := coverageFor(t, info, browserChrome) + if c.Status != model.BrowserCoveragePartial || c.ReasonCode != model.BrowserExtReasonManifestUnavailable { + t.Errorf("status = %q/%q, want partial and the missing metadata named", c.Status, c.ReasonCode) + } +} + +// TestChromium_ByteOrderMarkedPrefsStillParse covers the class of failure a +// Windows-authored file introduces: a mark in front of the document makes every +// parser reject the whole thing, and a profile full of extensions would report as +// empty. +func TestChromium_ByteOrderMarkedPrefsStillParse(t *testing.T) { + home := tempHome(t) + root := chromeRoot(home) + localState(t, root, "Default") + const bom = "\uFEFF" + writeFile(t, filepath.Join(root, "Default", "Secure Preferences"), + bom+`{"extensions": {"settings": {"`+idA+`": {"location": 1, "manifest": {"name": "Example", "version": "1.0"}}}}}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + if got := findingsFor(info, browserChrome); len(got) != 1 { + t.Fatalf("findings = %d, want the byte order mark stripped and the file parsed", len(got)) + } +} + +// TestChromium_TwoByteEncodingIsReportedRatherThanParsed covers the other +// encoding. A byte-oriented parser reads almost nothing from it rather than +// failing, so the file would read as holding no extensions. +func TestChromium_TwoByteEncodingIsReportedRatherThanParsed(t *testing.T) { + home := tempHome(t) + root := chromeRoot(home) + localState(t, root, "Default") + if err := os.MkdirAll(filepath.Join(root, "Default"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(root, "Default", "Secure Preferences"), + []byte{0xFF, 0xFE, 0x7B, 0x00}, 0o644); err != nil { + t.Fatalf("write: %v", err) + } + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserChrome) + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonUnsupportedEncoding { + t.Errorf("status = %q/%q, want failed and the encoding named", got.Status, got.ReasonCode) + } +} + +// TestChromium_PreinstalledIsReportedAndNotHidden keeps a default-installed +// extension in the inventory. It is a real extension with real permissions; the +// flag is there so a console can de-emphasize it rather than so this can drop it. +func TestChromium_PreinstalledIsReportedAndNotHidden(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": { + "location": 6, "was_installed_by_default": true, + "manifest": {"name": "Example Bundled", "version": "1.0"} + }`) + if !got.Preinstalled { + t.Error("preinstalled = false, want the default-install flag reported") + } + if got.InstallSource != model.BrowserExtInstallSideload { + t.Errorf("install_source = %q, want the location's own answer rather than the flag's", got.InstallSource) + } +} diff --git a/internal/detector/browserext/detector.go b/internal/detector/browserext/detector.go new file mode 100644 index 00000000..d1982571 --- /dev/null +++ b/internal/detector/browserext/detector.go @@ -0,0 +1,584 @@ +package browserext + +import ( + "bytes" + "context" + "os" + "os/user" + "path/filepath" + "sort" + "strings" + "time" + "unicode/utf8" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/safepath" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +// Detector inventories the extensions installed in this machine's browsers. It +// reads the browsers' own state files and nothing else: no browser is launched, +// no store is asked what it published, and no extension's code is opened. +type Detector struct { + exec executor.Executor + skipper *tcc.Skipper + + // serviceSession reports whether this process runs in a context that has no + // interactive user behind it. A function field because the answer comes from + // the process itself rather than from any value a caller can pass, and a test + // has to be able to state it. + serviceSession func() bool +} + +// New builds a detector. +func New(exec executor.Executor) *Detector { + return &Detector{exec: exec, serviceSession: serviceSession} +} + +// WithSkipper attaches the consent guard. A nil skipper is a no-op. +func (d *Detector) WithSkipper(s *tcc.Skipper) *Detector { + d.skipper = s + return d +} + +// Detect returns the inventory for one run, for the account named by target. +// +// It returns nil — the "did not run" sentinel — when there is no interactive +// account to describe, and that refusal is load-bearing rather than tidy. Reading +// a service account's home would find no browser at all and report every browser +// missing, which is an authoritative answer: a reader would honour it by deleting +// the device's real inventory. Never an error, and never a panic: a browser that +// could not be read is a coverage status, and a per-extension failure degrades +// one finding. +func (d *Detector) Detect(ctx context.Context, target *user.User) (info *model.BrowserExtensionScanInfo) { + home, ok := d.resolveTarget(target) + if !ok { + return nil + } + + started := time.Now() + info = &model.BrowserExtensionScanInfo{ + PayloadSchemaVersion: model.CurrentBrowserExtensionSchemaVersion, + CatalogVersion: catalogVersion, + ScanComplete: true, + Browsers: []model.BrowserCoverage{}, + Findings: []model.BrowserExtensionFinding{}, + } + defer func() { + // The one sanctioned recover. Each browser's coverage entry and its + // findings are committed together, so what survives a panic is a payload + // describing the browsers that finished — and a browser missing from the + // coverage list was never claimed, so a reader leaves its rows alone. + // That is why nothing here has to mark the result incomplete: the section + // says less than it would have, not something untrue. + _ = recover() + info.CollectedAt = time.Now().Unix() + info.DurationMs = time.Since(started).Milliseconds() + }() + + // The detector's own deadline, inside the phase budget. It bounds the work + // this phase chooses to take on; it cannot interrupt a syscall, which is why + // every read is built so that it cannot block. + ctx, cancel := context.WithTimeout(ctx, browserExtPhaseBudget) + defer cancel() + + platform := d.exec.GOOS() + scan := &scanState{ + info: info, + platform: platform, + // Every path is opened through descriptors that follow no symlink. A + // link anywhere on the path refuses, which costs a visible gap in one + // scan; following one could read somewhere this has no business being, + // and skipping it quietly under an authoritative status could delete a + // real extension's stored row. + resolver: safepath.NewNoFollow(home, d.consentGuard(platform, home)), + } + + for _, spec := range catalog { + roots := spec.roots(platform, home) + if len(roots) == 0 { + // Not attempted is silence: a browser this platform's catalog does + // not carry appears nowhere rather than as an absence. + continue + } + if scan.stopped != "" { + // A cap or the deadline already bounded the payload. Every remaining + // browser reports the same cause and ships nothing, so its stored + // rows survive. + scan.commit(spec.ID, browserResult{status: model.BrowserCoverageFailed, reason: scan.stopped}) + continue + } + scan.commit(spec.ID, d.scanBrowser(ctx, scan, spec, roots)) + } + return info +} + +// resolveTarget establishes whose browsers this run describes, and refuses every +// identity that is not a developer at a keyboard. Nothing comes from the +// environment: the agent commonly runs as a system account, so an inherited home +// names the service profile. +func (d *Detector) resolveTarget(target *user.User) (string, bool) { + if target == nil || target.Username == "" { + return "", false + } + if d.serviceSession != nil && d.serviceSession() { + // Windows carries the answer in the process rather than in the account: + // a service running under a custom or domain account has an ordinary SID + // that no allowlist can enumerate, while its session is always 0. + return "", false + } + if isServiceIdentity(d.exec.GOOS(), target) { + return "", false + } + home := target.HomeDir + if home == "" { + resolved, err := d.exec.HomeDir(target.Username) + if err != nil || resolved == "" { + return "", false + } + home = resolved + } + return home, true +} + +// Well-known Windows service accounts. A backstop only — the session check above +// is the predicate that catches a service under a custom account, and localized +// account names make a name comparison a sieve in either direction. +var windowsServiceSIDs = map[string]bool{ + "S-1-5-18": true, // LocalSystem + "S-1-5-19": true, // LocalService + "S-1-5-20": true, // NetworkService +} + +// isServiceIdentity reports whether the resolved account is a machine identity +// rather than a person. Compared by identity, never by name: a renamed UID-0 +// account is still root. +func isServiceIdentity(platform string, u *user.User) bool { + if platform == model.PlatformWindows { + return windowsServiceSIDs[strings.ToUpper(u.Uid)] + } + return u.Uid == "0" +} + +// consentGuard is what the resolver asks before it touches a path, answering in +// this phase's own reason code so a refusal reads like every other one. +// +// The macOS skipper declines ~/Library wholesale, which is right for a walk and +// wrong for this detector: three of the four browsers keep their data directory +// under it. So the browsers' own directories are exempt, along with the +// directories above them that a descent has to pass through — matched against the +// cleaned path, so "Library/Application Support/Google/Chrome/../Mail" cannot ride +// the exemption. What is left protected is the one path class this detector does +// not fix itself: a profile directory named by a browser's own config file, which +// is a string an attacker can write and must be refused rather than touched. +func (d *Detector) consentGuard(platform, home string) safepath.Guard { + if d.skipper == nil { + return nil + } + var exempt []string + for _, spec := range catalog { + exempt = append(exempt, spec.roots(platform, home)...) + } + return func(path string) string { + cleaned := filepath.Clean(path) + for _, ex := range exempt { + if atOrUnder(cleaned, ex) || atOrUnder(ex, cleaned) { + return "" + } + } + if d.skipper.WithinProtected(cleaned) { + return model.BrowserExtReasonRefusedTCC + } + return "" + } +} + +// atOrUnder reports whether path is dir or sits under it, compared on a +// separator boundary so a sibling directory cannot pass for a nested one. +func atOrUnder(path, dir string) bool { + if path == dir { + return true + } + return strings.HasPrefix(path, dir) && len(path) > len(dir) && path[len(dir)] == filepath.Separator +} + +// scanState carries what every browser's scan needs, and the two run-wide facts: +// the payload being built and whether a bound has already cut it short. +type scanState struct { + info *model.BrowserExtensionScanInfo + platform string + resolver *safepath.Resolver + + // stopped is the reason every browser after this point reports. Set when a + // cap or the deadline bounds the whole payload rather than one browser. + stopped string +} + +// browserResult is one browser's whole contribution: its coverage status and the +// findings that status vouches for. +type browserResult struct { + status string + reason string + profiles int + findings []model.BrowserExtensionFinding +} + +// commit records one browser's coverage entry and its findings together. They are +// never appended separately: a finding whose browser has no coverage entry is a +// payload a reader rejects whole, so the two have to move as one. +func (s *scanState) commit(browserID string, r browserResult) { + if r.status == model.BrowserCoverageFailed || r.status == model.BrowserCoverageNotPresent { + // A browser whose membership is not known complete ships nothing. Half a + // list under an authoritative status would retire the extensions it left + // out, which is the one outcome this design exists to prevent. + r.findings = nil + } + if len(s.info.Findings)+len(r.findings) > maxExtensionsTotal { + // The cap cuts at a browser boundary. This browser and every later one + // report the cause and ship nothing. + s.stop(model.BrowserExtReasonCapped, model.BrowserExtTruncatedFindingCap) + r = browserResult{status: model.BrowserCoverageFailed, reason: model.BrowserExtReasonCapped, profiles: r.profiles} + } + if r.status == model.BrowserCoverageFailed { + s.info.ScanComplete = false + } + s.info.Browsers = append(s.info.Browsers, model.BrowserCoverage{ + BrowserID: browserID, + Status: r.status, + ReasonCode: r.reason, + ProfileCount: r.profiles, + ExtensionCount: len(r.findings), + }) + s.info.Findings = append(s.info.Findings, r.findings...) +} + +// truncate records that a bound cut something out of the payload. Truncation and +// incompleteness travel together: the two fields describe one event and a reader +// validates the pair in both directions. The first cause is kept, because the +// bound that hit first is the one that shaped the result. +func (s *scanState) truncate(truncatedReason string) { + s.info.Truncated = true + if s.info.TruncatedReason == "" { + s.info.TruncatedReason = truncatedReason + } + s.info.ScanComplete = false +} + +// stop is a bound that ends the run rather than one browser: the total cap and +// the deadline apply to the payload as a whole, so every browser after this point +// reports the same cause and ships nothing. A bound on one browser's own list +// calls truncate instead, which leaves the browsers after it to be scanned +// normally — a machine with twenty-one profiles in one browser still has a +// readable answer for the other three. +func (s *scanState) stop(reason, truncatedReason string) { + s.stopped = reason + s.truncate(truncatedReason) +} + +// scanBrowser runs one browser across every data directory it has on this +// platform and reduces the result to one coverage status. +// +// The union of two directories that both exist (a native install and a snap) is +// the answer, which is why a failure in any existing one fails the whole browser: +// half a union that read as complete would retire the missing half's rows. +func (d *Detector) scanBrowser(ctx context.Context, scan *scanState, spec browserSpec, roots []string) browserResult { + b := &browserScan{occurrences: map[string][]occurrence{}} + existing := 0 + for _, root := range roots { + if ctx.Err() != nil { + b.failPayload(model.BrowserExtReasonTimedOut, model.BrowserExtTruncatedDeadline) + } else if d.scanRoot(ctx, scan, spec, root, b) { + existing++ + } + if b.failure != "" { + switch { + case b.payloadWide: + scan.stop(b.failure, b.truncatedReason) + case b.truncatedReason != "": + scan.truncate(b.truncatedReason) + } + return browserResult{status: model.BrowserCoverageFailed, reason: b.failure, profiles: b.profiles} + } + } + if existing == 0 { + // No data directory for this browser, or one holding no installation. + // Authoritative, and safely so: the only rows it can retire are rows a + // previous scan of these same directories wrote. + return browserResult{status: model.BrowserCoverageNotPresent} + } + status := model.BrowserCoverageScanned + if b.degraded != "" { + // Membership is still complete; an attribute is not. + status = model.BrowserCoveragePartial + } + return browserResult{status: status, reason: b.degraded, profiles: b.profiles, findings: b.fold(spec.ID)} +} + +// scanRoot dispatches one data directory to its engine's parser and reports +// whether the directory held an installation. +func (d *Detector) scanRoot(ctx context.Context, scan *scanState, spec browserSpec, root string, b *browserScan) bool { + if spec.Engine == engineGecko { + return d.scanGeckoRoot(ctx, scan, root, b) + } + return d.scanChromiumRoot(ctx, scan, root, b) +} + +// occurrence is one extension as one profile recorded it. Profiles never reach +// the wire — their names are user-chosen text and per-profile state was not the +// ask — so they exist only to drive enumeration and this reduction. +type occurrence struct { + // sortKey orders the occurrences of one extension: the data directory and + // then the profile directory. Arbitrary but fixed, which is what makes two + // runs over an unchanged machine emit identical findings. + sortKey string + enabled string + disabledBy string + // block is taken whole from the winning occurrence. Never merged field by + // field: pairing one profile's version with another's permission set would + // describe an extension that exists nowhere. + block model.BrowserExtensionFinding +} + +// browserScan accumulates one browser's occurrences and the first thing that went +// wrong with it. +type browserScan struct { + occurrences map[string][]occurrence + profiles int + + // degraded is the headline reason for a partial status: an attribute this + // scan could not recover. The first cause wins — one reason per browser is + // what a reader is given, and a list of them is not read by anything. + degraded string + + // failure is the headline reason for a failed status. Set once, and it stops + // this browser: after it, nothing more is claimed about the browser. + failure string + // truncatedReason is set when failure is a bound being reached rather than a + // document that could not be read, so the payload says it was cut. + truncatedReason string + // payloadWide separates a bound that ends the run from one that ends this + // browser: the deadline is shared by everything after it, while a cap on this + // browser's own list says nothing about the next browser. + payloadWide bool +} + +func (b *browserScan) degrade(reason string) { + if b.degraded == "" { + b.degraded = reason + } +} + +// fail records a browser-local failure: this browser's membership is not known +// complete, and the rest of the scan carries on. +func (b *browserScan) fail(reason string) { + if b.failure == "" { + b.failure = reason + } +} + +// failBounded records that a bound on this browser's own list was reached. The +// browser's membership is not known complete, and the payload says it was cut — +// but the browsers after it are unaffected and are scanned as usual. +func (b *browserScan) failBounded(reason, truncatedReason string) { + if b.failure == "" { + b.failure = reason + b.truncatedReason = truncatedReason + } +} + +// failPayload records a bound that ends the run — the deadline — so every later +// browser reports it too. +func (b *browserScan) failPayload(reason, truncatedReason string) { + if b.failure == "" { + b.failBounded(reason, truncatedReason) + b.payloadWide = true + } +} + +// add records one occurrence, reporting whether the browser may continue. The +// per-browser cap fails the browser rather than shortening its list, because the +// list is read as the complete set. +func (b *browserScan) add(id string, occ occurrence) bool { + if _, seen := b.occurrences[id]; !seen && len(b.occurrences) >= maxExtensionsPerBrowser { + b.failBounded(model.BrowserExtReasonCapped, model.BrowserExtTruncatedFindingCap) + return false + } + b.occurrences[id] = append(b.occurrences[id], occ) + return true +} + +// fold reduces every extension's occurrences to one finding. +func (b *browserScan) fold(browserID string) []model.BrowserExtensionFinding { + ids := make([]string, 0, len(b.occurrences)) + for id := range b.occurrences { + ids = append(ids, id) + } + sort.Strings(ids) + + out := make([]model.BrowserExtensionFinding, 0, len(ids)) + for _, id := range ids { + occs := b.occurrences[id] + sort.SliceStable(occs, func(i, j int) bool { return occs[i].sortKey < occs[j].sortKey }) + + f := occs[0].block + f.BrowserID = browserID + f.ExtensionID = id + // Enabled in any one profile means the extension can run on this + // machine, which is the question being asked. + f.EnabledState = model.BrowserExtStateUnknown + for _, occ := range occs { + if occ.enabled == model.BrowserExtEnabled { + f.EnabledState = model.BrowserExtEnabled + break + } + if occ.enabled == model.BrowserExtDisabled { + f.EnabledState = model.BrowserExtDisabled + } + } + // The cause follows the resolved state, not the winning occurrence, + // which may well be an enabled one: reading it off that occurrence would + // attach an enabled profile's empty cause to a disabled row. + f.DisabledBy = "" + if f.EnabledState == model.BrowserExtDisabled { + f.DisabledBy = model.BrowserExtDisabledByUnknown + for _, occ := range occs { + if occ.enabled == model.BrowserExtDisabled && occ.disabledBy != "" { + f.DisabledBy = occ.disabledBy + break + } + } + } + if f.Permissions == nil { + f.Permissions = []string{} + } + if f.HostPermissions == nil { + f.HostPermissions = []string{} + } + out = append(out, f) + } + return out +} + +// readState reads one of a browser's state files through the no-follow resolver. +// +// A file that is simply absent is reported as missing rather than as a failure: a +// browser that has never had a second profile has no second profile's +// preferences, and that is not a problem to report. Everything else comes back as +// a reason code, because a decoder's or a library's own message quotes the +// document it choked on and these documents are the browser's private state. +func (s *scanState) readState(path string, limit int64) (data []byte, missing bool, reason string) { + raw, _, info, truncated, err := s.resolver.Read(path, limit) + switch { + case err == nil: + case os.IsNotExist(err): + return nil, true, "" + default: + return nil, false, refusalReason(err) + } + if !info.Mode().IsRegular() { + // A directory, device or FIFO where a state file belongs. The open + // already refused to block on it; reading it would describe something + // other than the browser's state. + return nil, false, model.BrowserExtReasonParseError + } + if truncated { + // The file outgrew its cap. Its prefix is not a shorter version of the + // document: it either fails to parse for the wrong reason or, worse, + // parses and reports a fraction of the extensions as the whole set. + return nil, false, model.BrowserExtReasonCapped + } + if hasUTF16BOM(raw) { + // These parsers are byte-oriented, so a two-byte encoding decodes to + // almost nothing rather than failing, and a profile full of extensions + // would read as empty. + return nil, false, model.BrowserExtReasonUnsupportedEncoding + } + return stripUTF8BOM(raw), false, "" +} + +// listNames returns at most limit immediate entries of a directory. Names only, +// so nothing can accidentally descend, and the bound is applied at the read: +// these directories are exactly the ones a local process can fill. +func (s *scanState) listNames(path string, limit int) (names []string, missing bool, reason string) { + entries, _, more, err := s.resolver.ReadDirNames(path, limit) + switch { + case err == nil: + case os.IsNotExist(err): + return nil, true, "" + default: + return nil, false, refusalReason(err) + } + if more { + return nil, false, model.BrowserExtReasonCapped + } + sort.Strings(entries) + return entries, false, "" +} + +// statEntry reports what a discovered path is without opening it, so a file +// found where a profile directory was expected can be skipped rather than read. +func (s *scanState) statEntry(path string) (isDir, missing bool, reason string) { + _, info, err := s.resolver.Stat(path) + switch { + case err == nil: + case os.IsNotExist(err): + return false, true, "" + default: + return false, false, refusalReason(err) + } + return info.IsDir(), false, "" +} + +// refusalReason maps a refused read to one of this phase's codes. Never a +// library's own message: those quote the input they choked on. +func refusalReason(err error) string { + switch safepath.ReasonOf(err) { + case safepath.ReasonSymlink: + return model.BrowserExtReasonSymlinkRejected + case safepath.ReasonDenied: + return model.BrowserExtReasonPermissionDenied + case model.BrowserExtReasonRefusedTCC: + // The consent guard's own answer, travelling back as the refusal. + return model.BrowserExtReasonRefusedTCC + case safepath.ReasonOutsideRoots: + // A location outside the account's own tree, named by a config file + // rather than by the catalog. Reported as a refusal rather than as a + // permission error because nothing was ever asked of the filesystem. + return model.BrowserExtReasonRefusedTCC + } + if os.IsPermission(err) { + return model.BrowserExtReasonPermissionDenied + } + return model.BrowserExtReasonParseError +} + +// hasUTF16BOM reports whether data starts with a UTF-16 byte order mark. +func hasUTF16BOM(data []byte) bool { + if len(data) < 2 { + return false + } + return (data[0] == 0xFF && data[1] == 0xFE) || (data[0] == 0xFE && data[1] == 0xFF) +} + +// stripUTF8BOM removes a leading UTF-8 byte order mark. Windows-authored state +// files carry them, and a BOM in front of a JSON document makes every parser +// reject the whole thing — a profile full of extensions would report as empty. +func stripUTF8BOM(data []byte) []byte { + return bytes.TrimPrefix(data, []byte{0xEF, 0xBB, 0xBF}) +} + +// capBytes shortens a display string to a byte budget at a rune boundary, so a +// capped value is still valid UTF-8. Only for the fields a human reads: an +// identity and a permission are matched rather than read, and a shortened one is +// a different value. +func capBytes(s string, limit int) string { + if len(s) <= limit { + return s + } + cut := limit + for cut > 0 && !utf8.RuneStart(s[cut]) { + cut-- + } + return s[:cut] +} diff --git a/internal/detector/browserext/detector_test.go b/internal/detector/browserext/detector_test.go new file mode 100644 index 00000000..de8d3800 --- /dev/null +++ b/internal/detector/browserext/detector_test.go @@ -0,0 +1,658 @@ +package browserext + +import ( + "context" + "encoding/json" + "os" + "os/user" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +// The fixtures run as the Linux catalog because its paths are the shortest; the +// platform only selects which catalog rows apply, so the parsers under test are +// the same ones every platform runs. + +// tempHome returns a home directory with every symlink already resolved. The +// detector refuses a path with a link anywhere on it, and the system temporary +// directory is reached through one on macOS. +func tempHome(t *testing.T) string { + t.Helper() + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatalf("resolve temp dir: %v", err) + } + return home +} + +func testUser(home string) *user.User { + return &user.User{Username: "dev", Uid: "501", HomeDir: home} +} + +// newDetector builds a detector for a platform, with the session probe answering +// "there is an interactive user" so a test host's own context cannot decide it. +func newDetector(platform string) *Detector { + mock := executor.NewMock() + mock.SetGOOS(platform) + d := New(mock) + d.serviceSession = func() bool { return false } + return d +} + +// scanHome runs one scan over a prepared home. +func scanHome(t *testing.T, home string) *model.BrowserExtensionScanInfo { + t.Helper() + info := newDetector(model.PlatformLinux).Detect(context.Background(), testUser(home)) + if info == nil { + t.Fatal("Detect returned the did-not-run sentinel for a resolved user") + } + return info +} + +func chromeRoot(home string) string { return filepath.Join(home, ".config", "google-chrome") } +func firefoxRoot(home string) string { return filepath.Join(home, ".mozilla", "firefox") } + +func writeFile(t *testing.T, path, body string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +func mkdir(t *testing.T, path string) { + t.Helper() + if err := os.MkdirAll(path, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", path, err) + } +} + +// localState writes the file the profile list comes from. +func localState(t *testing.T, root string, profiles ...string) { + t.Helper() + quoted := make([]string, 0, len(profiles)) + for _, p := range profiles { + // The values carry the profile label and the signed-in account's e-mail + // address in real life; the fixture keeps them so the test proves they + // are never read. + quoted = append(quoted, `"`+p+`": {"name": "Person 1", "user_name": "dev@example.internal"}`) + } + writeFile(t, filepath.Join(root, "Local State"), + `{"profile": {"info_cache": {`+strings.Join(quoted, ",")+`}}}`) +} + +// securePrefs writes one profile's authoritative extension map. +func securePrefs(t *testing.T, root, profile, settings string) { + t.Helper() + writeFile(t, filepath.Join(root, profile, "Secure Preferences"), + `{"extensions": {"settings": {`+settings+`}}}`) +} + +func coverageFor(t *testing.T, info *model.BrowserExtensionScanInfo, browserID string) model.BrowserCoverage { + t.Helper() + for _, b := range info.Browsers { + if b.BrowserID == browserID { + return b + } + } + t.Fatalf("no coverage entry for %s", browserID) + return model.BrowserCoverage{} +} + +func findingsFor(info *model.BrowserExtensionScanInfo, browserID string) []model.BrowserExtensionFinding { + var out []model.BrowserExtensionFinding + for _, f := range info.Findings { + if f.BrowserID == browserID { + out = append(out, f) + } + } + return out +} + +// assertPayloadInvariants checks the rules a reader rejects a whole payload over. +// Every scan in this suite goes through it: a fixture that violates one would be +// accepted here and refused on arrival. +func assertPayloadInvariants(t *testing.T, info *model.BrowserExtensionScanInfo) { + t.Helper() + if info.PayloadSchemaVersion != model.CurrentBrowserExtensionSchemaVersion { + t.Errorf("payload_schema_version = %d", info.PayloadSchemaVersion) + } + if info.CatalogVersion == "" { + t.Error("catalog_version is empty") + } + if info.CollectedAt <= 0 { + t.Error("collected_at is not set") + } + if info.Truncated != (info.TruncatedReason != "") { + t.Errorf("truncated = %v with reason %q", info.Truncated, info.TruncatedReason) + } + + failed := false + seen := map[string]bool{} + counts := map[string]int{} + for _, f := range info.Findings { + counts[f.BrowserID]++ + if (f.EnabledState == model.BrowserExtDisabled) != (f.DisabledBy != "") { + t.Errorf("%s: enabled_state %q with disabled_by %q", f.ExtensionID, f.EnabledState, f.DisabledBy) + } + if f.ExtensionID == "" { + t.Error("finding with no identity") + } + } + for _, b := range info.Browsers { + if seen[b.BrowserID] { + t.Errorf("%s: duplicate coverage entry", b.BrowserID) + } + seen[b.BrowserID] = true + switch b.Status { + case model.BrowserCoveragePartial, model.BrowserCoverageFailed: + if b.ReasonCode == "" { + t.Errorf("%s: status %q with no reason_code", b.BrowserID, b.Status) + } + default: + if b.ReasonCode != "" { + t.Errorf("%s: status %q with reason_code %q", b.BrowserID, b.Status, b.ReasonCode) + } + } + if b.Status == model.BrowserCoverageFailed { + failed = true + } + if b.Status == model.BrowserCoverageFailed || b.Status == model.BrowserCoverageNotPresent { + if b.ExtensionCount != 0 || counts[b.BrowserID] != 0 { + t.Errorf("%s: status %q ships %d findings", b.BrowserID, b.Status, counts[b.BrowserID]) + } + } else if b.ExtensionCount != counts[b.BrowserID] { + t.Errorf("%s: extension_count = %d, findings = %d", b.BrowserID, b.ExtensionCount, counts[b.BrowserID]) + } + if b.ProfileCount < 0 || b.ProfileCount > maxProfilesPerBrowser { + t.Errorf("%s: profile_count = %d", b.BrowserID, b.ProfileCount) + } + } + for _, f := range info.Findings { + if !seen[f.BrowserID] { + t.Errorf("%s: finding for a browser with no coverage entry", f.BrowserID) + } + } + if want := !(failed || info.Truncated); info.ScanComplete != want { + t.Errorf("scan_complete = %v, want %v (failed=%v truncated=%v)", + info.ScanComplete, want, failed, info.Truncated) + } +} + +// TestDetect_DeclinesWithoutADeveloper is the wipe guard. A scan of an account +// with no browsers finds none and says so authoritatively, so scanning the wrong +// account would tell a reader this machine has no extensions — and it would act +// on that. The refusal has to happen before any path is built. +func TestDetect_DeclinesWithoutADeveloper(t *testing.T) { + home := tempHome(t) + // A real installation, so a scan that ran would certainly report findings. + localState(t, chromeRoot(home), "Default") + securePrefs(t, chromeRoot(home), "Default", `"`+idA+`": {"location": 1, "manifest": {"name": "Example", "version": "1.0"}}`) + + tests := []struct { + name string + platform string + target *user.User + session0 bool + }{ + {name: "no user resolved", platform: model.PlatformLinux, target: nil}, + {name: "empty username", platform: model.PlatformLinux, target: &user.User{HomeDir: home}}, + {name: "root", platform: model.PlatformLinux, target: &user.User{Username: "root", Uid: "0", HomeDir: home}}, + { + name: "windows local system", + platform: model.PlatformWindows, + target: &user.User{Username: "SYSTEM", Uid: "S-1-5-18", HomeDir: home}, + }, + { + // The predicate that catches a service under a custom account, whose + // SID no allowlist can enumerate. + name: "windows service session with an ordinary account", + platform: model.PlatformWindows, + target: &user.User{Username: "svc-scanner", Uid: "S-1-5-21-1-2-3-1104", HomeDir: home}, + session0: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + d := newDetector(tc.platform) + d.serviceSession = func() bool { return tc.session0 } + if info := d.Detect(context.Background(), tc.target); info != nil { + t.Errorf("Detect returned a %d-browser section, want the did-not-run sentinel", len(info.Browsers)) + } + }) + } +} + +// TestDetect_CleanMachineIsAnAuthoritativeEmpty pins the case an earlier +// two-directional reading of the invariants got wrong: zero findings is the +// normal result for a machine with no browsers, and it must not read as a +// failure. +func TestDetect_CleanMachineIsAnAuthoritativeEmpty(t *testing.T) { + info := scanHome(t, tempHome(t)) + assertPayloadInvariants(t, info) + + if !info.ScanComplete || info.Truncated { + t.Errorf("scan_complete = %v truncated = %v, want a complete untruncated scan", info.ScanComplete, info.Truncated) + } + if len(info.Findings) != 0 { + t.Errorf("findings = %d, want none", len(info.Findings)) + } + if len(info.Browsers) != len(catalog) { + t.Errorf("browsers = %d, want one entry per catalog browser (%d)", len(info.Browsers), len(catalog)) + } + for _, b := range info.Browsers { + if b.Status != model.BrowserCoverageNotPresent { + t.Errorf("%s: status = %q, want %q", b.BrowserID, b.Status, model.BrowserCoverageNotPresent) + } + } +} + +// TestDetect_ExistingRootWithNoInstallation separates the two ways a directory +// can hold no extensions. Installers leave a data directory behind holding +// nothing; calling that a failure paints a permanent red row for a browser nobody +// installed. A directory with a profile but no profile list is the opposite case +// and must stay a failure — the two must not collapse into one answer. +func TestDetect_ExistingRootWithNoInstallation(t *testing.T) { + t.Run("leftover directory is not present", func(t *testing.T) { + home := tempHome(t) + mkdir(t, filepath.Join(chromeRoot(home), "NativeMessagingHosts")) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserChrome) + if got.Status != model.BrowserCoverageNotPresent || got.ReasonCode != "" { + t.Errorf("status = %q/%q, want %q with no reason", got.Status, got.ReasonCode, model.BrowserCoverageNotPresent) + } + }) + + t.Run("profile with an unreadable profile list fails", func(t *testing.T) { + home := tempHome(t) + mkdir(t, filepath.Join(chromeRoot(home), "Default")) + writeFile(t, filepath.Join(chromeRoot(home), "Local State"), `{"profile": {`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserChrome) + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonParseError { + t.Errorf("status = %q/%q, want %q/%q", got.Status, got.ReasonCode, + model.BrowserCoverageFailed, model.BrowserExtReasonParseError) + } + }) +} + +// TestDetect_SymlinksRefuse covers the rule that has no exceptions: a link +// anywhere on a path refuses, above or below the data directory, and the browser +// fails rather than reporting an absence. Failing retains the browser's stored +// rows; reporting an absence would delete them, and following the link would read +// somewhere this has no business being. +func TestDetect_SymlinksRefuse(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation needs a privilege the test host may not hold") + } + settings := `"` + idA + `": {"location": 1, "manifest": {"name": "Example Elsewhere", "version": "1.0"}}` + + tests := []struct { + name string + // setup prepares a home whose chrome data directory is reached through a + // link, and returns the directory a read must never enter. + setup func(t *testing.T, home string) string + }{ + { + name: "the data directory itself is a link", + setup: func(t *testing.T, home string) string { + elsewhere := filepath.Join(home, "elsewhere") + localState(t, elsewhere, "Default") + securePrefs(t, elsewhere, "Default", settings) + mkdir(t, filepath.Dir(chromeRoot(home))) + if err := os.Symlink(elsewhere, chromeRoot(home)); err != nil { + t.Fatalf("symlink: %v", err) + } + return elsewhere + }, + }, + { + name: "a profile directory below it is a link", + setup: func(t *testing.T, home string) string { + elsewhere := filepath.Join(home, "elsewhere") + securePrefs(t, elsewhere, "Default", settings) + localState(t, chromeRoot(home), "Default") + if err := os.Symlink(filepath.Join(elsewhere, "Default"), filepath.Join(chromeRoot(home), "Default")); err != nil { + t.Fatalf("symlink: %v", err) + } + return elsewhere + }, + }, + { + name: "a state file is a link", + setup: func(t *testing.T, home string) string { + elsewhere := filepath.Join(home, "elsewhere") + writeFile(t, filepath.Join(elsewhere, "Local State"), `{"profile": {"info_cache": {"Default": {}}}}`) + mkdir(t, chromeRoot(home)) + if err := os.Symlink(filepath.Join(elsewhere, "Local State"), filepath.Join(chromeRoot(home), "Local State")); err != nil { + t.Fatalf("symlink: %v", err) + } + return elsewhere + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := tempHome(t) + tc.setup(t, home) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserChrome) + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonSymlinkRejected { + t.Errorf("status = %q/%q, want %q/%q", got.Status, got.ReasonCode, + model.BrowserCoverageFailed, model.BrowserExtReasonSymlinkRejected) + } + // Nothing behind the link may appear in the payload: a refusal that + // still read the target would be no refusal at all. + for _, f := range info.Findings { + if strings.Contains(f.Name, "Elsewhere") { + t.Errorf("read through the link: %q", f.Name) + } + } + }) + } +} + +// TestDetect_NonRegularStateFileFails plants a pipe where a state file belongs. +// Opening one without asking not to block would hang the phase for ever, and no +// deadline interrupts a blocked open — so the test's own completion is half the +// assertion. +func TestDetect_NonRegularStateFileFails(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no in-directory pipes on this platform") + } + home := tempHome(t) + mkdir(t, chromeRoot(home)) + if err := mkfifo(filepath.Join(chromeRoot(home), "Local State")); err != nil { + t.Skipf("cannot create a fifo here: %v", err) + } + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserChrome) + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonParseError { + t.Errorf("status = %q/%q, want %q/%q", got.Status, got.ReasonCode, + model.BrowserCoverageFailed, model.BrowserExtReasonParseError) + } +} + +// TestDetect_OversizeStateFileIsNotParsedFromItsPrefix covers the read that grew +// past its cap. A prefix of a JSON document is not a shorter version of it: parsed +// as one it would report a fraction of the extensions as the complete set, which +// is the one thing a coverage status must never say wrongly. +func TestDetect_OversizeStateFileIsNotParsedFromItsPrefix(t *testing.T) { + home := tempHome(t) + padding := strings.Repeat(" ", maxLocalStateBytes) + writeFile(t, filepath.Join(chromeRoot(home), "Local State"), + `{"profile": {"info_cache": {"Default": {}}}}`+padding) + securePrefs(t, chromeRoot(home), "Default", + `"`+idA+`": {"location": 1, "manifest": {"name": "Example", "version": "1.0"}}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserChrome) + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonCapped { + t.Errorf("status = %q/%q, want %q/%q", got.Status, got.ReasonCode, + model.BrowserCoverageFailed, model.BrowserExtReasonCapped) + } + if len(info.Findings) != 0 { + t.Errorf("findings = %d, want none from a browser that failed", len(info.Findings)) + } + if info.Truncated { + // The payload was not cut short; one browser failed. Saying otherwise + // would describe a bounded result when the bound was on a file. + t.Error("truncated is set for a file that failed its own cap") + } +} + +// TestDetect_DeadlineFailsTheRemainingBrowsers checks the other polarity: a +// deadline does bound the payload, so it fails the browsers it did not reach and +// says the result was cut short. +func TestDetect_DeadlineFailsTheRemainingBrowsers(t *testing.T) { + home := tempHome(t) + localState(t, chromeRoot(home), "Default") + securePrefs(t, chromeRoot(home), "Default", + `"`+idA+`": {"location": 1, "manifest": {"name": "Example", "version": "1.0"}}`) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + info := newDetector(model.PlatformLinux).Detect(ctx, testUser(home)) + if info == nil { + t.Fatal("Detect returned the did-not-run sentinel") + } + assertPayloadInvariants(t, info) + + if !info.Truncated || info.TruncatedReason != model.BrowserExtTruncatedDeadline { + t.Errorf("truncated = %v/%q, want a deadline", info.Truncated, info.TruncatedReason) + } + for _, b := range info.Browsers { + if b.Status != model.BrowserCoverageFailed || b.ReasonCode != model.BrowserExtReasonTimedOut { + t.Errorf("%s: status = %q/%q, want every browser failed and timed out", + b.BrowserID, b.Status, b.ReasonCode) + } + } + if len(info.Findings) != 0 { + t.Errorf("findings = %d, want none", len(info.Findings)) + } +} + +// TestDetect_ConsentGuardRefusesADerivedPath covers the one location class the +// catalog does not fix: a profile directory named by the browser's own config +// file. It is a string an attacker can write, so it is refused before it is +// touched rather than after — the browsers' own directories stay readable under +// the same guard, which is the whole point of the exemption. +func TestDetect_ConsentGuardRefusesADerivedPath(t *testing.T) { + home := tempHome(t) + guarded := filepath.Join(home, "Documents", "hidden-profile") + writeFile(t, filepath.Join(guarded, "extensions.json"), + `{"addons": [{"id": "guarded@example-org", "type": "extension", "active": true}]}`) + writeFile(t, filepath.Join(firefoxRoot(home), "profiles.ini"), + "[Profile0]\nIsRelative=0\nPath="+filepath.ToSlash(guarded)+"\n") + + d := newDetector(model.PlatformLinux).WithSkipper(tcc.New(home)) + info := d.Detect(context.Background(), testUser(home)) + if info == nil { + t.Fatal("Detect returned the did-not-run sentinel") + } + assertPayloadInvariants(t, info) + + got := coverageFor(t, info, browserFirefox) + if runtime.GOOS != "darwin" { + // The consent layer only exists on macOS, so elsewhere the path is read + // and the add-on reported. The assertion that travels is the negative + // one below: nothing may be refused for the browsers' own directories. + if got.Status == model.BrowserCoverageFailed && got.ReasonCode == model.BrowserExtReasonRefusedTCC { + t.Errorf("%s refused a readable path on a platform with no consent layer", browserFirefox) + } + return + } + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonRefusedTCC { + t.Errorf("status = %q/%q, want %q/%q", got.Status, got.ReasonCode, + model.BrowserCoverageFailed, model.BrowserExtReasonRefusedTCC) + } +} + +// TestDetect_GuardExemptsTheBrowsersOwnDirectories is the other half of the +// consent rule. The macOS skipper declines the whole Library directory, which is +// where three of the four browsers keep their state — filtering candidates +// through it unmodified would report every one of them unreadable. +func TestDetect_GuardExemptsTheBrowsersOwnDirectories(t *testing.T) { + home := tempHome(t) + root := filepath.Join(home, "Library", "Application Support", "Google", "Chrome") + localState(t, root, "Default") + securePrefs(t, root, "Default", + `"`+idA+`": {"location": 1, "manifest": {"name": "Example", "version": "1.0"}}`) + + d := newDetector(model.PlatformDarwin).WithSkipper(tcc.New(home)) + info := d.Detect(context.Background(), testUser(home)) + if info == nil { + t.Fatal("Detect returned the did-not-run sentinel") + } + assertPayloadInvariants(t, info) + + got := coverageFor(t, info, browserChrome) + if got.Status != model.BrowserCoverageScanned { + t.Fatalf("status = %q/%q, want %q", got.Status, got.ReasonCode, model.BrowserCoverageScanned) + } + if len(findingsFor(info, browserChrome)) != 1 { + t.Errorf("findings = %d, want the one installed extension", len(findingsFor(info, browserChrome))) + } +} + +// TestDetect_CapFailsOnlyTheOverflowingBrowser pins where a cap cuts, in both +// directions. Mid-browser truncation is banned: a browser's reported list is read +// as complete, so a list cut in half would retire the extensions below the cut. But +// the bound belongs to that browser's own list, so the browsers on either side of +// it are answered normally — one browser with too many profiles must not blind the +// scan to the rest of the machine. The per-browser bound is the reachable one: +// four browsers cannot fill the whole-run budget. +func TestDetect_CapFailsOnlyTheOverflowingBrowser(t *testing.T) { + home := tempHome(t) + // One extension for the browser before the cap and one for the browser after + // it, with the overflowing browser between them. + localState(t, chromeRoot(home), "Default") + securePrefs(t, chromeRoot(home), "Default", + `"`+idA+`": {"location": 1, "manifest": {"name": "Example", "version": "1.0"}}`) + + edgeRoot := filepath.Join(home, ".config", "microsoft-edge") + localState(t, edgeRoot, "Default") + securePrefs(t, edgeRoot, "Default", manySettings(maxExtensionsPerBrowser+1)) + + braveRoot := filepath.Join(home, ".config", "BraveSoftware", "Brave-Browser") + localState(t, braveRoot, "Default") + securePrefs(t, braveRoot, "Default", + `"`+idB+`": {"location": 1, "manifest": {"name": "Example Notes", "version": "2.0"}}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + + if got := coverageFor(t, info, browserChrome); got.Status != model.BrowserCoverageScanned { + t.Errorf("chrome: status = %q, want the browser scanned before the cap to survive", got.Status) + } + got := coverageFor(t, info, browserEdge) + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonCapped { + t.Errorf("edge: status = %q/%q, want failed and capped", got.Status, got.ReasonCode) + } + if got := coverageFor(t, info, browserBrave); got.Status != model.BrowserCoverageScanned { + t.Errorf("brave: status = %q, want the browser after the cap scanned on its own merits", got.Status) + } + if len(findingsFor(info, browserBrave)) != 1 { + t.Errorf("brave findings = %d, want the one installed extension", len(findingsFor(info, browserBrave))) + } + if !info.Truncated || info.TruncatedReason != model.BrowserExtTruncatedFindingCap { + t.Errorf("truncated = %v/%q, want the finding cap", info.Truncated, info.TruncatedReason) + } + if info.ScanComplete { + t.Error("scan_complete = true, want a cut payload to say so") + } +} + +// TestDetect_FoldsOneExtensionSeenInTwoProfiles covers the reduction. Enabled +// anywhere means the extension can run on this machine; the rest of the record +// comes from one profile as a block, because pairing one profile's version with +// another's permissions would describe an extension that exists nowhere. +func TestDetect_FoldsOneExtensionSeenInTwoProfiles(t *testing.T) { + home := tempHome(t) + localState(t, chromeRoot(home), "Default", "Profile 1") + securePrefs(t, chromeRoot(home), "Default", `"`+idA+`": { + "location": 1, "disable_reasons": [1], + "manifest": {"name": "Example Reader", "version": "1.0.0"}, + "granted_permissions": {"api": ["storage"]} + }`) + securePrefs(t, chromeRoot(home), "Profile 1", `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example Reader", "version": "2.0.0"}, + "granted_permissions": {"api": ["tabs"]} + }`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + + got := findingsFor(info, browserChrome) + if len(got) != 1 { + t.Fatalf("findings = %d, want one per extension however many profiles hold it", len(got)) + } + f := got[0] + if f.EnabledState != model.BrowserExtEnabled { + t.Errorf("enabled_state = %q, want enabled: it runs in the second profile", f.EnabledState) + } + if f.DisabledBy != "" { + t.Errorf("disabled_by = %q, want none on an enabled extension", f.DisabledBy) + } + // The first-sorting profile owns the block, so both fields come from it. + if f.Version != "1.0.0" || len(f.Permissions) != 1 || f.Permissions[0] != "storage" { + t.Errorf("version/permissions = %q/%v, want one profile's whole record", f.Version, f.Permissions) + } + if got := coverageFor(t, info, browserChrome); got.ProfileCount != 2 { + t.Errorf("profile_count = %d, want 2", got.ProfileCount) + } +} + +// TestDetect_DisabledCauseComesFromADisabledProfile is the trap in the reduction: +// the profile that owns the record may be an enabled one, and reading the cause +// off it would attach an enabled profile's empty cause to a disabled row. +func TestDetect_DisabledCauseComesFromADisabledProfile(t *testing.T) { + home := tempHome(t) + localState(t, chromeRoot(home), "Default", "Profile 1") + // The first-sorting profile is disabled by the user, the second by the + // browser; neither is enabled, so the row is disabled and needs one cause. + securePrefs(t, chromeRoot(home), "Default", `"`+idA+`": { + "location": 1, "disable_reasons": [1], "manifest": {"name": "Example", "version": "1.0"} + }`) + securePrefs(t, chromeRoot(home), "Profile 1", `"`+idA+`": { + "location": 1, "disable_reasons": [512], "manifest": {"name": "Example", "version": "1.0"} + }`) + + got := findingsFor(scanHome(t, home), browserChrome) + if len(got) != 1 { + t.Fatalf("findings = %d, want one", len(got)) + } + if got[0].EnabledState != model.BrowserExtDisabled || got[0].DisabledBy != model.BrowserExtDisabledByUser { + t.Errorf("state/cause = %q/%q, want disabled by the user", got[0].EnabledState, got[0].DisabledBy) + } +} + +// TestDetect_IsDeterministic asserts what a fixed ordering rule is for: two runs +// over an unchanged machine produce the same payload, so a reader never sees a +// change that is only this detector's map iteration. +func TestDetect_IsDeterministic(t *testing.T) { + home := tempHome(t) + localState(t, chromeRoot(home), "Default", "Profile 1", "Profile 2") + settings := `"` + idA + `": {"location": 1, "manifest": {"name": "A", "version": "1.0"}},` + + `"` + idB + `": {"location": 4, "manifest": {"name": "B", "version": "2.0"}},` + + `"` + idC + `": {"location": 2, "manifest": {"name": "C", "version": "3.0"}}` + for _, profile := range []string{"Default", "Profile 1", "Profile 2"} { + securePrefs(t, chromeRoot(home), profile, settings) + } + + want := marshalFindings(t, scanHome(t, home)) + for run := range 4 { + if got := marshalFindings(t, scanHome(t, home)); got != want { + t.Fatalf("run %d produced different findings\n got: %s\nwant: %s", run+1, got, want) + } + } +} + +// marshalFindings renders the findings list, which is what a reader compares two +// scans by. Timings differ between runs and are not part of the question. +func marshalFindings(t *testing.T, info *model.BrowserExtensionScanInfo) string { + t.Helper() + raw, err := json.Marshal(info.Findings) + if err != nil { + t.Fatalf("marshal findings: %v", err) + } + return string(raw) +} diff --git a/internal/detector/browserext/fifo_other_test.go b/internal/detector/browserext/fifo_other_test.go new file mode 100644 index 00000000..54e6a309 --- /dev/null +++ b/internal/detector/browserext/fifo_other_test.go @@ -0,0 +1,10 @@ +//go:build windows + +package browserext + +import "errors" + +// mkfifo has no counterpart here; the caller skips. +func mkfifo(string) error { + return errors.New("named pipes are not directory entries on this platform") +} diff --git a/internal/detector/browserext/fifo_unix_test.go b/internal/detector/browserext/fifo_unix_test.go new file mode 100644 index 00000000..2f1856d5 --- /dev/null +++ b/internal/detector/browserext/fifo_unix_test.go @@ -0,0 +1,10 @@ +//go:build !windows + +package browserext + +import "golang.org/x/sys/unix" + +// mkfifo plants a named pipe, which is the hostile object a state-file read has to +// survive: opening one the ordinary way blocks until a writer appears, and no +// deadline interrupts a blocked open. +func mkfifo(path string) error { return unix.Mkfifo(path, 0o644) } diff --git a/internal/detector/browserext/gecko.go b/internal/detector/browserext/gecko.go new file mode 100644 index 00000000..61fde907 --- /dev/null +++ b/internal/detector/browserext/gecko.go @@ -0,0 +1,435 @@ +package browserext + +import ( + "context" + "encoding/json" + "path/filepath" + "slices" + "sort" + "strings" + + "github.com/step-security/dev-machine-guard/internal/model" +) + +// The gecko family. Profiles are listed in an INI file and each one keeps its +// add-on database in a single JSON document. + +// Add-on classes. A closed vocabulary rather than a permissive filter: an entry +// whose class this build does not recognise may be a real extension, and under a +// status that reads as a complete list, silently skipping it un-reports it. So an +// unrecognised class fails the browser instead, which is a visible gap that one +// vocabulary entry closes rather than a membership list that quietly shrank. +const geckoTypeExtension = "extension" + +var geckoNonMemberTypes = map[string]bool{ + "theme": true, + "locale": true, + "dictionary": true, + "sitepermission": true, +} + +// Where the add-on is installed from, as the database records it. The builtin +// scopes are the browser's own components and are not reported; the sideload +// scopes are alive on the extended-support channel, which is exactly the +// enterprise population, so they are reported rather than dropped. +var geckoInstallSources = map[string]string{ + "app-profile": model.BrowserExtInstallUser, + "winreg-app-user": model.BrowserExtInstallRegistry, + "winreg-app-global": model.BrowserExtInstallRegistry, + "app-system-user": model.BrowserExtInstallSideload, + "app-global": model.BrowserExtInstallSideload, + "app-system-share": model.BrowserExtInstallSideload, + "app-system-local": model.BrowserExtInstallSideload, + "app-temporary": model.BrowserExtInstallUnpacked, +} + +var geckoBuiltinScopes = map[string]bool{ + "app-builtin": true, + "app-builtin-addons": true, + "app-system-addons": true, + "app-system-profile": true, + "app-system-defaults": true, +} + +// The only on-disk discriminator for an administrator-installed add-on: its +// scope is an ordinary profile install and nothing else distinguishes it. +const geckoPolicySource = "enterprise-policy" + +// Signing states, as the database numbers them. An unsigned add-on that is +// enabled is the headline security signal on this engine. +var geckoSignedStates = map[int]string{ + -2: model.BrowserExtSignedBroken, + -1: model.BrowserExtSignedUnknownChain, + 0: model.BrowserExtSignedMissing, + 1: model.BrowserExtSignedPreliminary, + 2: model.BrowserExtSignedSigned, + 3: model.BrowserExtSignedSystem, + 4: model.BrowserExtSignedPrivileged, +} + +// scanGeckoRoot reads one gecko data directory and reports whether it held an +// installation. +func (d *Detector) scanGeckoRoot(ctx context.Context, scan *scanState, root string, b *browserScan) bool { + declared, registered, reason := d.geckoDeclaredProfiles(scan, root) + if reason != "" { + b.fail(reason) + return true + } + discovered, reason := d.geckoDiscoveredProfiles(scan, root) + if reason != "" { + b.fail(reason) + return true + } + + candidates := declared + for _, dir := range discovered { + if !slices.Contains(candidates, dir) { + candidates = append(candidates, dir) + } + } + sort.Strings(candidates) + + found := false + for _, dir := range candidates { + if ctx.Err() != nil { + b.failPayload(model.BrowserExtReasonTimedOut, model.BrowserExtTruncatedDeadline) + return true + } + if b.profiles >= maxProfilesPerBrowser { + b.failBounded(model.BrowserExtReasonCapped, model.BrowserExtTruncatedFindingCap) + return true + } + data, missing, reason := scan.readState(filepath.Join(dir, "extensions.json"), maxExtensionsJSONBytes) + if reason != "" { + b.fail(reason) + return true + } + if missing { + // A directory that is not a profile, or a profile the browser has + // registered and never opened. Neither is a failure to report. + continue + } + found = true + b.profiles++ + d.scanGeckoProfile(scan, dir, data, b) + if b.failure != "" { + return true + } + } + // An installation is a registered profile list or a profile holding a + // database, and nothing else. A directory left behind by an uninstall — or by + // some other installer — is reported as absent rather than as a browser that + // could not be read, which would paint a permanent failure for a browser + // nobody has. + return found || registered +} + +// geckoDeclaredProfiles reads the profile list the browser maintains. +// +// A profile may be declared at an absolute path, which is the one location class +// this detector does not fix itself: it is a string a config file handed over, so +// it is checked before it is touched rather than after. A path outside the +// account's own tree, or inside a directory the consent layer gates, is refused +// unread. +func (d *Detector) geckoDeclaredProfiles(scan *scanState, root string) (dirs []string, registered bool, reason string) { + data, missing, reason := scan.readState(filepath.Join(root, "profiles.ini"), maxProfilesINIBytes) + if reason != "" { + return nil, false, reason + } + if missing { + return nil, false, "" + } + parsed, ok := parseProfilesINI(data) + if !ok { + // This file is the membership list's outer boundary: a profile it declares + // and this build cannot place is a profile full of add-ons that would go + // unlisted, under a status claiming the list is complete. + return nil, false, model.BrowserExtReasonParseError + } + for _, p := range parsed { + path := filepath.FromSlash(p.path) + if p.relative || !filepath.IsAbs(path) { + path = filepath.Join(root, path) + } + dirs = append(dirs, filepath.Clean(path)) + } + // The file itself is the evidence of an installation: a browser that has + // registered profiles is installed even if none of them has been opened. + return dirs, true, "" +} + +// geckoDiscoveredProfiles lists the directories that look like profiles, in both +// layouts the platforms use — directly under the root, and under a Profiles +// subdirectory. +// +// The listing is a union with the declared list rather than a replacement for it: +// a profile unregistered from the INI file still holds real extensions, and a +// missing one would be a membership gap under a status that claims completeness. +func (d *Detector) geckoDiscoveredProfiles(scan *scanState, root string) (dirs []string, reason string) { + for _, base := range []string{root, filepath.Join(root, "Profiles")} { + names, missing, reason := scan.listNames(base, maxProfileEntries) + if reason != "" { + return nil, reason + } + if missing { + continue + } + for _, name := range names { + candidate := filepath.Join(base, name) + isDir, missing, reason := scan.statEntry(candidate) + if reason != "" { + return nil, reason + } + if missing || !isDir { + // The root holds files as well: the INI files themselves, and + // whatever else the browser keeps beside its profiles. + continue + } + dirs = append(dirs, candidate) + } + } + return dirs, "" +} + +// iniProfile is one profile as the INI file declares it. +type iniProfile struct { + path string + // Relative to the data directory. The default, and the common case. + relative bool +} + +// parseProfilesINI reads the profile sections, reporting false when a section +// declares a profile this cannot place: no path, or a relative flag outside the +// two values the file uses. Hand-rolled because the file is a flat list of two +// interesting keys, which is not worth a dependency. +func parseProfilesINI(data []byte) ([]iniProfile, bool) { + var profiles []iniProfile + current := -1 + for _, raw := range strings.Split(string(data), "\n") { + line := strings.TrimSpace(strings.TrimSuffix(raw, "\r")) + if line == "" || strings.HasPrefix(line, ";") || strings.HasPrefix(line, "#") { + continue + } + if strings.HasPrefix(line, "[") && strings.HasSuffix(line, "]") { + // Install sections name each installation's default profile and + // duplicate what the profile sections already say, so only the + // profile sections are read. + if strings.HasPrefix(strings.ToLower(line), "[profile") { + profiles = append(profiles, iniProfile{relative: true}) + current = len(profiles) - 1 + } else { + current = -1 + } + continue + } + if current < 0 { + continue + } + key, value, ok := strings.Cut(line, "=") + if !ok { + continue + } + switch strings.ToLower(strings.TrimSpace(key)) { + case "path": + profiles[current].path = strings.TrimSpace(value) + case "isrelative": + switch strings.TrimSpace(value) { + case "0": + profiles[current].relative = false + case "1": + profiles[current].relative = true + default: + return nil, false + } + } + } + for _, p := range profiles { + if p.path == "" { + return nil, false + } + } + return profiles, true +} + +// geckoAddon is one record in the add-on database. Every field is optional and +// nothing is gated on the document's schema version, which changes every release +// — but tolerance has a floor: a record whose identity or class cannot be +// recovered is a membership question, and those fail the browser. +type geckoAddon struct { + ID *string `json:"id"` + Version string `json:"version"` + Type *string `json:"type"` + Active *bool `json:"active"` + // The three ways an add-on can be off. They separate the user's own choice + // from the browser's. + UserDisabled *bool `json:"userDisabled"` + AppDisabled *bool `json:"appDisabled"` + SoftDisabled *bool `json:"softDisabled"` + Location string `json:"location"` + SignedState *int `json:"signedState"` + Visible *bool `json:"visible"` + Hidden *bool `json:"hidden"` + + DefaultLocale struct { + Name string `json:"name"` + } `json:"defaultLocale"` + + UserPermissions struct { + Permissions []string `json:"permissions"` + Origins []string `json:"origins"` + } `json:"userPermissions"` + + InstallTelemetryInfo struct { + Source string `json:"source"` + } `json:"installTelemetryInfo"` +} + +// scanGeckoProfile reads one profile's add-on database. +func (d *Detector) scanGeckoProfile(scan *scanState, dir string, data []byte, b *browserScan) { + var db struct { + // A pointer so an absent list is told apart from an empty one. A profile + // with no add-ons still writes the key, so a document without it is one + // this build cannot read — and reading it as zero add-ons would publish a + // complete-looking empty list that retires everything stored for the + // browser. + Addons *[]json.RawMessage `json:"addons"` + } + if err := json.Unmarshal(data, &db); err != nil || db.Addons == nil { + b.fail(model.BrowserExtReasonParseError) + return + } + for _, raw := range *db.Addons { + var a geckoAddon + if err := json.Unmarshal(raw, &a); err != nil { + // Unlike the other family, there is no map key to fall back on: the + // identity lives inside the record, so a record that will not decode + // is an extension whose identity cannot be recovered, and the list is + // no longer known to be complete. + b.fail(model.BrowserExtReasonParseError) + return + } + if a.ID == nil || *a.ID == "" { + b.fail(model.BrowserExtReasonParseError) + return + } + id := *a.ID + if len(id) > maxExtensionIDBytes { + // An identity cannot be shortened to fit — a truncated identity is a + // different extension — and dropping it under a status that claims a + // complete list would retire the real one's stored row. + b.fail(model.BrowserExtReasonParseError) + return + } + if a.Type == nil { + b.fail(model.BrowserExtReasonParseError) + return + } + if geckoNonMemberTypes[*a.Type] { + continue + } + if *a.Type != geckoTypeExtension { + b.fail(model.BrowserExtReasonParseError) + return + } + if (a.Hidden != nil && *a.Hidden) || (a.Visible != nil && !*a.Visible) { + // A system add-on, or a duplicate the browser has shadowed. + continue + } + if geckoBuiltinScopes[a.Location] { + continue + } + + occ, ok := d.geckoOccurrence(a, b) + if !ok { + continue + } + occ.sortKey = dir + if !b.add(id, occ) { + return + } + } +} + +// geckoOccurrence turns one add-on record into one occurrence. +func (d *Detector) geckoOccurrence(a geckoAddon, b *browserScan) (occurrence, bool) { + source := model.BrowserExtInstallUnknown + if mapped, ok := geckoInstallSources[a.Location]; ok { + source = mapped + } + if a.InstallTelemetryInfo.Source == geckoPolicySource { + // An administrator install looks like an ordinary profile install + // everywhere else in the record. + source = model.BrowserExtInstallPolicy + } + + state, disabledBy := geckoEnabledState(a) + perms, permsCapped := capPermissionList(a.UserPermissions.Permissions) + hosts, hostsCapped := capPermissionList(a.UserPermissions.Origins) + if permsCapped || hostsCapped { + b.degrade(model.BrowserExtReasonCapped) + } + return occurrence{ + enabled: state, + disabledBy: disabledBy, + block: model.BrowserExtensionFinding{ + Name: capBytes(a.DefaultLocale.Name, maxNameBytes), + Version: capBytes(a.Version, maxVersionBytes), + EnabledState: state, + // The store fields have no counterpart on this engine: there is no + // listing state in the database to read. + InstallSource: source, + Store: geckoStore(a.SignedState), + SignedState: geckoSignedState(a.SignedState), + Permissions: perms, + HostPermissions: hosts, + }, + }, true +} + +// geckoEnabledState derives whether the add-on runs, and who stopped it. A record +// that does not say whether it is active is reported as unknown rather than +// assumed off: an invented cause would be displayed as a fact. +func geckoEnabledState(a geckoAddon) (state, disabledBy string) { + if a.Active == nil { + return model.BrowserExtStateUnknown, "" + } + if *a.Active { + return model.BrowserExtEnabled, "" + } + switch { + case a.UserDisabled != nil && *a.UserDisabled: + return model.BrowserExtDisabled, model.BrowserExtDisabledByUser + case (a.AppDisabled != nil && *a.AppDisabled) || (a.SoftDisabled != nil && *a.SoftDisabled): + return model.BrowserExtDisabled, model.BrowserExtDisabledByBrowser + default: + return model.BrowserExtDisabled, model.BrowserExtDisabledByUnknown + } +} + +// geckoSignedState maps the recorded state, and omits a value it does not +// recognise rather than inventing a label for it. +func geckoSignedState(signed *int) string { + if signed == nil { + return "" + } + return geckoSignedStates[*signed] +} + +// geckoStore attributes the add-on to a store from its signature alone. The +// download URL would be the honest signal and is dropped unread, because it can +// embed a private address. The cost is recorded rather than hidden: an +// enterprise add-on that is self-hosted but vendor-signed attributes to the public +// store. +func geckoStore(signed *int) string { + if signed == nil { + return model.BrowserExtStoreUnknown + } + switch *signed { + case 1, 2: + return model.BrowserExtStoreAMO + case 0: + return model.BrowserExtStoreNone + default: + return model.BrowserExtStoreUnknown + } +} diff --git a/internal/detector/browserext/gecko_test.go b/internal/detector/browserext/gecko_test.go new file mode 100644 index 00000000..09194e1f --- /dev/null +++ b/internal/detector/browserext/gecko_test.go @@ -0,0 +1,476 @@ +package browserext + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/step-security/dev-machine-guard/internal/model" +) + +// geckoAddonJSON writes one profile's add-on database. +func geckoAddonJSON(t *testing.T, dir, addons string) { + t.Helper() + writeFile(t, filepath.Join(dir, "extensions.json"), `{"schemaVersion": 37, "addons": [`+addons+`]}`) +} + +// firefoxFinding runs one Firefox profile and returns its single finding plus the +// browser's coverage entry. +func firefoxFinding(t *testing.T, addon string) (model.BrowserExtensionFinding, model.BrowserCoverage) { + t.Helper() + home := tempHome(t) + root := firefoxRoot(home) + writeFile(t, filepath.Join(root, "profiles.ini"), "[Profile0]\nIsRelative=1\nPath=abcd1234.default-release\n") + geckoAddonJSON(t, filepath.Join(root, "abcd1234.default-release"), addon) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserFirefox) + if len(got) != 1 { + t.Fatalf("findings = %d, want exactly one", len(got)) + } + return got[0], coverageFor(t, info, browserFirefox) +} + +// TestGecko_ProfileEnumeration covers both ways a profile is found. The listing is +// a union with the declared list rather than a replacement: a profile +// unregistered from the declaration still holds real extensions, and missing it +// would be a membership gap under a status that claims completeness. +func TestGecko_ProfileEnumeration(t *testing.T) { + home := tempHome(t) + root := firefoxRoot(home) + orphan := filepath.Join(root, "zzzz9999.orphan") + declared := filepath.Join(root, "abcd1234.default-release") + absolute := filepath.Join(home, "mozilla-profiles", "work") + + writeFile(t, filepath.Join(root, "profiles.ini"), strings.Join([]string{ + "[Profile0]", "IsRelative=1", "Path=abcd1234.default-release", + "[Profile1]", "IsRelative=0", "Path=" + filepath.ToSlash(absolute), + // An installation section repeats what the profile sections say and is + // not read as a profile of its own. + "[Install4F96D1932A9F858E]", "Default=abcd1234.default-release", "Locked=1", + "", + }, "\n")) + geckoAddonJSON(t, declared, `{"id": "declared@example-org", "type": "extension", "active": true}`) + geckoAddonJSON(t, absolute, `{"id": "relocated@example-org", "type": "extension", "active": true}`) + geckoAddonJSON(t, orphan, `{"id": "orphan@example-org", "type": "extension", "active": true}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + + got := map[string]bool{} + for _, f := range findingsFor(info, browserFirefox) { + got[f.ExtensionID] = true + } + for _, want := range []string{"declared@example-org", "relocated@example-org", "orphan@example-org"} { + if !got[want] { + t.Errorf("no finding for %s", want) + } + } + if c := coverageFor(t, info, browserFirefox); c.ProfileCount != 3 { + t.Errorf("profile_count = %d, want 3", c.ProfileCount) + } +} + +// TestGecko_TypeVocabularyInBothPolarities is the rule that trades a visible gap +// for a silent one. A recognised non-member is skipped and costs nothing; a class +// this build does not know may be a real extension, so it fails the browser rather +// than being dropped from a list that reads as complete or reported as something +// it is not. +func TestGecko_TypeVocabularyInBothPolarities(t *testing.T) { + tests := []struct { + name string + addon string + status string + reason string + }{ + { + name: "an extension is a member", + addon: `{"id": "ext@example-org", "type": "extension", "active": true}`, + status: model.BrowserCoverageScanned, + }, + { + name: "a theme is a recognised non-member", + addon: `{"id": "theme@example-org", "type": "theme", "active": true}`, + status: model.BrowserCoverageScanned, + }, + { + name: "a language pack is a recognised non-member", + addon: `{"id": "langpack-de@example-org", "type": "locale", "active": true}`, + status: model.BrowserCoverageScanned, + }, + { + name: "a dictionary is a recognised non-member", + addon: `{"id": "dict-de@example-org", "type": "dictionary", "active": true}`, + status: model.BrowserCoverageScanned, + }, + { + name: "a site permission is a recognised non-member", + addon: `{"id": "sitepermission@example-org", "type": "sitepermission", "active": true}`, + status: model.BrowserCoverageScanned, + }, + { + name: "a class this build does not know fails the browser", + addon: `{"id": "future@example-org", "type": "recipe", "active": true}`, + status: model.BrowserCoverageFailed, + reason: model.BrowserExtReasonParseError, + }, + { + name: "a record with no class fails the browser", + addon: `{"id": "classless@example-org", "active": true}`, + status: model.BrowserCoverageFailed, + reason: model.BrowserExtReasonParseError, + }, + { + // No map key to fall back on here: the identity lives inside the + // record, so a record that will not decode is an extension whose + // identity cannot be recovered. + name: "a record that will not decode fails the browser", + addon: `"not a record"`, + status: model.BrowserCoverageFailed, + reason: model.BrowserExtReasonParseError, + }, + { + name: "a record with no identity fails the browser", + addon: `{"type": "extension", "active": true}`, + status: model.BrowserCoverageFailed, + reason: model.BrowserExtReasonParseError, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := tempHome(t) + root := firefoxRoot(home) + writeFile(t, filepath.Join(root, "profiles.ini"), "[Profile0]\nIsRelative=1\nPath=p1\n") + geckoAddonJSON(t, filepath.Join(root, "p1"), tc.addon) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserFirefox) + if got.Status != tc.status || got.ReasonCode != tc.reason { + t.Errorf("status = %q/%q, want %q/%q", got.Status, got.ReasonCode, tc.status, tc.reason) + } + if tc.status == model.BrowserCoverageFailed && len(findingsFor(info, browserFirefox)) != 0 { + t.Error("a failed browser shipped findings") + } + }) + } +} + +// TestGecko_InstallSourceAndExclusions maps the recorded scopes. The sideload +// scopes are alive on the extended-support channel, which is exactly the +// enterprise population, so they are reported; the builtin scopes are the +// browser's own parts and are not. +func TestGecko_InstallSourceAndExclusions(t *testing.T) { + tests := []struct { + name string + record string + want string + excluded bool + }{ + {name: "an ordinary profile install", record: `"location": "app-profile"`, want: model.BrowserExtInstallUser}, + { + name: "an administrator install, which looks like any other", + record: `"location": "app-profile", "installTelemetryInfo": {"source": "enterprise-policy"}`, + want: model.BrowserExtInstallPolicy, + }, + {name: "a registry sideload", record: `"location": "winreg-app-user"`, want: model.BrowserExtInstallRegistry}, + {name: "a filesystem sideload", record: `"location": "app-system-share"`, want: model.BrowserExtInstallSideload}, + {name: "a temporary load", record: `"location": "app-temporary"`, want: model.BrowserExtInstallUnpacked}, + {name: "a scope this build does not know", record: `"location": "app-future"`, want: model.BrowserExtInstallUnknown}, + {name: "a browser component", record: `"location": "app-builtin"`, excluded: true}, + {name: "a system add-on", record: `"location": "app-system-addons"`, excluded: true}, + {name: "a hidden add-on", record: `"location": "app-profile", "hidden": true`, excluded: true}, + {name: "a shadowed duplicate", record: `"location": "app-profile", "visible": false`, excluded: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + addon := `{"id": "ext@example-org", "type": "extension", "active": true, ` + tc.record + `}` + if tc.excluded { + home := tempHome(t) + root := firefoxRoot(home) + writeFile(t, filepath.Join(root, "profiles.ini"), "[Profile0]\nIsRelative=1\nPath=p1\n") + geckoAddonJSON(t, filepath.Join(root, "p1"), addon) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + if got := findingsFor(info, browserFirefox); len(got) != 0 { + t.Errorf("findings = %d, want none", len(got)) + } + if c := coverageFor(t, info, browserFirefox); c.Status != model.BrowserCoverageScanned { + t.Errorf("status = %q/%q, want the browser scanned: an exclusion is not damage", c.Status, c.ReasonCode) + } + return + } + got, _ := firefoxFinding(t, addon) + if got.InstallSource != tc.want { + t.Errorf("install_source = %q, want %q", got.InstallSource, tc.want) + } + }) + } +} + +// TestGecko_SigningStateAndStore covers the engine-specific field pair. An +// unsigned add-on that is enabled is the headline signal here, and the signature is +// also all there is to attribute a store by: the download address would be the +// honest signal and is dropped unread because it can embed a private one. +func TestGecko_SigningStateAndStore(t *testing.T) { + tests := []struct { + name string + record string + signed string + store string + }{ + {name: "unsigned", record: `"signedState": 0`, signed: model.BrowserExtSignedMissing, store: model.BrowserExtStoreNone}, + {name: "signed", record: `"signedState": 2`, signed: model.BrowserExtSignedSigned, store: model.BrowserExtStoreAMO}, + {name: "preliminarily signed", record: `"signedState": 1`, signed: model.BrowserExtSignedPreliminary, store: model.BrowserExtStoreAMO}, + {name: "a broken signature", record: `"signedState": -2`, signed: model.BrowserExtSignedBroken, store: model.BrowserExtStoreUnknown}, + {name: "an unverifiable chain", record: `"signedState": -1`, signed: model.BrowserExtSignedUnknownChain, store: model.BrowserExtStoreUnknown}, + {name: "a browser-privileged add-on", record: `"signedState": 4`, signed: model.BrowserExtSignedPrivileged, store: model.BrowserExtStoreUnknown}, + {name: "no signature record at all", record: `"version": "1.0"`, signed: "", store: model.BrowserExtStoreUnknown}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, _ := firefoxFinding(t, + `{"id": "ext@example-org", "type": "extension", "active": true, "location": "app-profile", `+tc.record+`}`) + if got.SignedState != tc.signed { + t.Errorf("signed_state = %q, want %q", got.SignedState, tc.signed) + } + if got.Store != tc.store { + t.Errorf("store = %q, want %q", got.Store, tc.store) + } + // The store fields have no counterpart on this engine, and a reader + // rejects a payload that carries them here. + if got.StoreListing != "" || got.StoreViolation != "" { + t.Errorf("store fields = %q/%q, want neither on this engine", got.StoreListing, got.StoreViolation) + } + }) + } +} + +// TestGecko_EnabledState covers who turned the add-on off. A record that does not +// say whether it is active reports unknown rather than an invented cause. +func TestGecko_EnabledState(t *testing.T) { + tests := []struct { + name string + record string + state string + disabledBy string + }{ + {name: "active", record: `"active": true`, state: model.BrowserExtEnabled}, + { + name: "the user's own choice", + record: `"active": false, "userDisabled": true`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByUser, + }, + { + name: "the browser's decision", + record: `"active": false, "appDisabled": true`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByBrowser, + }, + { + name: "off with no reason recorded", + record: `"active": false`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByUnknown, + }, + {name: "no state recorded", record: `"version": "1.0"`, state: model.BrowserExtStateUnknown}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, _ := firefoxFinding(t, + `{"id": "ext@example-org", "type": "extension", "location": "app-profile", `+tc.record+`}`) + if got.EnabledState != tc.state || got.DisabledBy != tc.disabledBy { + t.Errorf("state/cause = %q/%q, want %q/%q", got.EnabledState, got.DisabledBy, tc.state, tc.disabledBy) + } + }) + } +} + +// TestGecko_NameAndPermissions covers the metadata this engine records in one +// place, so no second file is opened for it. +func TestGecko_NameAndPermissions(t *testing.T) { + got, _ := firefoxFinding(t, `{ + "id": "ext@example-org", "type": "extension", "active": true, "location": "app-profile", + "version": "3.2.1", + "defaultLocale": {"name": "Example Content Filter", "description": "unread"}, + "userPermissions": {"permissions": ["webRequest", "storage"], "origins": [""]} + }`) + + if got.Name != "Example Content Filter" || got.Version != "3.2.1" { + t.Errorf("name/version = %q/%q, want the recorded values", got.Name, got.Version) + } + if strings.Join(got.Permissions, ",") != "storage,webRequest" { + t.Errorf("permissions = %v, want them sorted", got.Permissions) + } + if strings.Join(got.HostPermissions, ",") != "" { + t.Errorf("host_permissions = %v", got.HostPermissions) + } +} + +// TestGecko_OverlongIdentityFailsTheBrowser pins the one string that is never +// shortened. A truncated identity is a different extension, and dropping it under +// a status that claims a complete list would retire the real one's stored row. +func TestGecko_OverlongIdentityFailsTheBrowser(t *testing.T) { + home := tempHome(t) + root := firefoxRoot(home) + writeFile(t, filepath.Join(root, "profiles.ini"), "[Profile0]\nIsRelative=1\nPath=p1\n") + long := strings.Repeat("x", maxExtensionIDBytes+1) + "@example-org" + geckoAddonJSON(t, filepath.Join(root, "p1"), `{"id": "`+long+`", "type": "extension", "active": true}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserFirefox) + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonParseError { + t.Errorf("status = %q/%q, want failed rather than partial", got.Status, got.ReasonCode) + } + if len(findingsFor(info, browserFirefox)) != 0 { + t.Error("a failed browser shipped findings") + } +} + +// TestGecko_TwoDataDirectoriesDeduplicate covers the layout where a native install +// and a packaged one both exist. Their union is the answer, and one extension in +// both is one finding. +func TestGecko_TwoDataDirectoriesDeduplicate(t *testing.T) { + home := tempHome(t) + native := firefoxRoot(home) + snap := filepath.Join(home, "snap", "firefox", "common", ".mozilla", "firefox") + + for _, root := range []string{native, snap} { + writeFile(t, filepath.Join(root, "profiles.ini"), "[Profile0]\nIsRelative=1\nPath=p1\n") + } + geckoAddonJSON(t, filepath.Join(native, "p1"), + `{"id": "shared@example-org", "type": "extension", "active": false, "userDisabled": true, "location": "app-profile", "version": "1.0"}`) + geckoAddonJSON(t, filepath.Join(snap, "p1"), + `{"id": "shared@example-org", "type": "extension", "active": true, "location": "app-profile", "version": "2.0"}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserFirefox) + if len(got) != 1 { + t.Fatalf("findings = %d, want one per extension however many directories hold it", len(got)) + } + if got[0].EnabledState != model.BrowserExtEnabled { + t.Errorf("enabled_state = %q, want enabled: it runs in one of the two", got[0].EnabledState) + } + if c := coverageFor(t, info, browserFirefox); c.ProfileCount != 2 { + t.Errorf("profile_count = %d, want both directories' profiles counted together", c.ProfileCount) + } +} + +// TestGecko_MissingProfileListIsNotAnInstallation keeps a leftover directory from +// reading as a broken browser, which would paint a permanent red row for something +// nobody installed. An installation is a registered profile list or a profile +// holding a database; the residue of an uninstall is neither. +func TestGecko_MissingProfileListIsNotAnInstallation(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, home string) + }{ + { + name: "an empty directory", + setup: func(t *testing.T, home string) { mkdir(t, firefoxRoot(home)) }, + }, + { + name: "a directory holding leftovers but no profile", + setup: func(t *testing.T, home string) { + mkdir(t, filepath.Join(firefoxRoot(home), "Crash Reports")) + writeFile(t, filepath.Join(firefoxRoot(home), "installs.ini"), "\n") + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := tempHome(t) + tc.setup(t, home) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserFirefox) + if got.Status != model.BrowserCoverageNotPresent || got.ReasonCode != "" { + t.Errorf("status = %q/%q, want not present with no reason", got.Status, got.ReasonCode) + } + }) + } +} + +// TestGecko_UnreadableMembershipDocumentFailsTheBrowser covers the floor under +// tolerant parsing. Individual fields may be missing, but the two documents that +// define the membership list — which profiles exist, and which add-ons each holds +// — have to be understood whole. Each case below would otherwise produce a +// complete-looking empty list, which is the one answer that retires stored rows. +func TestGecko_UnreadableMembershipDocumentFailsTheBrowser(t *testing.T) { + tests := []struct { + name string + ini string + addon string + }{ + { + name: "a database that will not decode", + ini: "[Profile0]\nIsRelative=1\nPath=p1\n", + addon: `{"addons": [`, + }, + { + name: "a database carrying no add-on list at all", + ini: "[Profile0]\nIsRelative=1\nPath=p1\n", + // Not the same as an empty list: a profile with no add-ons still + // writes the key, so its absence is a document this cannot read. + addon: `{"schemaVersion": 37}`, + }, + { + name: "a declared profile with no path", + ini: "[Profile0]\nIsRelative=1\n", + addon: `{"addons": []}`, + }, + { + name: "a declared profile whose path is neither relative nor absolute", + ini: "[Profile0]\nIsRelative=maybe\nPath=p1\n", + addon: `{"addons": []}`, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := tempHome(t) + root := firefoxRoot(home) + writeFile(t, filepath.Join(root, "profiles.ini"), tc.ini) + writeFile(t, filepath.Join(root, "p1", "extensions.json"), tc.addon) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserFirefox) + if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonParseError { + t.Errorf("status = %q/%q, want failed and a parse error", got.Status, got.ReasonCode) + } + }) + } +} + +// TestGecko_ProfileOutsideTheHomeIsRefusedUnread covers the one path class this +// detector does not build itself. A declared profile is a string from a config +// file, so a location outside the account's own tree is refused before it is +// touched — a stat into a network volume is itself the call that blocks. Refused +// and absent are not interchangeable here: the browser fails rather than +// publishing a list that would read as complete. +func TestGecko_ProfileOutsideTheHomeIsRefusedUnread(t *testing.T) { + home := tempHome(t) + root := firefoxRoot(home) + // Nothing exists at this path. A resolver that walked to it before deciding + // would report it missing and carry on. + outside := filepath.Join(t.TempDir(), "elsewhere", "work") + writeFile(t, filepath.Join(root, "profiles.ini"), strings.Join([]string{ + "[Profile0]", "IsRelative=0", "Path=" + filepath.ToSlash(outside), "", + }, "\n")) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserFirefox) + if got.Status != model.BrowserCoverageFailed { + t.Errorf("status = %q/%q, want the browser failed rather than reported empty", got.Status, got.ReasonCode) + } + if len(findingsFor(info, browserFirefox)) != 0 { + t.Error("a failed browser shipped findings") + } +} diff --git a/internal/detector/browserext/session_other.go b/internal/detector/browserext/session_other.go new file mode 100644 index 00000000..8b541fd8 --- /dev/null +++ b/internal/detector/browserext/session_other.go @@ -0,0 +1,8 @@ +//go:build !windows + +package browserext + +// serviceSession has no meaning off Windows: no session concept separates a +// service from a login, and the account check covers what does. A daemon there is +// caught by its identity rather than by its session. +func serviceSession() bool { return false } diff --git a/internal/detector/browserext/session_windows.go b/internal/detector/browserext/session_windows.go new file mode 100644 index 00000000..4601660e --- /dev/null +++ b/internal/detector/browserext/session_windows.go @@ -0,0 +1,21 @@ +//go:build windows + +package browserext + +import "golang.org/x/sys/windows" + +// serviceSession reports whether this process runs in session 0, the +// non-interactive services session. It is the primary refusal predicate on +// Windows because a service under a custom or domain account carries an ordinary +// SID that no allowlist can enumerate, while its session is always 0. +// +// A failed lookup answers yes. The consequence of a wrong "no" is scanning the +// service profile, finding no browser, and reporting every browser missing — an +// authoritative answer that would retire the device's real inventory. +func serviceSession() bool { + var session uint32 + if err := windows.ProcessIdToSessionId(windows.GetCurrentProcessId(), &session); err != nil { + return true + } + return session == 0 +} diff --git a/internal/featuregate/featuregate.go b/internal/featuregate/featuregate.go index 26e58cb9..447ad974 100644 --- a/internal/featuregate/featuregate.go +++ b/internal/featuregate/featuregate.go @@ -25,6 +25,10 @@ const ( FeatureYarnConfigAudit Feature = "yarn-config-audit" FeatureDevicePolicy Feature = "device-policy" FeatureAgentSkillsScan Feature = "agent-skills-scan" + // Gated because the collector and its reader release independently: a reader + // that does not know this payload block discards it silently, which reads as + // "the feature is live and this machine has no browser extensions". + FeatureBrowserExtensionsScan Feature = "browser-extensions-scan" ) // enabled lists features safe to ship today. Uncomment a line once its @@ -38,6 +42,7 @@ var enabled = map[Feature]bool{ FeatureYarnConfigAudit: true, FeatureDevicePolicy: true, FeatureAgentSkillsScan: true, + // FeatureBrowserExtensionsScan: true, } var override bool diff --git a/internal/model/browserext.go b/internal/model/browserext.go new file mode 100644 index 00000000..6d11e9d0 --- /dev/null +++ b/internal/model/browserext.go @@ -0,0 +1,243 @@ +package model + +// Browser extension inventory wire types. +// +// A finding says which extension is installed in which browser, whether it is +// enabled and why not, where it came from, whether its store still lists it, and +// what it is permitted to touch. It carries no browsing state: no history, no +// cookies, no passwords, no page content, no profile identity, and no +// filesystem paths. Host-access patterns do ship, because they are what an +// extension can reach and can name an internal hostname, and that is the point +// of collecting them. +// +// Two things make the shape what it is. The first is that state is stored per +// extension rather than as one replaceable snapshot, so the payload has to say +// which browsers it speaks for — that is browsers[], and a reader deletes stored +// rows only for a browser whose entry claims a complete membership list. The +// second is that a browser whose extension set could not be fully enumerated +// ships no findings at all: half a list that read as complete would delete the +// unlisted half. + +// Coverage statuses, one per browser attempted. The distinction they draw is +// membership completeness — whether the set of extension identities reported for +// that browser is the whole set — and not whether every attribute parsed. +const ( + // Membership complete, every attribute parsed. Zero extensions is a real + // answer, not a missing one. + BrowserCoverageScanned = "scanned" + // Membership complete, one or more attributes degraded. Still authoritative, + // because an attribute is not an identity. + BrowserCoveragePartial = "partial" + // Membership NOT known complete. Findings for this browser are dropped before + // the payload is built, and a reader keeps whatever it already stored. + BrowserCoverageFailed = "failed" + // No data directory for this browser exists, or one exists holding no + // installation. Stored rows for it describe a browser that is gone. + BrowserCoverageNotPresent = "not_present" +) + +// Reason codes for a browser that could not be fully read. Typed rather than +// free text because a decoder's own message quotes the document it choked on, +// and these documents are the browser's private state. +const ( + BrowserExtReasonRefusedTCC = "refused_tcc" + BrowserExtReasonPermissionDenied = "permission_denied" + BrowserExtReasonParseError = "parse_error" + BrowserExtReasonUnsupportedEncoding = "unsupported_encoding" + BrowserExtReasonSymlinkRejected = "symlink_rejected" + BrowserExtReasonManifestUnavailable = "manifest_unavailable" + BrowserExtReasonCapped = "capped" + BrowserExtReasonTimedOut = "timed_out" +) + +// Enabled state. Enabled in any one profile counts as enabled: the extension can +// run on this machine. +const ( + BrowserExtEnabled = "enabled" + BrowserExtDisabled = "disabled" + BrowserExtStateUnknown = "unknown" +) + +// Who disabled it. Present exactly when the state is disabled, because the +// interesting case is not that an extension is off but that the browser turned +// it off. "Cannot tell" is spelled unknown rather than left out. +const ( + BrowserExtDisabledByUser = "user" + BrowserExtDisabledByBrowser = "browser" + BrowserExtDisabledByPolicy = "policy" + BrowserExtDisabledByUnknown = "unknown" +) + +// Whether the extension's store still lists it. Independent of enabled state: +// an extension the store has pulled while the machine still runs it, with its +// permissions granted, is the case this exists for. The browser's own cached +// belief, refreshed on its update ping, so delisted is a strong positive and +// listed is a weak negative. +const ( + BrowserExtStoreListingListed = "listed" + BrowserExtStoreListingDelisted = "delisted" + BrowserExtStoreListingUnknown = "unknown" +) + +// Whether the store recorded a policy violation against it. +const ( + BrowserExtStoreViolationNone = "none" + BrowserExtStoreViolationFlagged = "flagged" + BrowserExtStoreViolationUnknown = "unknown" +) + +// How the extension got installed. unpacked and sideload are the highest-signal +// values in the whole record and are never excluded. +const ( + BrowserExtInstallUser = "user" + BrowserExtInstallSideload = "sideload" + BrowserExtInstallRegistry = "registry" + BrowserExtInstallUnpacked = "unpacked" + BrowserExtInstallPolicy = "policy" + BrowserExtInstallUnknown = "unknown" +) + +// Which store the extension is attributed to. An enum and never a URL: a +// self-hosted update server's hostname is internal infrastructure. +const ( + BrowserExtStoreChromeWebStore = "chrome_web_store" + BrowserExtStoreEdgeAddons = "edge_addons" + BrowserExtStoreAMO = "amo" + BrowserExtStoreSelfHosted = "self_hosted" + BrowserExtStoreNone = "none" + BrowserExtStoreUnknown = "unknown" +) + +// Signing state, gecko only. An unsigned extension that is enabled is the +// headline security signal on that engine. +const ( + BrowserExtSignedBroken = "broken" + BrowserExtSignedUnknownChain = "unknown_chain" + BrowserExtSignedMissing = "missing" + BrowserExtSignedPreliminary = "preliminary" + BrowserExtSignedSigned = "signed" + BrowserExtSignedSystem = "system" + BrowserExtSignedPrivileged = "privileged" +) + +// Why the result was cut short. Set exactly when Truncated is, and never for a +// file that failed its own byte cap — that fails one browser rather than +// shortening the payload. +const ( + BrowserExtTruncatedFindingCap = "finding_cap" + BrowserExtTruncatedDeadline = "deadline" +) + +// CurrentBrowserExtensionSchemaVersion is this block's own shape version, so a +// reader can reject a shape it does not know instead of silently dropping the +// fields it has no home for. +const CurrentBrowserExtensionSchemaVersion = 1 + +// BrowserExtensionScanInfo is the browser extension inventory for one run. Its +// presence is the "scan ran" sentinel and that is load-bearing: nil means no +// information at all, while non-nil with zero findings and authoritative +// coverage means the machine really holds no extensions. A reader reconciles +// stored rows against a non-nil section, so an eagerly initialised struct where +// nil was meant erases a device's inventory. +type BrowserExtensionScanInfo struct { + PayloadSchemaVersion int `json:"payload_schema_version"` + + // The revision of the browser list probed. An identifier, not a quantity: + // nothing compares two arithmetically. + CatalogVersion string `json:"catalog_version"` + + CollectedAt int64 `json:"collected_at"` + + DurationMs int64 `json:"duration_ms"` + + // False when any browser failed or a cap cut the result. A degraded + // attribute does not clear it: partial coverage describes an attribute, and + // one extension whose metadata went missing must not mark the whole scan + // incomplete. + ScanComplete bool `json:"scan_complete"` + + Truncated bool `json:"truncated,omitempty"` + + TruncatedReason string `json:"truncated_reason,omitempty"` + + // One entry per catalog browser attempted on this platform. A browser this + // platform's catalog does not carry appears nowhere: not attempted is + // silence. + Browsers []BrowserCoverage `json:"browsers"` + + // One entry per (browser, extension). The same extension in three profiles + // is one finding. + Findings []BrowserExtensionFinding `json:"findings"` +} + +// BrowserCoverage is what one browser's attempt produced. It exists so stored +// rows can be reconciled per browser: without it a payload could not say +// whether zero findings for a browser means "nothing installed" or "could not +// look". +type BrowserCoverage struct { + BrowserID string `json:"browser_id"` + + Status string `json:"status"` + + // The browser's headline reason for being degraded, required for partial and + // failed and empty otherwise. One reason per browser: a browser that + // degraded twice reports the first cause rather than a list nothing reads. + ReasonCode string `json:"reason_code"` + + // Profiles enumerated across every data directory this browser has. The + // profiles themselves stay inside the detector — their names are + // user-chosen text and their identity was never the ask. + ProfileCount int `json:"profile_count"` + + // Extensions reported for this browser, after exclusions and deduplication. + // Zero for failed and not_present. It must equal the findings carrying this + // browser_id: a count that decorates rather than describes is worse than no + // count. + ExtensionCount int `json:"extension_count"` +} + +// BrowserExtensionFinding describes one extension in one browser. +type BrowserExtensionFinding struct { + BrowserID string `json:"browser_id"` + + // 32 characters of [a-p] on the Chromium family, the addon id on gecko. Never + // truncated: a shortened identity is a different identity, and a reader would + // treat it as a different extension. + ExtensionID string `json:"extension_id"` + + // May be empty. An extension whose metadata could not be recovered still has + // an identity, and reporting it without a name beats not reporting it. + Name string `json:"name"` + + Version string `json:"version"` + + EnabledState string `json:"enabled_state"` + + // Present exactly when EnabledState is disabled. + DisabledBy string `json:"disabled_by,omitempty"` + + // Present on Chromium-family findings only; gecko has no store-listing + // concept here. + StoreListing string `json:"store_listing,omitempty"` + StoreViolation string `json:"store_violation,omitempty"` + + InstallSource string `json:"install_source"` + + Store string `json:"store"` + + // The browser shipped it as a default. Still a real extension with real + // permissions, so it is reported; the flag is here so a console can + // de-emphasize it. + Preinstalled bool `json:"preinstalled"` + + // gecko only. + SignedState string `json:"signed_state,omitempty"` + + // The API permissions and host patterns the browser recorded as granted, + // including what the user granted at runtime — the honest upper bound of + // what the extension can reach, rather than what its manifest asked for. An + // entry too long for its cap is omitted rather than shortened: these strings + // are matched, so a shortened one is a different grant. + Permissions []string `json:"permissions"` + HostPermissions []string `json:"host_permissions"` +} diff --git a/internal/model/browserext_golden_test.go b/internal/model/browserext_golden_test.go new file mode 100644 index 00000000..4612e503 --- /dev/null +++ b/internal/model/browserext_golden_test.go @@ -0,0 +1,257 @@ +package model + +import ( + "bytes" + "encoding/json" + "os" + "reflect" + "testing" +) + +// browserExtGoldenPath holds one browser extension snapshot exercising every +// enum value a single valid payload can carry, plus the coverage combinations +// that are easy to get wrong: a failed browser with no findings, an +// authoritative browser with none, a reduced finding with no name. The same +// bytes are the contract the reader in the other repository is tested against. +const browserExtGoldenPath = "testdata/browser_extension_scan_golden.json" + +// browserExtGoldenGeckoIDs names the fixture's gecko browsers. The engine of a +// browser_id is catalog knowledge and this package is dependency-free, so the +// two engine-specific field rules are checked against the one gecko id the +// fixture uses. +var browserExtGoldenGeckoIDs = map[string]bool{"firefox": true} + +// TestBrowserExtensionScanGolden_RoundTripsWithNoDroppedField is the contract +// check between this struct and the reader on the other end of the wire. Both +// sides are hand-maintained Go types in separate repositories, and the reader +// drops a field it does not know rather than rejecting it — so a field renamed +// here does not fail anything, it silently stops arriving. +func TestBrowserExtensionScanGolden_RoundTripsWithNoDroppedField(t *testing.T) { + raw, err := os.ReadFile(browserExtGoldenPath) + if err != nil { + t.Fatalf("read golden: %v", err) + } + + // A field in the fixture this struct has no home for is a field this agent + // would never send, which is how the two shapes drift apart unnoticed. + var info BrowserExtensionScanInfo + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&info); err != nil { + t.Fatalf("golden payload does not fit BrowserExtensionScanInfo: %v", err) + } + + // A field decoded but not emitted back is the same drift the other way, so + // the comparison is on the re-encoded document rather than on the struct. + encoded, err := json.Marshal(&info) + if err != nil { + t.Fatalf("marshal: %v", err) + } + want := decodeGeneric(t, raw) + got := decodeGeneric(t, encoded) + if !reflect.DeepEqual(got, want) { + t.Errorf("round trip changed the payload\n got: %s\nwant: %s", encoded, raw) + } +} + +// TestBrowserExtensionScanGolden_CoversTheWholeVocabulary keeps the fixture +// honest. Its value is entirely in what it exercises, so one that has quietly +// stopped covering a state is worse than none: it passes while the field it +// protects goes unchecked. +func TestBrowserExtensionScanGolden_CoversTheWholeVocabulary(t *testing.T) { + info := loadBrowserExtGolden(t) + + statuses := map[string]bool{} + for _, b := range info.Browsers { + statuses[b.Status] = true + } + states, disabledBy, listings, violations := map[string]bool{}, map[string]bool{}, map[string]bool{}, map[string]bool{} + sources, stores, signed := map[string]bool{}, map[string]bool{}, map[string]bool{} + for _, f := range info.Findings { + states[f.EnabledState] = true + stores[f.Store] = true + sources[f.InstallSource] = true + if f.DisabledBy != "" { + disabledBy[f.DisabledBy] = true + } + if f.StoreListing != "" { + listings[f.StoreListing] = true + } + if f.StoreViolation != "" { + violations[f.StoreViolation] = true + } + if f.SignedState != "" { + signed[f.SignedState] = true + } + } + + for _, tt := range []struct { + what string + got map[string]bool + want []string + }{ + {"status", statuses, []string{ + BrowserCoverageScanned, BrowserCoveragePartial, + BrowserCoverageFailed, BrowserCoverageNotPresent, + }}, + {"enabled_state", states, []string{ + BrowserExtEnabled, BrowserExtDisabled, BrowserExtStateUnknown, + }}, + {"disabled_by", disabledBy, []string{ + BrowserExtDisabledByUser, BrowserExtDisabledByBrowser, + BrowserExtDisabledByPolicy, BrowserExtDisabledByUnknown, + }}, + {"store_listing", listings, []string{ + BrowserExtStoreListingListed, BrowserExtStoreListingDelisted, + BrowserExtStoreListingUnknown, + }}, + {"store_violation", violations, []string{ + BrowserExtStoreViolationNone, BrowserExtStoreViolationFlagged, + BrowserExtStoreViolationUnknown, + }}, + {"install_source", sources, []string{ + BrowserExtInstallUser, BrowserExtInstallSideload, BrowserExtInstallRegistry, + BrowserExtInstallUnpacked, BrowserExtInstallPolicy, BrowserExtInstallUnknown, + }}, + {"store", stores, []string{ + BrowserExtStoreChromeWebStore, BrowserExtStoreEdgeAddons, BrowserExtStoreAMO, + BrowserExtStoreSelfHosted, BrowserExtStoreNone, BrowserExtStoreUnknown, + }}, + {"signed_state", signed, []string{ + BrowserExtSignedBroken, BrowserExtSignedUnknownChain, BrowserExtSignedMissing, + BrowserExtSignedPreliminary, BrowserExtSignedSigned, BrowserExtSignedSystem, + BrowserExtSignedPrivileged, + }}, + } { + for _, want := range tt.want { + if !tt.got[want] { + t.Errorf("golden payload has no %s %q", tt.what, want) + } + } + } + + // The two closed sets a single valid payload cannot demonstrate. A browser + // carries one reason code and a payload one truncated reason, so the fixture + // can only ever show a couple of each — while a reader matches every one of + // them against a list it maintains separately. Spelled out here so a renamed + // or dropped code fails at home rather than as a rejected payload. + for _, tt := range []struct{ got, want string }{ + {BrowserExtReasonRefusedTCC, "refused_tcc"}, + {BrowserExtReasonPermissionDenied, "permission_denied"}, + {BrowserExtReasonParseError, "parse_error"}, + {BrowserExtReasonUnsupportedEncoding, "unsupported_encoding"}, + {BrowserExtReasonSymlinkRejected, "symlink_rejected"}, + {BrowserExtReasonManifestUnavailable, "manifest_unavailable"}, + {BrowserExtReasonCapped, "capped"}, + {BrowserExtReasonTimedOut, "timed_out"}, + {BrowserExtTruncatedFindingCap, "finding_cap"}, + {BrowserExtTruncatedDeadline, "deadline"}, + } { + if tt.got != tt.want { + t.Errorf("code is spelled %q, and a reader matches %q", tt.got, tt.want) + } + } + + // A reduced finding: metadata that could not be recovered, identity that + // could. A reader that requires a name drops a real extension on this row. + reduced := false + for _, f := range info.Findings { + if f.Name == "" && f.ExtensionID != "" { + reduced = true + } + } + if !reduced { + t.Error("golden payload must carry a finding whose name is empty and whose identity is not") + } + + // Both incompleteness flags together, and the reason for the second: a + // reader validates the pair in both directions. + if info.ScanComplete || !info.Truncated || info.TruncatedReason == "" { + t.Error("golden payload must exercise an incomplete, truncated snapshot with its reason") + } + if info.CatalogVersion == "" { + t.Error("golden payload must declare a catalog_version") + } + if info.PayloadSchemaVersion != CurrentBrowserExtensionSchemaVersion { + t.Errorf("golden payload declares schema %d, want %d", + info.PayloadSchemaVersion, CurrentBrowserExtensionSchemaVersion) + } +} + +// TestBrowserExtensionScanGolden_HonoursTheCoverageInvariants checks the rules a +// reader rejects the whole block over. A fixture that violates one would be +// accepted here and refused there, which is the worst possible fixture: it +// proves the contract while breaking it. +func TestBrowserExtensionScanGolden_HonoursTheCoverageInvariants(t *testing.T) { + info := loadBrowserExtGolden(t) + + perBrowser := map[string]int{} + for _, f := range info.Findings { + perBrowser[f.BrowserID]++ + } + + seen := map[string]bool{} + for _, b := range info.Browsers { + if seen[b.BrowserID] { + t.Errorf("%s: second coverage entry for one browser", b.BrowserID) + } + seen[b.BrowserID] = true + + switch b.Status { + case BrowserCoveragePartial, BrowserCoverageFailed: + if b.ReasonCode == "" { + t.Errorf("%s: status %q needs a reason_code", b.BrowserID, b.Status) + } + default: + if b.ReasonCode != "" { + t.Errorf("%s: status %q must carry no reason_code, got %q", b.BrowserID, b.Status, b.ReasonCode) + } + } + // failed ships nothing: a partial list under an authoritative status + // would delete the extensions it left out. + if b.Status == BrowserCoverageFailed || b.Status == BrowserCoverageNotPresent { + if b.ExtensionCount != 0 || perBrowser[b.BrowserID] != 0 { + t.Errorf("%s: status %q must carry zero findings, got count=%d findings=%d", + b.BrowserID, b.Status, b.ExtensionCount, perBrowser[b.BrowserID]) + } + continue + } + if b.ExtensionCount != perBrowser[b.BrowserID] { + t.Errorf("%s: extension_count = %d, findings = %d", b.BrowserID, b.ExtensionCount, perBrowser[b.BrowserID]) + } + } + + for _, f := range info.Findings { + if !seen[f.BrowserID] { + t.Errorf("%s: finding for a browser with no coverage entry", f.BrowserID) + } + if f.ExtensionID == "" { + t.Error("finding with no identity") + } + // The cause is present exactly when there is something to explain. + if (f.EnabledState == BrowserExtDisabled) != (f.DisabledBy != "") { + t.Errorf("%s: enabled_state %q with disabled_by %q", f.ExtensionID, f.EnabledState, f.DisabledBy) + } + gecko := browserExtGoldenGeckoIDs[f.BrowserID] + if gecko != (f.SignedState != "") { + t.Errorf("%s: signed_state %q on browser %q", f.ExtensionID, f.SignedState, f.BrowserID) + } + if gecko == (f.StoreListing != "") || gecko == (f.StoreViolation != "") { + t.Errorf("%s: store fields %q/%q on browser %q", + f.ExtensionID, f.StoreListing, f.StoreViolation, f.BrowserID) + } + } +} + +func loadBrowserExtGolden(t *testing.T) BrowserExtensionScanInfo { + t.Helper() + raw, err := os.ReadFile(browserExtGoldenPath) + if err != nil { + t.Fatalf("read golden: %v", err) + } + var info BrowserExtensionScanInfo + if err := json.Unmarshal(raw, &info); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return info +} diff --git a/internal/model/model.go b/internal/model/model.go index 6418f1fe..b4418ace 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -43,6 +43,12 @@ type ScanResult struct { // non-nil section with zero findings means it ran and found nothing. CredentialScan *CredentialScanInfo `json:"credential_scan,omitempty"` + // BrowserExtensionScan is the browser extension inventory. Nil means the + // phase did not run — the only "no information" signal a reader has, and the + // difference between it and a section carrying zero findings is what keeps a + // skipped scan from erasing a device's extensions. + BrowserExtensionScan *BrowserExtensionScanInfo `json:"browser_extension_scan,omitempty"` + Summary Summary `json:"summary"` } diff --git a/internal/model/scanresult_jsonshape_test.go b/internal/model/scanresult_jsonshape_test.go index 62d27183..eeb10e30 100644 --- a/internal/model/scanresult_jsonshape_test.go +++ b/internal/model/scanresult_jsonshape_test.go @@ -38,3 +38,18 @@ func TestScanResult_CredentialScan_OmittedWhenNil(t *testing.T) { t.Errorf("zero ScanResult should omit \"credential_scan\", got: %s", s) } } + +// TestScanResult_BrowserExtensionScan_OmittedWhenNil guards the same signal for +// browser extensions, where it carries more weight: a reader reconciles stored +// per-browser rows against any section it receives, so a section rendered when +// nobody scanned reads as "this machine has no extensions" and deletes the rows +// a real scan wrote. +func TestScanResult_BrowserExtensionScan_OmittedWhenNil(t *testing.T) { + b, err := json.Marshal(&ScanResult{}) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if s := string(b); strings.Contains(s, `"browser_extension_scan"`) { + t.Errorf("zero ScanResult should omit \"browser_extension_scan\", got: %s", s) + } +} diff --git a/internal/model/testdata/browser_extension_scan_golden.json b/internal/model/testdata/browser_extension_scan_golden.json new file mode 100644 index 00000000..b4bf5be5 --- /dev/null +++ b/internal/model/testdata/browser_extension_scan_golden.json @@ -0,0 +1,224 @@ +{ + "payload_schema_version": 1, + "catalog_version": "1", + "collected_at": 1754870400, + "duration_ms": 412, + "scan_complete": false, + "truncated": true, + "truncated_reason": "deadline", + "browsers": [ + { + "browser_id": "chrome", + "status": "partial", + "reason_code": "manifest_unavailable", + "profile_count": 2, + "extension_count": 6 + }, + { + "browser_id": "edge", + "status": "failed", + "reason_code": "timed_out", + "profile_count": 0, + "extension_count": 0 + }, + { + "browser_id": "brave", + "status": "not_present", + "reason_code": "", + "profile_count": 0, + "extension_count": 0 + }, + { + "browser_id": "firefox", + "status": "scanned", + "reason_code": "", + "profile_count": 1, + "extension_count": 7 + } + ], + "findings": [ + { + "browser_id": "chrome", + "extension_id": "abcdefghijklmnopabcdefghijklmnop", + "name": "Example Password Manager", + "version": "3.14.2", + "enabled_state": "enabled", + "store_listing": "listed", + "store_violation": "none", + "install_source": "user", + "store": "chrome_web_store", + "preinstalled": false, + "permissions": ["storage", "tabs"], + "host_permissions": ["https://*/*", "http://*/*"] + }, + { + "browser_id": "chrome", + "extension_id": "bcdefghijklmnopabcdefghijklmnopa", + "name": "Example Shopping Helper", + "version": "1.0.0", + "enabled_state": "disabled", + "disabled_by": "user", + "store_listing": "listed", + "store_violation": "none", + "install_source": "sideload", + "store": "self_hosted", + "preinstalled": true, + "permissions": ["cookies"], + "host_permissions": ["*://example.internal/*"] + }, + { + "browser_id": "chrome", + "extension_id": "cdefghijklmnopabcdefghijklmnopab", + "name": "", + "version": "", + "enabled_state": "disabled", + "disabled_by": "browser", + "store_listing": "delisted", + "store_violation": "flagged", + "install_source": "unpacked", + "store": "unknown", + "preinstalled": false, + "permissions": [], + "host_permissions": [] + }, + { + "browser_id": "chrome", + "extension_id": "defghijklmnopabcdefghijklmnopabc", + "name": "Example Corporate Toolbar", + "version": "7.0", + "enabled_state": "disabled", + "disabled_by": "policy", + "store_listing": "unknown", + "store_violation": "unknown", + "install_source": "policy", + "store": "edge_addons", + "preinstalled": false, + "permissions": ["management"], + "host_permissions": [""] + }, + { + "browser_id": "chrome", + "extension_id": "efghijklmnopabcdefghijklmnopabcd", + "name": "Example Vendor Agent", + "version": "0.9.1", + "enabled_state": "disabled", + "disabled_by": "unknown", + "store_listing": "listed", + "store_violation": "none", + "install_source": "registry", + "store": "chrome_web_store", + "preinstalled": false, + "permissions": [], + "host_permissions": [] + }, + { + "browser_id": "chrome", + "extension_id": "fghijklmnopabcdefghijklmnopabcde", + "name": "Example Unreadable State", + "version": "2.2.2", + "enabled_state": "unknown", + "store_listing": "unknown", + "store_violation": "unknown", + "install_source": "unknown", + "store": "unknown", + "preinstalled": false, + "permissions": [], + "host_permissions": [] + }, + { + "browser_id": "firefox", + "extension_id": "content-filter@example-org", + "name": "Example Content Filter", + "version": "1.55.0", + "enabled_state": "enabled", + "install_source": "user", + "store": "amo", + "preinstalled": false, + "signed_state": "signed", + "permissions": ["webRequest"], + "host_permissions": [""] + }, + { + "browser_id": "firefox", + "extension_id": "{d1bf0192-b0f4-4a5c-9b0f-0c9b0f0a1b2c}", + "name": "Example Local Build", + "version": "0.0.1", + "enabled_state": "enabled", + "install_source": "unpacked", + "store": "none", + "preinstalled": false, + "signed_state": "missing", + "permissions": [], + "host_permissions": [] + }, + { + "browser_id": "firefox", + "extension_id": "beta-preview@example-org", + "name": "Example Beta Preview", + "version": "2.0.0b1", + "enabled_state": "disabled", + "disabled_by": "user", + "install_source": "user", + "store": "amo", + "preinstalled": false, + "signed_state": "preliminary", + "permissions": ["storage"], + "host_permissions": [] + }, + { + "browser_id": "firefox", + "extension_id": "damaged-package@example-org", + "name": "Example Damaged Package", + "version": "4.1.0", + "enabled_state": "disabled", + "disabled_by": "browser", + "install_source": "sideload", + "store": "unknown", + "preinstalled": false, + "signed_state": "broken", + "permissions": [], + "host_permissions": [] + }, + { + "browser_id": "firefox", + "extension_id": "unverified-chain@example-org", + "name": "Example Unverified Chain", + "version": "1.2.3", + "enabled_state": "disabled", + "disabled_by": "unknown", + "install_source": "registry", + "store": "unknown", + "preinstalled": false, + "signed_state": "unknown_chain", + "permissions": [], + "host_permissions": [] + }, + { + "browser_id": "firefox", + "extension_id": "vendor-system-addon@example-org", + "name": "Example Vendor System Addon", + "version": "9.0", + "enabled_state": "enabled", + "install_source": "policy", + "store": "amo", + "preinstalled": true, + "signed_state": "system", + "permissions": [], + "host_permissions": [] + }, + { + "browser_id": "firefox", + "extension_id": "privileged-tooling@example-org", + "name": "Example Privileged Tooling", + "version": "5.5.5", + "enabled_state": "disabled", + "disabled_by": "policy", + "install_source": "unknown", + "store": "unknown", + "preinstalled": false, + "signed_state": "privileged", + "permissions": [], + "host_permissions": [] + } + ] +} diff --git a/internal/output/html.go b/internal/output/html.go index dcb22e4e..16d478dc 100644 --- a/internal/output/html.go +++ b/internal/output/html.go @@ -28,7 +28,10 @@ type htmlData struct { PythonProjects []model.ProjectInfo AgentSkills []model.AgentSkill AgentSkillScan *model.AgentSkillScanInfo - Summary model.Summary + // Nil means the phase did not run, which the template shows differently from a + // scan that ran and found nothing. + BrowserExtensionScan *model.BrowserExtensionScanInfo + Summary model.Summary } func typeLabel(t string) string { @@ -72,7 +75,9 @@ func HTML(outputFile string, result *model.ScanResult) error { PythonProjects: result.PythonProjects, AgentSkills: result.AgentSkills, AgentSkillScan: result.AgentSkillScan, - Summary: result.Summary, + + BrowserExtensionScan: result.BrowserExtensionScan, + Summary: result.Summary, } funcMap := template.FuncMap{ @@ -271,6 +276,25 @@ const htmlTemplate = ` +
+
+

Browser Extensions {{if .BrowserExtensionScan}}{{len .BrowserExtensionScan.Findings}}{{else}}0{{end}}

+ +
+ +
+

IDE Extensions {{.Summary.IDEExtensionsCount}}

diff --git a/internal/output/pretty.go b/internal/output/pretty.go index 721dabdb..e22e9fd3 100644 --- a/internal/output/pretty.go +++ b/internal/output/pretty.go @@ -174,6 +174,9 @@ func Pretty(w io.Writer, result *model.ScanResult, colorMode string) error { } fmt.Fprintln(w) + // BROWSER EXTENSIONS + printBrowserExtensions(w, c, result) + // NODE.JS PACKAGE MANAGERS (only if npm scan was enabled) if len(result.NodePkgManagers) > 0 { printSectionHeader(w, c, "NODE.JS PACKAGE MANAGERS", len(result.NodePkgManagers)) @@ -434,7 +437,62 @@ func printPipAuditSummary(w io.Writer, c *colors, a *model.PipAudit) { fmt.Fprintln(w) } +// printBrowserExtensions renders the browser extension inventory in three states, +// because two of them are easy to confuse and mean opposite things: the phase not +// having run at all, the phase having run and found nothing, and a list. Each row +// leads with what a reader acts on — a browser that turned an extension off, or a +// store that has pulled one the machine still runs. +// //nolint:errcheck // terminal output +func printBrowserExtensions(w io.Writer, c *colors, result *model.ScanResult) { + scan := result.BrowserExtensionScan + count := 0 + if scan != nil { + count = len(scan.Findings) + } + printSectionHeader(w, c, "BROWSER EXTENSIONS", count) + + switch { + case scan == nil: + fmt.Fprintf(w, " %sNot scanned%s\n", c.dim, c.reset) + case count == 0: + fmt.Fprintf(w, " %sNone detected%s\n", c.dim, c.reset) + default: + for _, f := range scan.Findings { + tag := "" + if f.EnabledState != model.BrowserExtEnabled { + tag = " [" + f.EnabledState + if f.DisabledBy != "" { + tag += " by " + f.DisabledBy + } + tag += "]" + } + if f.StoreListing == model.BrowserExtStoreListingDelisted { + tag += " [delisted]" + } + name := f.Name + if name == "" { + // A finding whose metadata could not be recovered still has an + // identity, and that is what a reader needs to look it up. + name = f.ExtensionID + } + fmt.Fprintf(w, " %-30s %s%-10s %-12s %s%s%s\n", + truncate(name, 30), c.dim, truncate(f.BrowserID, 10), + truncate(f.InstallSource, 12), truncate(f.Version, 12), tag, c.reset) + } + } + // Coverage is part of the answer, not a footnote: a browser that could not be + // read is why a list is shorter than a user expects. + if scan != nil { + for _, b := range scan.Browsers { + if b.Status == model.BrowserCoverageFailed || b.Status == model.BrowserCoveragePartial { + fmt.Fprintf(w, " %s%s: %s (%s)%s\n", c.dim, b.BrowserID, b.Status, b.ReasonCode, c.reset) + } + } + } + fmt.Fprintln(w) +} + func printSectionHeader(w io.Writer, c *colors, title string, count int) { padding := 35 - len(title) if padding < 1 { diff --git a/internal/output/pretty_test.go b/internal/output/pretty_test.go index d212c995..bd258a68 100644 --- a/internal/output/pretty_test.go +++ b/internal/output/pretty_test.go @@ -238,3 +238,76 @@ func TestIdeDisplayName(t *testing.T) { } } } + +// TestPretty_BrowserExtensionsTriState covers the distinction the section exists +// to draw. "Not scanned" and "None detected" look alike and mean opposite things: +// one is no information, the other is the machine positively holding nothing. +func TestPretty_BrowserExtensionsTriState(t *testing.T) { + tests := []struct { + name string + scan *model.BrowserExtensionScanInfo + want string + notWant string + }{ + { + name: "the phase did not run", + scan: nil, + want: "Not scanned", + notWant: "None detected", + }, + { + name: "it ran and the machine holds none", + scan: &model.BrowserExtensionScanInfo{ + Browsers: []model.BrowserCoverage{{BrowserID: "chrome", Status: model.BrowserCoverageNotPresent}}, + }, + want: "None detected", + notWant: "Not scanned", + }, + { + name: "a delisted extension is called out", + scan: &model.BrowserExtensionScanInfo{ + Browsers: []model.BrowserCoverage{{BrowserID: "chrome", Status: model.BrowserCoverageScanned, ExtensionCount: 1}}, + Findings: []model.BrowserExtensionFinding{{ + BrowserID: "chrome", + ExtensionID: "abcdefghijklmnopabcdefghijklmnop", + Name: "Example Screen Capture", + Version: "8.6", + EnabledState: model.BrowserExtDisabled, + DisabledBy: model.BrowserExtDisabledByBrowser, + StoreListing: model.BrowserExtStoreListingDelisted, + }}, + }, + want: "delisted", + notWant: "Not scanned", + }, + { + name: "a browser that could not be read is shown, not hidden", + scan: &model.BrowserExtensionScanInfo{ + Browsers: []model.BrowserCoverage{{ + BrowserID: "brave", + Status: model.BrowserCoverageFailed, + ReasonCode: model.BrowserExtReasonSymlinkRejected, + }}, + }, + want: model.BrowserExtReasonSymlinkRejected, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + _ = Pretty(&buf, &model.ScanResult{BrowserExtensionScan: tc.scan}, "never") + + out := buf.String() + if !strings.Contains(out, "BROWSER EXTENSIONS") { + t.Fatal("output has no browser extension section") + } + section := out[strings.Index(out, "BROWSER EXTENSIONS"):] + if !strings.Contains(section, tc.want) { + t.Errorf("section does not mention %q:\n%s", tc.want, section) + } + if tc.notWant != "" && strings.Contains(section, tc.notWant) { + t.Errorf("section wrongly mentions %q:\n%s", tc.notWant, section) + } + }) + } +} diff --git a/internal/safepath/open_unix.go b/internal/safepath/open_unix.go index dd3adc86..3fe9d244 100644 --- a/internal/safepath/open_unix.go +++ b/internal/safepath/open_unix.go @@ -19,7 +19,11 @@ import ( // metadata comes back with the handle because it is read from the handle: a // caller that stat'd the path instead would be describing whatever is at that // name now, not what it is holding open. -func openVerified(resolved string, wantDir bool) (*os.File, os.FileInfo, error) { +// +// noFollow only changes how a rejected component is reported: the open is +// no-follow either way, so a resolver that tolerates links sees a swap as an +// unresolvable path while one that tolerates none sees the symlink it refuses. +func openVerified(resolved string, wantDir, noFollow bool) (*os.File, os.FileInfo, error) { _, comps := split(resolved) if len(comps) == 0 { return nil, nil, refuse(ReasonUnresolved) @@ -42,7 +46,7 @@ func openVerified(resolved string, wantDir bool) (*os.File, os.FileInfo, error) for _, comp := range comps[:len(comps)-1] { next, oerr := unix.Openat(dirfd, comp, dirFlags, 0) if oerr != nil { - return nil, nil, openErr(oerr) + return nil, nil, openErr(oerr, noFollow) } _ = unix.Close(dirfd) dirfd = next @@ -57,7 +61,7 @@ func openVerified(resolved string, wantDir bool) (*os.File, os.FileInfo, error) // would return nothing useful either way. fd, err := unix.Openat(dirfd, comps[len(comps)-1], leafFlags|unix.O_NONBLOCK, 0) if err != nil { - return nil, nil, openErr(err) + return nil, nil, openErr(err, noFollow) } // #nosec G115 -- fd is the descriptor openat just returned on success: a small // non-negative int, which is what uintptr carries for the rest of its life. A @@ -74,9 +78,14 @@ func openVerified(resolved string, wantDir bool) (*os.File, os.FileInfo, error) // openErr maps an openat failure to a refusal. ELOOP means O_NOFOLLOW hit a // symlink where resolution had seen a real directory or file — the component // changed underneath us, so the read is abandoned rather than retried. -func openErr(err error) error { +func openErr(err error, noFollow bool) error { switch err { - case unix.ELOOP, unix.EMLINK, unix.ENOTDIR: + case unix.ELOOP, unix.EMLINK: + if noFollow { + return refuse(ReasonSymlink) + } + return refuse(ReasonUnresolved) + case unix.ENOTDIR: return refuse(ReasonUnresolved) case unix.ENOENT: return os.ErrNotExist diff --git a/internal/safepath/open_windows.go b/internal/safepath/open_windows.go index ac5be750..31a3eb6c 100644 --- a/internal/safepath/open_windows.go +++ b/internal/safepath/open_windows.go @@ -21,7 +21,11 @@ import ( // resolution and open. The verification runs against the same handle the content is // read from, leaving no second window to swap into. There is no consent layer here, // so a mismatch prevents reading the wrong file rather than a blocked traversal. -func openVerified(resolved string, wantDir bool) (*os.File, os.FileInfo, error) { +// +// noFollow only changes how a rejected component is reported; a resolver that +// follows nothing has already refused a reparse point it saw during resolution, +// and this reports the ones that appear after it. +func openVerified(resolved string, wantDir, noFollow bool) (*os.File, os.FileInfo, error) { if _, comps := split(resolved); len(comps) == 0 { return nil, nil, refuse(ReasonUnresolved) } @@ -43,7 +47,7 @@ func openVerified(resolved string, wantDir bool) (*os.File, os.FileInfo, error) 0, ) if err != nil { - return nil, nil, openErr(err) + return nil, nil, openErr(err, noFollow) } final, err := finalPath(h) @@ -97,8 +101,13 @@ func finalPath(h windows.Handle) (string, error) { } } p := windows.UTF16ToString(buf) - p = strings.TrimPrefix(p, `\\?\`) - return p, nil + // A share comes back as \\?\UNC\server\share, whose ordinary spelling is + // \\server\share. Trimming the extended-length prefix alone would leave a path + // starting with the literal UNC, which matches nothing the caller resolved. + if rest, ok := strings.CutPrefix(p, `\\?\UNC\`); ok { + return `\\` + rest, nil + } + return strings.TrimPrefix(p, `\\?\`), nil } // samePath compares the kernel's path for the handle with the path that was @@ -111,13 +120,16 @@ func samePath(a, b string) bool { } // openErr maps a CreateFile failure to a refusal. -func openErr(err error) error { +func openErr(err error, noFollow bool) error { switch err { case windows.ERROR_FILE_NOT_FOUND, windows.ERROR_PATH_NOT_FOUND: return os.ErrNotExist case windows.ERROR_ACCESS_DENIED, windows.ERROR_SHARING_VIOLATION: return refuse(ReasonDenied) case windows.ERROR_CANT_ACCESS_FILE, windows.ERROR_INVALID_REPARSE_DATA: + if noFollow { + return refuse(ReasonSymlink) + } return refuse(ReasonUnresolved) default: return refuse(ReasonDenied) diff --git a/internal/safepath/safepath.go b/internal/safepath/safepath.go index cf7029fc..cf89fe62 100644 --- a/internal/safepath/safepath.go +++ b/internal/safepath/safepath.go @@ -37,6 +37,7 @@ import ( "io" "os" "path/filepath" + "runtime" "strings" "sync" ) @@ -53,6 +54,11 @@ const ( // concrete file: a symlink cycle, more hops than the budget allows, a // relative path, or a component swapped between resolution and open. ReasonUnresolved = "location_unresolved" + // ReasonSymlink is a symlink on the path, reported only by a resolver that + // follows none. A caller that reads a fixed set of directories owned by one + // application has no legitimate link to follow, and refusing is the only + // answer that cannot be talked into reading somewhere else. + ReasonSymlink = "symlink_rejected" ) // maxHops bounds symlink indirection. Each hop restarts resolution at the @@ -98,6 +104,9 @@ type Guard func(path string) string type Resolver struct { home string guard Guard + // noFollow refuses a path with a symlink anywhere on it instead of resolving + // the link. See NewNoFollow. + noFollow bool // rootsOnce resolves the containment roots once: the home as written plus its own // resolved form. A home behind a symlink is an ordinary layout, and containment @@ -115,6 +124,29 @@ func New(home string, guard Guard) *Resolver { return &Resolver{home: home, guard: guard} } +// NewNoFollow returns a Resolver that refuses any path with a symlink on it — +// above or below the root, leaf or ancestor — rather than resolving the link and +// deciding about its target. +// +// It is for a caller whose paths are all fixed locations owned by one +// application: there is no legitimate link on such a path, so a link is either a +// layout this build does not support or an attempt to redirect a privileged read, +// and the two are indistinguishable from the outside. Refusing is also the only +// rule with no exempt path class to reason about, which is what makes it +// auditable. A caller that must tolerate a symlinked home or a dotfile-managed +// directory wants New instead. +// +// The strength of the refusal follows the platform's open. On unix every +// component is opened with O_NOFOLLOW, so no link on the path is ever traversed. +// Windows has no openat: the leaf is opened without following a reparse point and +// the handle's own final path is compared against the resolved one, so a junction +// swapped in above the leaf is caught after the traversal rather than before it. +func NewNoFollow(home string, guard Guard) *Resolver { + r := New(home, guard) + r.noFollow = true + return r +} + // containmentRoots returns the home as written plus its resolved form, computed // once and reused: Contains runs at the end of every resolution, so it must not // rebuild the list or re-resolve the home each time. @@ -140,16 +172,29 @@ func (r *Resolver) containmentRoots() []string { func (r *Resolver) Contains(path string) bool { cleaned := filepath.Clean(path) for _, root := range r.containmentRoots() { - if cleaned == root { + if pathEqual(cleaned, root) { return true } - if strings.HasPrefix(cleaned, root) && cleaned[len(root)] == filepath.Separator { + if len(cleaned) > len(root) && cleaned[len(root)] == filepath.Separator && + pathEqual(cleaned[:len(root)], root) { return true } } return false } +// pathEqual compares two paths the way the filesystem underneath them resolves +// names. Windows matches without regard to case, so a root taken from the account +// record and a path taken from an application's own config file can name the same +// directory in different casing; comparing byte for byte there refuses a file the +// user owns. Elsewhere case is part of the name. +func pathEqual(a, b string) bool { + if runtime.GOOS == "windows" { + return strings.EqualFold(a, b) + } + return a == b +} + // Resolve returns the concrete path that path refers to, following symlinks // component by component without ever letting the kernel do the following. It // returns a *Refusal when the guard declines the target, when it escapes the root, @@ -170,6 +215,14 @@ func (r *Resolver) resolveChain(path string, checkRoots bool) (string, os.FileIn return "", nil, refuse(ReasonUnresolved) } current := filepath.Clean(path) + if checkRoots && r.noFollow && !r.Contains(current) { + // Nothing on this path may redirect, so where it lies is where it ends and + // the lexical answer is the final one. Refusing here rather than at the end + // of the walk is what keeps a target outside the roots from being stat'd on + // the way down — a component on a dead network mount blocks whether or not + // the path would have been refused once reached. + return "", nil, refuse(ReasonOutsideRoots) + } for hop := 0; ; hop++ { if hop > maxHops { @@ -206,6 +259,12 @@ func (r *Resolver) resolveChain(path string, checkRoots bool) (string, os.FileIn leaf = info continue } + if r.noFollow { + // Advisory: the open below refuses the same component without a + // window to swap in. Catching it here keeps the refusal specific, + // since the open reports one code for every redirection. + return "", nil, refuse(ReasonSymlink) + } target, err := os.Readlink(prefix) if err != nil { return "", nil, refuse(ReasonUnresolved) @@ -287,7 +346,7 @@ func (r *Resolver) Read(path string, max int64) (data []byte, resolved string, i if err != nil { return nil, "", nil, false, err } - f, info, err := openVerified(resolved, false) + f, info, err := openVerified(resolved, false, r.noFollow) if err != nil { return nil, "", nil, false, err } @@ -319,7 +378,7 @@ func (r *Resolver) ReadDirNames(path string, max int) (names []string, resolved if err != nil { return nil, "", false, err } - f, _, err := openVerified(resolved, true) + f, _, err := openVerified(resolved, true, r.noFollow) if err != nil { return nil, "", false, err } diff --git a/internal/safepath/safepath_test.go b/internal/safepath/safepath_test.go index 689326e0..98b3e3d2 100644 --- a/internal/safepath/safepath_test.go +++ b/internal/safepath/safepath_test.go @@ -335,6 +335,79 @@ func TestResolve_Guard(t *testing.T) { }) } +// TestNewNoFollow_RefusesEverySymlinkOnThePath covers the stricter resolver: for a +// caller whose paths are all fixed locations owned by one application there is no +// legitimate link to follow, so a link is refused wherever it sits rather than +// resolved and judged by its target. The refusal carries its own reason, because +// the caller reports an unsupported layout differently from a denied read. +func TestNewNoFollow_RefusesEverySymlinkOnThePath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation needs a privilege the test host may not hold") + } + tests := []struct { + name string + // setup returns the path to read, having planted a link on it. + setup func(t *testing.T, home string) string + }{ + { + name: "the leaf is a link", + setup: func(t *testing.T, home string) string { + writeFile(t, filepath.Join(home, "real", "state"), "{}\n") + link := filepath.Join(home, "app", "state") + if err := os.MkdirAll(filepath.Dir(link), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + symlink(t, filepath.Join(home, "real", "state"), link) + return link + }, + }, + { + name: "a directory above it is a link", + setup: func(t *testing.T, home string) string { + writeFile(t, filepath.Join(home, "real", "state"), "{}\n") + symlink(t, filepath.Join(home, "real"), filepath.Join(home, "app")) + return filepath.Join(home, "app", "state") + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := tempHome(t) + path := tc.setup(t, home) + + if _, err := NewNoFollow(home, nil).Resolve(path); ReasonOf(err) != ReasonSymlink { + t.Errorf("reason = %q (err %v), want %q", ReasonOf(err), err, ReasonSymlink) + } + // The ordinary resolver still follows it: the two modes differ only + // in this, and the contained target is a layout it supports. + if _, err := New(home, nil).Resolve(path); err != nil { + t.Errorf("the following resolver refused a contained target: %v", err) + } + }) + } +} + +// TestNewNoFollow_RefusesOutsideTheRootBeforeTouchingIt pins when containment is +// decided. A resolver that follows nothing cannot be redirected back into the +// root, so where a path lies is where it ends and the answer is available before +// the first syscall — which is the point: walking to a target on a dead network +// mount to discover it was never allowed blocks on the way there. A path that does +// not exist is the case that separates the two orders, since walking to it returns +// "missing" and deciding first returns "refused". +func TestNewNoFollow_RefusesOutsideTheRootBeforeTouchingIt(t *testing.T) { + home := tempHome(t) + outside := filepath.Join(t.TempDir(), "elsewhere", "state") + + if _, err := NewNoFollow(home, nil).Resolve(outside); ReasonOf(err) != ReasonOutsideRoots { + t.Errorf("reason = %q (err %v), want %q", ReasonOf(err), err, ReasonOutsideRoots) + } + // Inside the root the answer is unchanged: absence is not a refusal. + missing := filepath.Join(home, "app", "state") + if _, err := NewNoFollow(home, nil).Resolve(missing); !errors.Is(err, os.ErrNotExist) { + t.Errorf("err = %v, want a missing file inside the root reported as missing", err) + } +} + // A parent component swapped for a symlink between resolution and open must not // redirect the read — the property O_NOFOLLOW on the leaf alone does not deliver: // every directory above the leaf was validated by an earlier syscall, so a local @@ -364,7 +437,7 @@ func TestOpenVerified_RefusesAComponentSwappedAfterResolution(t *testing.T) { } symlink(t, elsewhere, real) - f, _, err := openVerified(resolved, false) + f, _, err := openVerified(resolved, false, false) if err == nil { _ = f.Close() t.Fatal("openVerified followed a component swapped after resolution") diff --git a/internal/scan/scanner.go b/internal/scan/scanner.go index ed444603..5824d613 100644 --- a/internal/scan/scanner.go +++ b/internal/scan/scanner.go @@ -9,6 +9,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/cli" "github.com/step-security/dev-machine-guard/internal/config" "github.com/step-security/dev-machine-guard/internal/detector" + "github.com/step-security/dev-machine-guard/internal/detector/browserext" "github.com/step-security/dev-machine-guard/internal/detector/configaudit" "github.com/step-security/dev-machine-guard/internal/detector/credentials" "github.com/step-security/dev-machine-guard/internal/device" @@ -262,6 +263,21 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { credentialScan := credentials.New(exec).WithSkipper(tccSkipper).Detect(ctx) log.StepDone(time.Since(start)) + // Browser extension inventory — which extensions the machine's browsers hold, + // whether they are enabled and why not, where they came from, and what they + // can touch. The browsers' own state files only: nothing is executed and the + // browsers' databases are never opened. A nil result means the phase declined, + // which is what a service context or a root caller produces, so the pointer is + // passed through untouched. + var browserExtensionScan *model.BrowserExtensionScanInfo + if featuregate.IsEnabled(featuregate.FeatureBrowserExtensionsScan) { + log.StepStart("Inventorying browser extensions") + start = time.Now() + browserTarget, _ := exec.LoggedInUser() + browserExtensionScan = browserext.New(exec).WithSkipper(tccSkipper).Detect(ctx, browserTarget) + log.StepDone(time.Since(start)) + } + // npm config audit — surface-only inventory of every .npmrc on the host // plus the merged effective view npm itself would resolve. The audit is // cheap (a few stat calls and at most two npm invocations) but stays @@ -392,6 +408,8 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { AgentSkills: agentSkills, AgentSkillScan: agentSkillScan, CredentialScan: credentialScan, + + BrowserExtensionScan: browserExtensionScan, Summary: model.Summary{ AIAgentsAndToolsCount: len(aiTools), IDEInstallationsCount: len(ides), diff --git a/internal/telemetry/phase_deadline.go b/internal/telemetry/phase_deadline.go index d9643fe6..f185d40a 100644 --- a/internal/telemetry/phase_deadline.go +++ b/internal/telemetry/phase_deadline.go @@ -25,20 +25,23 @@ import ( // "Disabled" means no per-phase deadline; the parent scan deadline // (STEPSEC_MAX_SCAN_DURATION) and the whole-process watchdog still apply. var phaseBudgets = map[string]time.Duration{ - "scheduler_info": 15 * time.Second, - "device_info": 30 * time.Second, - "ide_scan": 2 * time.Minute, - "extension_scan": 2 * time.Minute, - "ai_tools_scan": 5 * time.Minute, - "mcp_config_scan": 1 * time.Minute, - "agent_skills_scan": 2 * time.Minute, // detector self-caps at 60s; headroom for lock parsing - "credentials_scan": 1 * time.Minute, // fixed-path reads plus one bounded child process - "malicious_file_scan": 10 * time.Minute, - "brew_scan": 5 * time.Minute, - "python_scan": 10 * time.Minute, - "syspkg_scan": 5 * time.Minute, - "node_scan": 15 * time.Minute, - "telemetry_upload": 10 * time.Minute, + "scheduler_info": 15 * time.Second, + "device_info": 30 * time.Second, + "ide_scan": 2 * time.Minute, + "extension_scan": 2 * time.Minute, + "ai_tools_scan": 5 * time.Minute, + "mcp_config_scan": 1 * time.Minute, + "agent_skills_scan": 2 * time.Minute, // detector self-caps at 60s; headroom for lock parsing + "credentials_scan": 1 * time.Minute, // fixed-path reads plus one bounded child process + // Detector self-caps at 60s; headroom for a machine with several browsers and + // heavy profiles, whose preference files reach megabytes. + "browser_extensions_scan": 2 * time.Minute, + "malicious_file_scan": 10 * time.Minute, + "brew_scan": 5 * time.Minute, + "python_scan": 10 * time.Minute, + "syspkg_scan": 5 * time.Minute, + "node_scan": 15 * time.Minute, + "telemetry_upload": 10 * time.Minute, } const defaultPhaseBudget = 5 * time.Minute diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 44266da5..f44651ad 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -21,6 +21,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/cli" "github.com/step-security/dev-machine-guard/internal/config" "github.com/step-security/dev-machine-guard/internal/detector" + "github.com/step-security/dev-machine-guard/internal/detector/browserext" "github.com/step-security/dev-machine-guard/internal/detector/configaudit" "github.com/step-security/dev-machine-guard/internal/detector/credentials" "github.com/step-security/dev-machine-guard/internal/detector/rules" @@ -108,6 +109,10 @@ type Payload struct { AgentSkills []model.AgentSkill `json:"agent_skills,omitempty"` AgentSkillScan *model.AgentSkillScanInfo `json:"agent_skill_scan,omitempty"` CredentialScan *model.CredentialScanInfo `json:"credential_scan,omitempty"` + // Nil means the phase did not run, and that is the only signal a reader has + // for it: a section carrying zero findings is the positive claim that this + // machine's browsers hold no extensions. + BrowserExtensionScan *model.BrowserExtensionScanInfo `json:"browser_extension_scan,omitempty"` ExecutionLogs *ExecutionLogs `json:"execution_logs,omitempty"` PerformanceMetrics *PerformanceMetrics `json:"performance_metrics,omitempty"` @@ -1002,6 +1007,33 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err endPhase(phaseCtx, phaseCancel, tracker, log, "credentials_scan") postPhase() + // Browser extension inventory — which extensions are installed in this + // machine's browsers, whether they are enabled and why not, where they came + // from, and what they are permitted to touch. The browsers' own state files + // and nothing else: no browser is launched, no store is called, and the + // browsers' databases (cookies, history, passwords) are never opened. + // + // The target account is passed explicitly and the phase declines when it is a + // service identity: scanning the wrong home would find no browser and report + // every one of them missing, which a reader honours by deleting the device's + // real inventory. A nil section is that decline, and it must stay nil. + var browserExtensionScan *model.BrowserExtensionScanInfo + if featuregate.IsEnabled(featuregate.FeatureBrowserExtensionsScan) { + phaseCtx, phaseCancel = startPhase(ctx, tracker, "browser_extensions_scan") + log.Progress("Inventorying browser extensions...") + browserTarget, _ := exec.LoggedInUser() + browserExtensionScan = browserext.New(userExec).WithSkipper(tccSkipper).Detect(phaseCtx, browserTarget) + if browserExtensionScan == nil { + log.Progress(" Skipped: no interactive user to describe") + } else { + log.Progress(" Found %d browser extensions across %d browsers", + len(browserExtensionScan.Findings), len(browserExtensionScan.Browsers)) + } + fmt.Fprintln(os.Stderr) + endPhase(phaseCtx, phaseCancel, tracker, log, "browser_extensions_scan") + postPhase() + } + // npm + pip configuration audits — surface-only inventory of every // .npmrc and pip.conf on the host, plus the merged effective views // each tool would resolve. We use the user-aware executor so npm and @@ -1141,6 +1173,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err AgentSkills: agentSkills, AgentSkillScan: agentSkillScan, CredentialScan: credentialScan, + BrowserExtensionScan: browserExtensionScan, ExecutionLogs: &ExecutionLogs{ OutputBase64: execLogsBase64, diff --git a/tests/test_smoke_go.sh b/tests/test_smoke_go.sh index 63e9742f..08523656 100755 --- a/tests/test_smoke_go.sh +++ b/tests/test_smoke_go.sh @@ -135,6 +135,15 @@ for key in scan_timestamp scan_timestamp_iso agent_version; do fi done +# A gated phase must leave its section out entirely. Emitting it with zero +# findings is the positive claim that the machine holds no browser extensions, +# which the backend acts on by deleting what it has stored. +if echo "$JSON_OUTPUT" | python3 -c "import sys,json; d=json.load(sys.stdin); assert 'browser_extension_scan' not in d" 2>/dev/null; then + pass "JSON omits browser_extension_scan while the feature is gated off" +else + fail "JSON omits browser_extension_scan while the feature is gated off" +fi + # device object fields for key in hostname os_version serial_number platform user_identity; do if echo "$JSON_OUTPUT" | python3 -c "import sys,json; d=json.load(sys.stdin); assert '$key' in d['device']" 2>/dev/null; then From 0e777eddfc0b2dfc75b7b4a5f18145d4a810463d Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Sun, 16 Aug 2026 16:24:44 +0530 Subject: [PATCH 2/5] fix(browserext): do not report host permissions the user withheld Restricting an extension's site access does not rewrite its up-front grant. The browser sets withholding_permissions and moves the hosts that survive into the runtime record, leaving the withheld ones in granted_permissions as a record of what was originally handed over. Unioning the two records therefore reported access the user had taken back, so a restricted extension and an unrestricted one produced identical host lists. Hosts now come from the runtime record alone once the flag is set. API permissions are never withheld, so that union is unchanged, and a browser that does not write the flag keeps today's behaviour. --- internal/detector/browserext/chromium.go | 21 +++++-- internal/detector/browserext/chromium_test.go | 60 +++++++++++++++++++ internal/model/browserext.go | 9 +-- 3 files changed, 82 insertions(+), 8 deletions(-) diff --git a/internal/detector/browserext/chromium.go b/internal/detector/browserext/chromium.go index 6e2e4c86..a9807914 100644 --- a/internal/detector/browserext/chromium.go +++ b/internal/detector/browserext/chromium.go @@ -269,7 +269,13 @@ type chromiumEntry struct { WasInstalledByOEM bool `json:"was_installed_by_oem"` Granted *chromiumPermissions `json:"granted_permissions"` RuntimeGranted *chromiumPermissions `json:"runtime_granted_permissions"` - CWSInfo *chromiumCWSInfo `json:"cws-info"` + // Set when the user restricted the extension's site access. The browser + // leaves the withheld hosts in granted_permissions as a record of the + // original grant, so reading that set alone reports access the extension + // does not have. Hosts then come from runtime_granted_permissions alone. + // API permissions are never withheld. + WithholdingHostPermissions bool `json:"withholding_permissions"` + CWSInfo *chromiumCWSInfo `json:"cws-info"` } // chromiumManifest is the copy of the extension's manifest the browser keeps @@ -381,7 +387,7 @@ func (d *Detector) chromiumOccurrence(scan *scanState, profileDir, id string, ra state, disabledBy := chromiumEnabledState(e) listing, violation := storeDisposition(e.CWSInfo) - perms, hosts, capped := permissionLists(e.Granted, e.RuntimeGranted) + perms, hosts, capped := permissionLists(e.Granted, e.RuntimeGranted, e.WithholdingHostPermissions) if capped { b.degrade(model.BrowserExtReasonCapped) } @@ -655,14 +661,21 @@ func lookupMessage(data []byte, key string) string { // Both grant paths are unioned, because either alone misreports what the // extension can reach: the up-front set keeps patterns the user has since // withheld, and the runtime set is the only home for what they granted on -// demand. The union is the honest upper bound of effective access. -func permissionLists(granted, runtime *chromiumPermissions) (perms, hosts []string, capped bool) { +// demand. Once the user restricts site access the up-front set stops +// describing the present, so the hosts then come from the runtime set alone +// and the two lists part company: API permissions are never withheld. +func permissionLists(granted, runtime *chromiumPermissions, withholdingHosts bool) (perms, hosts []string, capped bool) { var api, host []string for _, set := range []*chromiumPermissions{granted, runtime} { if set == nil { continue } api = append(api, set.API...) + // A withheld host is not a granted host: it sits in granted_permissions + // only as a record of what the user later took back. + if withholdingHosts && set != runtime { + continue + } host = append(host, set.ExplicitHost...) host = append(host, set.ScriptableHost...) } diff --git a/internal/detector/browserext/chromium_test.go b/internal/detector/browserext/chromium_test.go index 6aa5621b..d189a9a6 100644 --- a/internal/detector/browserext/chromium_test.go +++ b/internal/detector/browserext/chromium_test.go @@ -437,6 +437,66 @@ func TestChromium_Permissions(t *testing.T) { } } +// TestChromium_WithheldHostsAreNotReported covers the one case where the union +// stops being true. Restricting an extension's site access does not rewrite the +// up-front grant — the browser sets a flag and moves what survives into the +// runtime record — so unioning the two reports the access the user took away. +func TestChromium_WithheldHostsAreNotReported(t *testing.T) { + const granted = `"api": ["storage", "tabs"], "explicit_host": ["*://*/*", ""]` + + for _, tt := range []struct { + name string + entry string + want []string + wantAPI []string + }{ + { + // Site access restricted to nothing: the extension holds no host at + // all, however much the up-front record still lists. + name: "withheld with nothing granted back reaches no host", + entry: `"withholding_permissions": true, "runtime_granted_permissions": {"api": ["cookies"]}`, + want: nil, + wantAPI: []string{"cookies", "storage", "tabs"}, + }, + { + // Site access restricted to one site: that site, and not the pattern + // it was carved out of. + name: "withheld with one site granted back reaches that site", + entry: `"withholding_permissions": true, "runtime_granted_permissions": { + "api": ["cookies"], "scriptable_host": ["https://kept.internal/*"] + }`, + want: []string{"https://kept.internal/*"}, + wantAPI: []string{"cookies", "storage", "tabs"}, + }, + { + // Every browser that does not write the flag, and every extension + // whose access was never restricted. + name: "no flag leaves the union alone", + entry: `"runtime_granted_permissions": {"api": ["cookies"], "scriptable_host": ["https://kept.internal/*"]}`, + want: []string{"*://*/*", "", "https://kept.internal/*"}, + wantAPI: []string{"cookies", "storage", "tabs"}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example", "version": "1.0"}, + "granted_permissions": {`+granted+`}, + `+tt.entry+` + }`) + + if strings.Join(got.HostPermissions, ",") != strings.Join(tt.want, ",") { + t.Errorf("host_permissions = %v, want %v", got.HostPermissions, tt.want) + } + // Withholding is about hosts. An API permission disappearing here + // means the branch caught the wrong list. + if strings.Join(got.Permissions, ",") != strings.Join(tt.wantAPI, ",") { + t.Errorf("permissions = %v, want %v", got.Permissions, tt.wantAPI) + } + }) + } +} + // TestChromium_OverlongFieldsByClass pins the difference between a string a person // reads and a string something matches. A name is shortened; a permission is // dropped, because a shortened grant is a different grant and showing an auditor a diff --git a/internal/model/browserext.go b/internal/model/browserext.go index 6d11e9d0..99b3ad4e 100644 --- a/internal/model/browserext.go +++ b/internal/model/browserext.go @@ -234,10 +234,11 @@ type BrowserExtensionFinding struct { SignedState string `json:"signed_state,omitempty"` // The API permissions and host patterns the browser recorded as granted, - // including what the user granted at runtime — the honest upper bound of - // what the extension can reach, rather than what its manifest asked for. An - // entry too long for its cap is omitted rather than shortened: these strings - // are matched, so a shortened one is a different grant. + // including what the user granted at runtime, rather than what the manifest + // asked for. Hosts the user has since withheld are left out, because the + // browser has taken them back. An entry too long for its cap is omitted + // rather than shortened: these strings are matched, so a shortened one is a + // different grant. Permissions []string `json:"permissions"` HostPermissions []string `json:"host_permissions"` } From 9698772550868b83608e39544577ceab26dc2e6d Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Sun, 16 Aug 2026 19:25:35 +0530 Subject: [PATCH 3/5] feat(browserext): report scriptable hosts, manifest version and unpacked paths Chromium collapsed granted_permissions.explicit_host and scriptable_host into one list, so an extension that can inject code into a host looked the same as one that can only reach it with a request. Split them: scriptable_host_permissions is derived from the already-capped host union, so it is a subset by construction. The field is a pointer, since an empty list means the extension injects nowhere and an absent one means the engine does not record the distinction. Manifest version now ships on both engines. A version 2 extension can hold blocking request interception, which version 3 removed, so the two are not the same risk class. Unpacked extensions no longer degrade a browser to partial for ever. The refusal to resolve an absolute path stays, but nothing was read and failed, so there is nothing to report as degraded. Their load path ships as install_path, capped and omitted rather than shortened, because an unreviewed extension running out of a user directory is the signal and a blank name is not. Gecko read runtime grants from nowhere: Firefox writes them only to extension-preferences.json, which was never opened. Union it into each add-on already present in extensions.json, dropping internal: bookkeeping and routing host patterns to origins. The file can only add attributes, so a missing one is silence while an unparseable or refused one degrades. Data collection comes across with it, keeping ["none"] distinct from nothing declared. Also carries the Chromium disable_reasons actor map, which reads the reason bits rather than the aggregate state so a policy-disabled extension is not reported as a user choice. --- internal/detector/browserext/catalog.go | 3 + internal/detector/browserext/chromium.go | 139 ++++++++--- internal/detector/browserext/chromium_test.go | 222 +++++++++++++++++- internal/detector/browserext/gecko.go | 104 +++++++- internal/detector/browserext/gecko_test.go | 155 ++++++++++++ internal/model/browserext.go | 35 ++- .../browser_extension_scan_golden.json | 25 +- 7 files changed, 621 insertions(+), 62 deletions(-) diff --git a/internal/detector/browserext/catalog.go b/internal/detector/browserext/catalog.go index 839039fd..a42be9cd 100644 --- a/internal/detector/browserext/catalog.go +++ b/internal/detector/browserext/catalog.go @@ -187,6 +187,9 @@ const ( maxVersionBytes = 64 maxExtensionIDBytes = 256 // gecko ids are e-mail or UUID shaped; chromium is 32 maxPermissionBytes = 256 // per entry + // An unpacked extension's load path. Omitted rather than shortened when it + // exceeds this: half a path names a directory nobody has. + maxInstallPathBytes = 1024 ) // File read caps. Validated against the descriptor the bytes come from, and a diff --git a/internal/detector/browserext/chromium.go b/internal/detector/browserext/chromium.go index a9807914..526b3691 100644 --- a/internal/detector/browserext/chromium.go +++ b/internal/detector/browserext/chromium.go @@ -40,19 +40,40 @@ const ( locationExternalComponent = 10 ) -// Disable reasons worth naming, as the browser's own enumeration numbers them. -// It records a set of them, and only these three change the answer: a set holding -// the user's own action was the user's doing, one holding a policy reason was the -// administrator's, and any other non-empty set means the browser disabled the -// extension for a reason of its own — which is the case worth surfacing, since a -// store takedown lands here. Bits are appended to that enumeration rather than -// renumbered, so an unrecognised value deliberately reads as "the browser did it" -// rather than as a value to interpret. -const ( - disableReasonUserAction = 1 << 0 - disableReasonPolicyUpdateRequire = 1 << 13 - disableReasonBlockedByPolicy = 1 << 15 -) +// Disable reasons as the browser's own enumeration numbers them, mapped to the +// actor a console should name. Membership here is recognition: a value absent +// from the table is a fork's own or a newer release's, and the browser itself +// collapses those to "unknown" rather than interpreting them. Bits 6, 7, 12, 17 +// and 18 do not exist. The two reasons that mean "we do not know why" are absent +// deliberately, so they fall to the same answer as an unrecognised value. +var disableReasonActor = map[int]string{ + 1 << 0: model.BrowserExtDisabledByUser, // user action + 1 << 1: model.BrowserExtDisabledByBrowser, // permissions increase + 1 << 2: model.BrowserExtDisabledByBrowser, // reload + 1 << 3: model.BrowserExtDisabledByBrowser, // unsupported requirement + 1 << 4: model.BrowserExtDisabledByBrowser, // sideload wipeout + 1 << 8: model.BrowserExtDisabledByBrowser, // not verified + 1 << 9: model.BrowserExtDisabledByBrowser, // greylist + 1 << 10: model.BrowserExtDisabledByBrowser, // corrupted + 1 << 11: model.BrowserExtDisabledByBrowser, // remote install + 1 << 13: model.BrowserExtDisabledByBrowser, // external extension + 1 << 14: model.BrowserExtDisabledByPolicy, // update required by policy + // Custodian approval is family supervision rather than administrative + // policy: reading it as policy would point a fleet console at an + // administrator who did nothing. + 1 << 15: model.BrowserExtDisabledByBrowser, + 1 << 16: model.BrowserExtDisabledByPolicy, // blocked by policy + 1 << 19: model.BrowserExtDisabledByBrowser, // reinstall + // Not allowlisted is the browser's own safety machinery, and it exempts + // policy-allowed extensions, so an administrator is the one actor it + // cannot be. + 1 << 20: model.BrowserExtDisabledByBrowser, + 1 << 21: model.BrowserExtDisabledByBrowser, // keeplist + 1 << 22: model.BrowserExtDisabledByPolicy, // store publication required by policy + 1 << 23: model.BrowserExtDisabledByBrowser, // unsupported manifest version + 1 << 24: model.BrowserExtDisabledByBrowser, // unsupported developer extension + 1 << 26: model.BrowserExtDisabledByPolicy, // blocked by cloud policy check +} // scanChromiumRoot reads one Chromium-family data directory and reports whether // it held an installation. @@ -261,7 +282,8 @@ type chromiumEntry struct { State *int `json:"state"` // Relative to the profile's own Extensions directory for a store install, // absolute for an unpacked one. An absolute path is never opened and never - // ships. + // resolved. It does reach the wire for an unpacked extension, whose load + // location is the most useful thing its record holds. Path string `json:"path"` Manifest *chromiumManifest `json:"manifest"` FromWebstore *bool `json:"from_webstore"` @@ -286,6 +308,10 @@ type chromiumManifest struct { Version string `json:"version"` DefaultLocale string `json:"default_locale"` UpdateURL string `json:"update_url"` + // Version 2 can hold blocking request interception, which version 3 removed, + // so two extensions doing the same job differ here in what they are able to + // do to a page. + ManifestVersion int `json:"manifest_version"` // Presence alone is the test: a manifest with either key is a theme or a // legacy packaged app rather than an extension. Theme json.RawMessage `json:"theme"` @@ -349,10 +375,11 @@ func (d *Detector) chromiumOccurrence(scan *scanState, profileDir, id string, ra manifest := e.Manifest // Only a store install's path is relative, which is what makes it safe to // resolve: it lands inside the browser's own tree. An unpacked extension's - // path is an arbitrary user location — this never opens one, and never ships - // it either, so nothing about it is read beyond what the preferences recorded. + // path is an arbitrary user location, so it is never opened and never + // resolved, and nothing about it is read beyond what the preferences recorded. extDir := "" - if e.Path != "" && !filepath.IsAbs(e.Path) && !hasParentComponent(e.Path) { + unpackedLocation := filepath.IsAbs(e.Path) + if e.Path != "" && !unpackedLocation && !hasParentComponent(e.Path) { extDir = filepath.Join(profileDir, "Extensions", filepath.FromSlash(e.Path)) } if manifest == nil && extDir != "" { @@ -377,17 +404,34 @@ func (d *Detector) chromiumOccurrence(scan *scanState, profileDir, id string, ra return occurrence{}, false } - name, version := "", "" + name, version, manifestVersion := "", "", 0 if manifest == nil { - b.degrade(model.BrowserExtReasonManifestUnavailable) + if !unpackedLocation { + // An unpacked extension's manifest was never going to be read, so + // nothing failed to read: reporting one as degraded would paint the + // browser partial on every scan for as long as a developer keeps a + // build loaded. Every other route to a nil manifest is a document this + // scan could not recover. + b.degrade(model.BrowserExtReasonManifestUnavailable) + } } else { name = d.resolveExtensionName(scan, extDir, manifest, b) version = manifest.Version + manifestVersion = manifest.ManifestVersion + } + + // The location an unpacked extension was loaded from is the most useful thing + // its record holds, and the only one that survives having no manifest. An + // over-long path is left out rather than shortened: half a path names a + // directory nobody has. + installPath := "" + if source == model.BrowserExtInstallUnpacked && len(e.Path) <= maxInstallPathBytes { + installPath = e.Path } state, disabledBy := chromiumEnabledState(e) listing, violation := storeDisposition(e.CWSInfo) - perms, hosts, capped := permissionLists(e.Granted, e.RuntimeGranted, e.WithholdingHostPermissions) + perms, hosts, scriptable, capped := permissionLists(e.Granted, e.RuntimeGranted, e.WithholdingHostPermissions) if capped { b.degrade(model.BrowserExtReasonCapped) } @@ -397,14 +441,20 @@ func (d *Detector) chromiumOccurrence(scan *scanState, profileDir, id string, ra block: model.BrowserExtensionFinding{ Name: capBytes(name, maxNameBytes), Version: capBytes(version, maxVersionBytes), + ManifestVersion: manifestVersion, EnabledState: state, StoreListing: listing, StoreViolation: violation, InstallSource: source, + InstallPath: installPath, Store: chromiumStore(manifest, e), Preinstalled: e.WasInstalledByDefault || e.WasInstalledByOEM, Permissions: perms, HostPermissions: hosts, + // Always answered on this family: the grant records name the + // scriptable hosts separately, so an empty list is "injects nowhere" + // rather than "cannot tell". + ScriptableHostPermissions: &scriptable, }, }, true } @@ -412,6 +462,10 @@ func (d *Detector) chromiumOccurrence(scan *scanState, profileDir, id string, ra // reducedOccurrence is what a record whose value could not be read produces: an // identity, and everything else spelled as unknown rather than left out. A reader // requires the two store fields on this family, and "cannot tell" is a value. +// +// The scriptable host list is the one field left absent rather than empty. Empty +// would claim the extension injects nowhere, and no grant record was read to say +// so. func reducedOccurrence() occurrence { return occurrence{ enabled: model.BrowserExtStateUnknown, @@ -497,17 +551,24 @@ func chromiumEnabledState(e chromiumEntry) (state, disabledBy string) { if len(reasons) == 0 { return model.BrowserExtEnabled, "" } + actor := "" for _, r := range reasons { - if r == disableReasonUserAction { + switch disableReasonActor[r] { + case model.BrowserExtDisabledByUser: return model.BrowserExtDisabled, model.BrowserExtDisabledByUser + case model.BrowserExtDisabledByPolicy: + actor = model.BrowserExtDisabledByPolicy + case model.BrowserExtDisabledByBrowser: + if actor == "" { + actor = model.BrowserExtDisabledByBrowser + } } } - for _, r := range reasons { - if r == disableReasonBlockedByPolicy || r == disableReasonPolicyUpdateRequire { - return model.BrowserExtDisabled, model.BrowserExtDisabledByPolicy - } + if actor == "" { + // Recognised nothing in the set, so the actor is genuinely not known. + return model.BrowserExtDisabled, model.BrowserExtDisabledByUnknown } - return model.BrowserExtDisabled, model.BrowserExtDisabledByBrowser + return model.BrowserExtDisabled, actor } // parseDisableReasons reads both shapes in the wild: current browsers write a @@ -656,7 +717,7 @@ func lookupMessage(data []byte, key string) string { return "" } -// permissionLists reduces the grant records to the two wire lists. +// permissionLists reduces the grant records to the three wire lists. // // Both grant paths are unioned, because either alone misreports what the // extension can reach: the up-front set keeps patterns the user has since @@ -664,24 +725,42 @@ func lookupMessage(data []byte, key string) string { // demand. Once the user restricts site access the up-front set stops // describing the present, so the hosts then come from the runtime set alone // and the two lists part company: API permissions are never withheld. -func permissionLists(granted, runtime *chromiumPermissions, withholdingHosts bool) (perms, hosts []string, capped bool) { +// +// The scriptable list is a subset of the host list and is derived from it after +// the cap rather than beside it, so a host the cap dropped cannot survive in the +// stronger list alone. Injecting code into a page is a larger capability than +// reaching it with a request, and a reader that saw one without the other would +// have to guess which. +func permissionLists(granted, runtime *chromiumPermissions, withholdingHosts bool) (perms, hosts, scriptable []string, capped bool) { var api, host []string + scriptableSet := map[string]struct{}{} for _, set := range []*chromiumPermissions{granted, runtime} { if set == nil { continue } api = append(api, set.API...) // A withheld host is not a granted host: it sits in granted_permissions - // only as a record of what the user later took back. + // only as a record of what the user later took back. Neither list carries + // it, because a host the extension cannot reach is not one it can inject + // into either. if withholdingHosts && set != runtime { continue } host = append(host, set.ExplicitHost...) host = append(host, set.ScriptableHost...) + for _, h := range set.ScriptableHost { + scriptableSet[h] = struct{}{} + } } perms, apiCapped := capPermissionList(api) hosts, hostCapped := capPermissionList(host) - return perms, hosts, apiCapped || hostCapped + scriptable = []string{} + for _, h := range hosts { + if _, ok := scriptableSet[h]; ok { + scriptable = append(scriptable, h) + } + } + return perms, hosts, scriptable, apiCapped || hostCapped } // capPermissionList sorts, deduplicates and bounds one permission list, reporting diff --git a/internal/detector/browserext/chromium_test.go b/internal/detector/browserext/chromium_test.go index d189a9a6..42334da8 100644 --- a/internal/detector/browserext/chromium_test.go +++ b/internal/detector/browserext/chromium_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "testing" @@ -120,19 +121,50 @@ func TestChromium_EnabledState(t *testing.T) { state: model.BrowserExtDisabled, disabledBy: model.BrowserExtDisabledByBrowser, }, + { + // The bit an administrator reaches for to ban an extension outright, + // as measured against a live blocklist. + name: "an administrator's policy blocking it outright", + record: `"location": 1, "disable_reasons": [65536]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByPolicy, + }, { name: "an administrator's policy holding an update back", - record: `"location": 1, "disable_reasons": [8192]`, + record: `"location": 1, "disable_reasons": [16384]`, state: model.BrowserExtDisabled, disabledBy: model.BrowserExtDisabledByPolicy, }, { - // The other policy bit, and the one an administrator reaches for to - // ban an extension outright. - name: "an administrator's policy blocking it outright", + // An externally installed extension awaiting the user's approval. No + // administrator is involved, so naming one would be wrong. + name: "an external installation awaiting approval", + record: `"location": 1, "disable_reasons": [8192]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByBrowser, + }, + { + // Family supervision, likewise not an administrator. + name: "a custodian's approval outstanding", record: `"location": 1, "disable_reasons": [32768]`, state: model.BrowserExtDisabled, - disabledBy: model.BrowserExtDisabledByPolicy, + disabledBy: model.BrowserExtDisabledByBrowser, + }, + { + // Out of range of the enumeration entirely, which is what one browser + // in the family writes. Naming an actor for it would be invention. + name: "a value the enumeration does not carry", + record: `"location": 1, "disable_reasons": [134217728]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByUnknown, + }, + { + // A reason kept only for profiles old enough to still hold it. Still + // recognised, so it must not turn the whole set unknown. + name: "a retired reason alongside the user's action", + record: `"location": 1, "disable_reasons": [2097152, 1]`, + state: model.BrowserExtDisabled, + disabledBy: model.BrowserExtDisabledByUser, }, { // The user's action wins the cause when a set carries several. @@ -497,6 +529,122 @@ func TestChromium_WithheldHostsAreNotReported(t *testing.T) { } } +// TestChromium_ScriptableHostsAreASubset covers the stronger of the two host +// capabilities. Injecting code into a page is not the same as sending it a +// request, and the browser records the difference, so the payload keeps it. +func TestChromium_ScriptableHostsAreASubset(t *testing.T) { + t.Run("the injectable hosts are named inside the reachable ones", func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example", "version": "1.0"}, + "granted_permissions": { + "explicit_host": [""], + "scriptable_host": ["https://example.internal/*"] + } + }`) + + wantHosts := []string{"", "https://example.internal/*"} + if strings.Join(got.HostPermissions, ",") != strings.Join(wantHosts, ",") { + t.Errorf("host_permissions = %v, want %v", got.HostPermissions, wantHosts) + } + if got.ScriptableHostPermissions == nil { + t.Fatal("scriptable_host_permissions is absent, want the answer this family always has") + } + want := []string{"https://example.internal/*"} + if strings.Join(*got.ScriptableHostPermissions, ",") != strings.Join(want, ",") { + t.Errorf("scriptable_host_permissions = %v, want %v", *got.ScriptableHostPermissions, want) + } + }) + + t.Run("an extension that injects nowhere says so", func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example", "version": "1.0"}, + "granted_permissions": {"explicit_host": ["https://example.internal/*"]} + }`) + + if got.ScriptableHostPermissions == nil || len(*got.ScriptableHostPermissions) != 0 { + t.Errorf("scriptable_host_permissions = %v, want an empty list: this family knows the answer", + got.ScriptableHostPermissions) + } + }) + + t.Run("withholding empties both lists together", func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example", "version": "1.0"}, + "withholding_permissions": true, + "granted_permissions": {"scriptable_host": ["https://taken-back.internal/*"]}, + "runtime_granted_permissions": {"api": ["cookies"]} + }`) + + if len(got.HostPermissions) != 0 { + t.Errorf("host_permissions = %v, want none", got.HostPermissions) + } + // A host the extension cannot reach is not one it can inject into, so the + // stronger list cannot outlive the weaker one. + if got.ScriptableHostPermissions == nil || len(*got.ScriptableHostPermissions) != 0 { + t.Errorf("scriptable_host_permissions = %v, want none", got.ScriptableHostPermissions) + } + }) + + t.Run("a host the cap drops is absent from both lists", func(t *testing.T) { + // Sorts after every filler entry, so the count cap is what removes it. + const last = `"https://zz-injectable.internal/*"` + filler := make([]string, maxPermissionsPerFinding) + for i := range filler { + filler[i] = fmt.Sprintf(`"https://a%02d.internal/*"`, i) + } + got, coverage := chromeFinding(t, `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example", "version": "1.0"}, + "granted_permissions": { + "explicit_host": [`+strings.Join(filler, ",")+`], + "scriptable_host": [`+last+`] + } + }`) + + if slices.Contains(got.HostPermissions, strings.Trim(last, `"`)) { + t.Fatalf("host_permissions still carries the capped entry: %v", got.HostPermissions) + } + if got.ScriptableHostPermissions == nil { + t.Fatal("scriptable_host_permissions is absent") + } + if len(*got.ScriptableHostPermissions) != 0 { + t.Errorf("scriptable_host_permissions = %v, want none: the stronger list must not outlive the cap", + *got.ScriptableHostPermissions) + } + if coverage.Status != model.BrowserCoveragePartial || coverage.ReasonCode != model.BrowserExtReasonCapped { + t.Errorf("status = %q/%q, want partial and capped", coverage.Status, coverage.ReasonCode) + } + }) +} + +// TestChromium_ManifestVersion separates two extensions that otherwise look +// identical. Version 2 can hold blocking request interception, which version 3 +// removed, so the same content blocker on two engines is not the same capability. +func TestChromium_ManifestVersion(t *testing.T) { + for _, tt := range []struct { + name string + manifest string + want int + }{ + {"the revision that can still block requests", `"manifest_version": 2`, 2}, + {"the revision that cannot", `"manifest_version": 3`, 3}, + {"a record that does not say", `"version": "1.0"`, 0}, + } { + t.Run(tt.name, func(t *testing.T) { + got, _ := chromeFinding(t, `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example", `+tt.manifest+`} + }`) + if got.ManifestVersion != tt.want { + t.Errorf("manifest_version = %d, want %d", got.ManifestVersion, tt.want) + } + }) + } +} + // TestChromium_OverlongFieldsByClass pins the difference between a string a person // reads and a string something matches. A name is shortened; a permission is // dropped, because a shortened grant is a different grant and showing an auditor a @@ -636,6 +784,12 @@ func TestChromium_ManifestFallbackAndLocalizedName(t *testing.T) { // unpacked extension's directory is an arbitrary user location, so its absolute // path is never resolved and never read. The manifest planted there would supply // a name if anything opened it. +// +// The path itself does ship. It is the one thing that makes the row actionable +// once the name is gone, and it is reported exactly as the preferences recorded +// it. Nothing was read and failed to read here, so the browser stays clean: a +// developer with a build loaded would otherwise see a degraded browser on every +// scan for ever. func TestChromium_UnpackedContentIsNeverOpened(t *testing.T) { home := tempHome(t) unpacked := filepath.Join(home, "projects", "client-work", "ext") @@ -657,15 +811,59 @@ func TestChromium_UnpackedContentIsNeverOpened(t *testing.T) { if got[0].InstallSource != model.BrowserExtInstallUnpacked { t.Errorf("install_source = %q, want unpacked — the whole signal this row carries", got[0].InstallSource) } - // No filesystem path of any kind reaches the wire, and this is the path that - // could have carried a client's name. - raw := marshalFindings(t, info) - if strings.Contains(raw, "client-work") { - t.Errorf("payload carries a filesystem path: %s", raw) + if got[0].InstallPath != filepath.ToSlash(unpacked) { + t.Errorf("install_path = %q, want %q: where it was loaded from is what this row is for", + got[0].InstallPath, unpacked) + } + c := coverageFor(t, info, browserChrome) + if c.Status != model.BrowserCoverageScanned { + t.Errorf("status = %q/%q, want scanned: no manifest was read, so none failed to read", + c.Status, c.ReasonCode) + } +} + +// TestChromium_UnpackedPathIsOmittedRatherThanShortened keeps the load path in +// the class of strings that are matched rather than read. Half a path names a +// directory nobody has, and a browser is not degraded over it: the row still +// carries the identity and the install source that make it worth looking at. +func TestChromium_UnpackedPathIsOmittedRatherThanShortened(t *testing.T) { + home := tempHome(t) + long := "/" + strings.Repeat("d", maxInstallPathBytes) + + localState(t, chromeRoot(home), "Default") + securePrefs(t, chromeRoot(home), "Default", `"`+idA+`": {"location": 4, "path": "`+long+`"}`) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserChrome) + if len(got) != 1 { + t.Fatalf("findings = %d, want the extension reported without its path", len(got)) } + if got[0].InstallPath != "" { + t.Errorf("install_path = %q, want none: a shortened path is a different directory", got[0].InstallPath) + } + if got[0].InstallSource != model.BrowserExtInstallUnpacked { + t.Errorf("install_source = %q, want unpacked", got[0].InstallSource) + } +} + +// TestChromium_UnreadableManifestStillDegrades is the other polarity of the same +// branch. An unpacked extension is not degraded because nothing was read; a store +// install whose manifest genuinely refuses to read still is, because something +// was. +func TestChromium_UnreadableManifestStillDegrades(t *testing.T) { + home := tempHome(t) + root := chromeRoot(home) + localState(t, root, "Default") + securePrefs(t, root, "Default", `"`+idA+`": {"location": 1, "path": "`+idA+`/1.2.3_0"}`) + writeFile(t, filepath.Join(root, "Default", "Extensions", idA, "1.2.3_0", "manifest.json"), + strings.Repeat("{", maxManifestBytes+1)) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) c := coverageFor(t, info, browserChrome) - if c.Status != model.BrowserCoveragePartial || c.ReasonCode != model.BrowserExtReasonManifestUnavailable { - t.Errorf("status = %q/%q, want partial and the missing metadata named", c.Status, c.ReasonCode) + if c.Status != model.BrowserCoveragePartial { + t.Errorf("status = %q, want partial: a document was reached and could not be read", c.Status) } } diff --git a/internal/detector/browserext/gecko.go b/internal/detector/browserext/gecko.go index 61fde907..5f301c94 100644 --- a/internal/detector/browserext/gecko.go +++ b/internal/detector/browserext/gecko.go @@ -55,6 +55,11 @@ var geckoBuiltinScopes = map[string]bool{ // scope is an ordinary profile install and nothing else distinguishes it. const geckoPolicySource = "enterprise-policy" +// The browser keeps its own bookkeeping in the same list as the add-on's +// permissions, under this prefix. It sits on every add-on on the machine and +// says nothing about what any of them can do. +const geckoInternalPrefPrefix = "internal:" + // Signing states, as the database numbers them. An unsigned add-on that is // enabled is the headline security signal on this engine. var geckoSignedStates = map[int]string{ @@ -269,6 +274,10 @@ type geckoAddon struct { SignedState *int `json:"signedState"` Visible *bool `json:"visible"` Hidden *bool `json:"hidden"` + // Version 2 can hold blocking request interception, which version 3 removed. + // The two content blockers on a machine can differ here while looking + // otherwise identical. + ManifestVersion int `json:"manifestVersion"` DefaultLocale struct { Name string `json:"name"` @@ -277,6 +286,12 @@ type geckoAddon struct { UserPermissions struct { Permissions []string `json:"permissions"` Origins []string `json:"origins"` + // What the add-on declared it collects. A list holding "none" says it + // collects nothing, which is a different answer from an empty list, where + // it declared nothing at all. The record also carries a top-level + // dataCollectionPermissions, which reads null even for add-ons that + // declared a value: the granted answer is this one. + DataCollection []string `json:"data_collection"` } `json:"userPermissions"` InstallTelemetryInfo struct { @@ -284,6 +299,48 @@ type geckoAddon struct { } `json:"installTelemetryInfo"` } +// geckoPrefEntry is one add-on's entry in the per-add-on preferences document, +// which is where this engine keeps what the user granted at runtime. The add-on +// database holds only the install-time grant, so an add-on handed a permission +// on demand reads as never having it if this file goes unread. +type geckoPrefEntry struct { + Permissions []string `json:"permissions"` + Origins []string `json:"origins"` + DataCollection []string `json:"data_collection"` +} + +// geckoRuntimeGrants reads one profile's per-add-on preferences. +// +// It can only add attributes: membership came from the add-on database and is +// already complete, so nothing here fails the browser. A file that is absent is +// silence rather than a degradation, because the browser writes it when a +// preference is set and a runtime grant cannot exist without one. A file that is +// present and unreadable is a real gap and says so. +func (d *Detector) geckoRuntimeGrants(scan *scanState, dir string, b *browserScan) map[string]geckoPrefEntry { + data, missing, reason := scan.readState( + filepath.Join(dir, "extension-preferences.json"), maxExtensionsJSONBytes) + if reason != "" { + b.degrade(reason) + return nil + } + if missing { + return nil + } + var entries map[string]geckoPrefEntry + if err := json.Unmarshal(data, &entries); err != nil { + b.degrade(model.BrowserExtReasonParseError) + return nil + } + return entries +} + +// isGeckoHostPattern reports whether a preference entry names a host rather than +// an API permission. The file mixes the two in one list: the origins an add-on +// was granted appear under its permissions as well. +func isGeckoHostPattern(entry string) bool { + return entry == "" || strings.Contains(entry, "://") +} + // scanGeckoProfile reads one profile's add-on database. func (d *Detector) scanGeckoProfile(scan *scanState, dir string, data []byte, b *browserScan) { var db struct { @@ -298,6 +355,11 @@ func (d *Detector) scanGeckoProfile(scan *scanState, dir string, data []byte, b b.fail(model.BrowserExtReasonParseError) return } + // Looked up per add-on rather than iterated, which is what keeps an entry the + // database does not list out of the inventory: those are temporary add-ons, + // and their entries outlive them. A finding built from one is an alert that + // can never clear. + grants := d.geckoRuntimeGrants(scan, dir, b) for _, raw := range *db.Addons { var a geckoAddon if err := json.Unmarshal(raw, &a); err != nil { @@ -339,7 +401,7 @@ func (d *Detector) scanGeckoProfile(scan *scanState, dir string, data []byte, b continue } - occ, ok := d.geckoOccurrence(a, b) + occ, ok := d.geckoOccurrence(a, grants[id], b) if !ok { continue } @@ -350,8 +412,9 @@ func (d *Detector) scanGeckoProfile(scan *scanState, dir string, data []byte, b } } -// geckoOccurrence turns one add-on record into one occurrence. -func (d *Detector) geckoOccurrence(a geckoAddon, b *browserScan) (occurrence, bool) { +// geckoOccurrence turns one add-on record into one occurrence, folding in what +// the profile's preferences say the user granted since. +func (d *Detector) geckoOccurrence(a geckoAddon, granted geckoPrefEntry, b *browserScan) (occurrence, bool) { source := model.BrowserExtInstallUnknown if mapped, ok := geckoInstallSources[a.Location]; ok { source = mapped @@ -363,18 +426,37 @@ func (d *Detector) geckoOccurrence(a geckoAddon, b *browserScan) (occurrence, bo } state, disabledBy := geckoEnabledState(a) - perms, permsCapped := capPermissionList(a.UserPermissions.Permissions) - hosts, hostsCapped := capPermissionList(a.UserPermissions.Origins) - if permsCapped || hostsCapped { + + api := a.UserPermissions.Permissions + origins := append(a.UserPermissions.Origins, granted.Origins...) + for _, entry := range granted.Permissions { + switch { + case strings.HasPrefix(entry, geckoInternalPrefPrefix): + case isGeckoHostPattern(entry): + origins = append(origins, entry) + default: + api = append(api, entry) + } + } + // The categories are left as the browser wrote them. Firefox adds one per + // release, and a closed vocabulary would turn the next one into a whole + // browser that could not be read. + collection := append(a.UserPermissions.DataCollection, granted.DataCollection...) + + perms, permsCapped := capPermissionList(api) + hosts, hostsCapped := capPermissionList(origins) + dataCollection, collectionCapped := capPermissionList(collection) + if permsCapped || hostsCapped || collectionCapped { b.degrade(model.BrowserExtReasonCapped) } return occurrence{ enabled: state, disabledBy: disabledBy, block: model.BrowserExtensionFinding{ - Name: capBytes(a.DefaultLocale.Name, maxNameBytes), - Version: capBytes(a.Version, maxVersionBytes), - EnabledState: state, + Name: capBytes(a.DefaultLocale.Name, maxNameBytes), + Version: capBytes(a.Version, maxVersionBytes), + ManifestVersion: a.ManifestVersion, + EnabledState: state, // The store fields have no counterpart on this engine: there is no // listing state in the database to read. InstallSource: source, @@ -382,6 +464,10 @@ func (d *Detector) geckoOccurrence(a geckoAddon, b *browserScan) (occurrence, bo SignedState: geckoSignedState(a.SignedState), Permissions: perms, HostPermissions: hosts, + // ScriptableHostPermissions is deliberately absent: this engine records + // one origins list and draws no line inside it, and an empty list would + // claim the add-on injects nowhere. + DataCollection: dataCollection, }, }, true } diff --git a/internal/detector/browserext/gecko_test.go b/internal/detector/browserext/gecko_test.go index 09194e1f..86956375 100644 --- a/internal/detector/browserext/gecko_test.go +++ b/internal/detector/browserext/gecko_test.go @@ -310,6 +310,161 @@ func TestGecko_NameAndPermissions(t *testing.T) { } } +// firefoxFindingWithGrants runs one Firefox profile that also carries the +// per-add-on preferences document, and returns its single finding. +func firefoxFindingWithGrants(t *testing.T, addon, prefs string) (model.BrowserExtensionFinding, model.BrowserCoverage) { + t.Helper() + home := tempHome(t) + root := firefoxRoot(home) + profile := filepath.Join(root, "abcd1234.default-release") + writeFile(t, filepath.Join(root, "profiles.ini"), "[Profile0]\nIsRelative=1\nPath=abcd1234.default-release\n") + geckoAddonJSON(t, profile, addon) + writeFile(t, filepath.Join(profile, "extension-preferences.json"), prefs) + + info := scanHome(t, home) + assertPayloadInvariants(t, info) + got := findingsFor(info, browserFirefox) + if len(got) != 1 { + t.Fatalf("findings = %d, want exactly one", len(got)) + } + return got[0], coverageFor(t, info, browserFirefox) +} + +// TestGecko_RuntimeGrants covers the permissions this engine records nowhere near +// the rest of the add-on. What the user granted on demand lives in its own +// document, and reading only the add-on database reports an add-on holding +// cookies and history as holding neither. +func TestGecko_RuntimeGrants(t *testing.T) { + const addon = `{ + "id": "ext@example-org", "type": "extension", "active": true, "location": "app-profile", + "userPermissions": {"permissions": ["storage"], "origins": ["https://declared.internal/*"]} + }` + + t.Run("what was granted later is unioned in", func(t *testing.T) { + got, coverage := firefoxFindingWithGrants(t, addon, `{ + "ext@example-org": { + "permissions": ["cookies", "history", "", "internal:privateBrowsingAllowed"], + "origins": ["https://granted.internal/*"] + } + }`) + + want := "cookies,history,storage" + if strings.Join(got.Permissions, ",") != want { + t.Errorf("permissions = %v, want %q", got.Permissions, want) + } + // The file mixes origins into the permissions list, and a host pattern + // reported as an API permission is a capability that does not exist. + wantHosts := ",https://declared.internal/*,https://granted.internal/*" + if strings.Join(got.HostPermissions, ",") != wantHosts { + t.Errorf("host_permissions = %v, want %q", got.HostPermissions, wantHosts) + } + if coverage.Status != model.BrowserCoverageScanned { + t.Errorf("status = %q/%q, want scanned", coverage.Status, coverage.ReasonCode) + } + }) + + t.Run("the browser's own bookkeeping never reaches the wire", func(t *testing.T) { + got, _ := firefoxFindingWithGrants(t, addon, `{ + "ext@example-org": { + "permissions": ["internal:privateBrowsingAllowed", "internal:svgContextPropertiesAllowed"] + } + }`) + + // These sit on every add-on on the machine and say nothing about any of + // them. + for _, entry := range append(got.Permissions, got.HostPermissions...) { + if strings.HasPrefix(entry, "internal:") { + t.Errorf("payload carries %q", entry) + } + } + }) + + t.Run("an entry the database does not list is not an extension", func(t *testing.T) { + // A temporary add-on: its preferences entry survives the add-on itself, so + // a finding built from one is an alert that can never clear. The helper + // requires exactly one finding, which is the assertion. + firefoxFindingWithGrants(t, addon, `{ + "ext@example-org": {"permissions": ["cookies"]}, + "probe@example-org": {"permissions": ["debugger", "history"]} + }`) + }) + + t.Run("the scriptable subset is absent on this engine", func(t *testing.T) { + got, _ := firefoxFindingWithGrants(t, addon, `{"ext@example-org": {"origins": [""]}}`) + if got.ScriptableHostPermissions != nil { + t.Errorf("scriptable_host_permissions = %v, want absent: this engine draws no such line", + *got.ScriptableHostPermissions) + } + }) + + t.Run("a document that cannot be read is a gap and says so", func(t *testing.T) { + _, coverage := firefoxFindingWithGrants(t, addon, `{"ext@example-org": `) + if coverage.Status != model.BrowserCoveragePartial || coverage.ReasonCode != model.BrowserExtReasonParseError { + t.Errorf("status = %q/%q, want partial and the parse named", coverage.Status, coverage.ReasonCode) + } + }) + + t.Run("a document that was never written is silence", func(t *testing.T) { + // The browser writes this file when a preference is set, so a runtime grant + // cannot exist without it. Degrading here would paint a clean profile + // partial on every scan for no information lost. + _, coverage := firefoxFinding(t, addon) + if coverage.Status != model.BrowserCoverageScanned { + t.Errorf("status = %q/%q, want scanned", coverage.Status, coverage.ReasonCode) + } + }) +} + +// TestGecko_ManifestVersionAndDataCollection covers the two attributes that +// separate add-ons which otherwise read alike: what a version 2 add-on can still +// do to a request, and what the add-on said it collects. +func TestGecko_ManifestVersionAndDataCollection(t *testing.T) { + t.Run("a declared collection is not an undeclared one", func(t *testing.T) { + got, _ := firefoxFindingWithGrants(t, `{ + "id": "ext@example-org", "type": "extension", "active": true, "location": "app-profile", + "manifestVersion": 2, + "userPermissions": {"permissions": ["webRequestBlocking"], "data_collection": ["none"]} + }`, `{"ext@example-org": {"data_collection": ["healthInfo"]}}`) + + if got.ManifestVersion != 2 { + t.Errorf("manifest_version = %d, want 2: the revision that can still block requests", + got.ManifestVersion) + } + // The vocabulary is left open. Firefox adds a category per release, and a + // closed set would fail a whole browser on the next one. + want := "healthInfo,none" + if strings.Join(got.DataCollection, ",") != want { + t.Errorf("data_collection = %v, want %q", got.DataCollection, want) + } + }) + + t.Run("declaring nothing is left empty rather than spelled", func(t *testing.T) { + got, _ := firefoxFinding(t, `{ + "id": "ext@example-org", "type": "extension", "active": true, "location": "app-profile", + "manifestVersion": 3, + "userPermissions": {"permissions": ["storage"]} + }`) + + if got.ManifestVersion != 3 { + t.Errorf("manifest_version = %d, want 3", got.ManifestVersion) + } + // An empty list and a list holding "none" are different answers: one add-on + // declared it collects nothing, the other declared nothing at all. + if len(got.DataCollection) != 0 { + t.Errorf("data_collection = %v, want none", got.DataCollection) + } + }) + + t.Run("a record that does not say", func(t *testing.T) { + got, _ := firefoxFinding(t, `{ + "id": "ext@example-org", "type": "extension", "active": true, "location": "app-profile" + }`) + if got.ManifestVersion != 0 { + t.Errorf("manifest_version = %d, want 0", got.ManifestVersion) + } + }) +} + // TestGecko_OverlongIdentityFailsTheBrowser pins the one string that is never // shortened. A truncated identity is a different extension, and dropping it under // a status that claims a complete list would retire the real one's stored row. diff --git a/internal/model/browserext.go b/internal/model/browserext.go index 99b3ad4e..608670ac 100644 --- a/internal/model/browserext.go +++ b/internal/model/browserext.go @@ -5,10 +5,12 @@ package model // A finding says which extension is installed in which browser, whether it is // enabled and why not, where it came from, whether its store still lists it, and // what it is permitted to touch. It carries no browsing state: no history, no -// cookies, no passwords, no page content, no profile identity, and no -// filesystem paths. Host-access patterns do ship, because they are what an -// extension can reach and can name an internal hostname, and that is the point -// of collecting them. +// cookies, no passwords, no page content and no profile identity. Two things do +// ship that name a place: host-access patterns, because they are what an +// extension can reach and can name an internal hostname, and the load path of an +// unpacked extension, because an unreviewed extension running out of a user +// directory is the signal and the path is the legible part of it. Neither is +// ever opened. // // Two things make the shape what it is. The first is that state is stored per // extension rather than as one replaceable snapshot, so the payload has to say @@ -211,6 +213,11 @@ type BrowserExtensionFinding struct { Version string `json:"version"` + // The manifest revision the browser recorded. A version 2 extension can hold + // blocking request interception, which version 3 removed, so this is a risk + // class rather than trivia. Zero where the browser recorded none. + ManifestVersion int `json:"manifest_version,omitempty"` + EnabledState string `json:"enabled_state"` // Present exactly when EnabledState is disabled. @@ -223,6 +230,12 @@ type BrowserExtensionFinding struct { InstallSource string `json:"install_source"` + // Where an unpacked extension was loaded from, as the user pointed the + // browser at it. Empty for every other install source. Never opened and never + // resolved: it is reported because an unreviewed extension running out of a + // user directory is the signal, and a blank name is not. + InstallPath string `json:"install_path,omitempty"` + Store string `json:"store"` // The browser shipped it as a default. Still a real extension with real @@ -241,4 +254,18 @@ type BrowserExtensionFinding struct { // different grant. Permissions []string `json:"permissions"` HostPermissions []string `json:"host_permissions"` + + // The hosts the extension declared content scripts for, which the browser + // injects into automatically at page load. Not the limit of what it can + // inject into: with host access and the scripting, userScripts or debugger + // permission it can inject programmatically anywhere it holds a host. Nil + // where the engine does not record the distinction, so an empty list means + // "declared no content scripts" and an absent one means "cannot tell". + ScriptableHostPermissions *[]string `json:"scriptable_host_permissions,omitempty"` + + // What the extension declared it collects, as far as the user's grant records + // it. Gecko only. A list holding "none" is a positive declaration that it + // collects nothing, which is not the same answer as an empty list, where + // nothing was declared at all. + DataCollection []string `json:"data_collection,omitempty"` } diff --git a/internal/model/testdata/browser_extension_scan_golden.json b/internal/model/testdata/browser_extension_scan_golden.json index b4bf5be5..dc7471a5 100644 --- a/internal/model/testdata/browser_extension_scan_golden.json +++ b/internal/model/testdata/browser_extension_scan_golden.json @@ -49,7 +49,9 @@ "store": "chrome_web_store", "preinstalled": false, "permissions": ["storage", "tabs"], - "host_permissions": ["https://*/*", "http://*/*"] + "host_permissions": ["https://*/*", "http://*/*"], + "scriptable_host_permissions": ["https://*/*"], + "manifest_version": 3 }, { "browser_id": "chrome", @@ -64,7 +66,10 @@ "store": "self_hosted", "preinstalled": true, "permissions": ["cookies"], - "host_permissions": ["*://example.internal/*"] + "host_permissions": ["*://example.internal/*"], + "scriptable_host_permissions": ["*://example.internal/*"], + "manifest_version": 2, + "data_collection": ["locationInfo", "someCategoryWeHaveNotSeenBefore"] }, { "browser_id": "chrome", @@ -79,7 +84,8 @@ "store": "unknown", "preinstalled": false, "permissions": [], - "host_permissions": [] + "host_permissions": [], + "install_path": "/Users/example-user/Downloads/example-unpacked-extension" }, { "browser_id": "chrome", @@ -93,8 +99,10 @@ "install_source": "policy", "store": "edge_addons", "preinstalled": false, - "permissions": ["management"], - "host_permissions": [""] + "permissions": ["management", "scripting", "userScripts"], + "host_permissions": [""], + "scriptable_host_permissions": [], + "manifest_version": 3 }, { "browser_id": "chrome", @@ -136,7 +144,9 @@ "preinstalled": false, "signed_state": "signed", "permissions": ["webRequest"], - "host_permissions": [""] + "host_permissions": [""], + "manifest_version": 2, + "data_collection": ["technicalAndInteraction"] }, { "browser_id": "firefox", @@ -149,7 +159,8 @@ "preinstalled": false, "signed_state": "missing", "permissions": [], - "host_permissions": [] + "host_permissions": [], + "install_path": "/Users/example-user/Downloads/example-local-build" }, { "browser_id": "firefox", From 7e8323dc09b19f190e9970c40851fa6c76895f86 Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Sun, 16 Aug 2026 19:30:00 +0530 Subject: [PATCH 4/5] test(browserext): pin the four new finding fields in the golden contract The shared fixture carried scriptable hosts, manifest version, data collection and install path, but nothing checked them, so a regression that stopped emitting one would have passed here and only shown up as a missing column. Coverage now requires the fixture to keep exercising all three scriptable states, since an empty list and an absent one are different answers, both manifest versions plus an unrecorded one, and a declared data collection. The invariants a reader rejects over are checked alongside them: a scriptable host absent from host_permissions, a scriptable list on a gecko browser, and an install path on anything but an unpacked extension. --- internal/model/browserext_golden_test.go | 67 ++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/internal/model/browserext_golden_test.go b/internal/model/browserext_golden_test.go index 4612e503..d4132f36 100644 --- a/internal/model/browserext_golden_test.go +++ b/internal/model/browserext_golden_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "os" "reflect" + "slices" "testing" ) @@ -152,6 +153,49 @@ func TestBrowserExtensionScanGolden_CoversTheWholeVocabulary(t *testing.T) { } } + // The four fields whose whole value is a distinction, so a fixture that + // stops showing both sides of one leaves the reader's handling of it + // unchecked. Scriptable hosts have three states, not two: a real subset, an + // empty list meaning the extension injects nowhere, and an absent one + // meaning nothing recorded the split. + scriptableSubset, scriptableEmpty, scriptableAbsent := false, false, false + manifestVersions := map[int]bool{} + dataCollection, installPath := false, false + for _, f := range info.Findings { + switch { + case f.ScriptableHostPermissions == nil: + scriptableAbsent = true + case len(*f.ScriptableHostPermissions) == 0: + scriptableEmpty = true + default: + scriptableSubset = true + } + manifestVersions[f.ManifestVersion] = true + if len(f.DataCollection) > 0 { + dataCollection = true + } + if f.InstallPath != "" { + installPath = true + } + } + for _, tt := range []struct { + got bool + want string + }{ + {scriptableSubset, "a finding whose scriptable_host_permissions names hosts"}, + {scriptableEmpty, "a finding that injects nowhere, whose scriptable_host_permissions is empty"}, + {scriptableAbsent, "a finding with no scriptable_host_permissions, where the split was not recorded"}, + {manifestVersions[2], "a manifest version 2 finding, which can hold blocking request interception"}, + {manifestVersions[3], "a manifest version 3 finding"}, + {manifestVersions[0], "a finding whose manifest version the browser never recorded"}, + {dataCollection, "a finding declaring data_collection"}, + {installPath, "an unpacked finding carrying its install_path"}, + } { + if !tt.got { + t.Errorf("golden payload must carry %s", tt.want) + } + } + // A reduced finding: metadata that could not be recovered, identity that // could. A reader that requires a name drops a real extension on this row. reduced := false @@ -240,6 +284,29 @@ func TestBrowserExtensionScanGolden_HonoursTheCoverageInvariants(t *testing.T) { t.Errorf("%s: store fields %q/%q on browser %q", f.ExtensionID, f.StoreListing, f.StoreViolation, f.BrowserID) } + if f.ScriptableHostPermissions != nil { + // Gecko keeps one origins list, so a list here would be an answer + // the engine never gave. + if gecko { + t.Errorf("%s: scriptable_host_permissions on gecko browser %q", f.ExtensionID, f.BrowserID) + } + // A host that can be injected into is one the extension reaches, so + // a scriptable entry missing from host_permissions understates the + // row it appears on. + for _, h := range *f.ScriptableHostPermissions { + if !slices.Contains(f.HostPermissions, h) { + t.Errorf("%s: scriptable host %q is absent from host_permissions", f.ExtensionID, h) + } + } + } + // The path is the load location of an unpacked extension and means + // nothing on any other install source. + if f.InstallPath != "" && f.InstallSource != BrowserExtInstallUnpacked { + t.Errorf("%s: install_path on install_source %q", f.ExtensionID, f.InstallSource) + } + if f.ManifestVersion < 0 { + t.Errorf("%s: manifest_version %d", f.ExtensionID, f.ManifestVersion) + } } } From 9aa8f32d67352b13b80b536d52741b7789c695c1 Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Mon, 17 Aug 2026 00:29:16 +0530 Subject: [PATCH 5/5] fix(browserext): rank profile occurrences by state and access An extension installed in several profiles reduces to one finding, and the whole record was taken from the profile whose path sorted first. A profile with site access withheld could therefore supply the permission lists while another profile held broad access, so the finding under-reported what the machine can reach. Rank occurrences instead: enabled state first, then broad host access, broad scriptable access, host count, permission count, and the profile path last as a total tiebreak so repeat scans stay identical. Ranking by state first makes the winning record's own state equal the state the row reports, so version, store and permissions always describe a profile the row really is in. Broad host access uses the same rule the reader applies, including the http and https pattern pair, so both sides agree on what breadth means. --- internal/detector/browserext/catalog.go | 28 ++++ internal/detector/browserext/detector.go | 58 +++++++- internal/detector/browserext/detector_test.go | 132 +++++++++++++++++- internal/detector/browserext/gecko_test.go | 5 + internal/model/browserext.go | 4 +- 5 files changed, 219 insertions(+), 8 deletions(-) diff --git a/internal/detector/browserext/catalog.go b/internal/detector/browserext/catalog.go index a42be9cd..4c9aad39 100644 --- a/internal/detector/browserext/catalog.go +++ b/internal/detector/browserext/catalog.go @@ -179,6 +179,34 @@ const ( maxPermissionsPerFinding = 64 // API permissions and host patterns each ) +// broadHostPatterns, together with the http/https pair below, is the same rule +// the reader applies to decide broad host access. The two move together: a +// pattern added on one side is a disagreement about what breadth means until it +// is added on the other. Exact equality, never a judgment call about equivalent +// patterns, because a host pattern is matched literally. +var broadHostPatterns = map[string]struct{}{ + "": {}, + "*://*/*": {}, + "file://*/*": {}, +} + +// hasBroadHosts reports whether granted host patterns amount to whole-web reach. +func hasBroadHosts(hosts []string) bool { + var http, https bool + for _, pattern := range hosts { + if _, ok := broadHostPatterns[pattern]; ok { + return true + } + switch pattern { + case "http://*/*": + http = true + case "https://*/*": + https = true + } + } + return http && https +} + // String caps in BYTES, not runes: the wire and the reader's caps are // byte-denominated, so the producer's have to be too. Truncation backs up to a // rune boundary so a capped string is still valid UTF-8. diff --git a/internal/detector/browserext/detector.go b/internal/detector/browserext/detector.go index d1982571..682479c8 100644 --- a/internal/detector/browserext/detector.go +++ b/internal/detector/browserext/detector.go @@ -328,9 +328,9 @@ func (d *Detector) scanRoot(ctx context.Context, scan *scanState, spec browserSp // the wire — their names are user-chosen text and per-profile state was not the // ask — so they exist only to drive enumeration and this reduction. type occurrence struct { - // sortKey orders the occurrences of one extension: the data directory and - // then the profile directory. Arbitrary but fixed, which is what makes two - // runs over an unchanged machine emit identical findings. + // sortKey is the last tiebreak between the occurrences of one extension: the + // data directory and then the profile directory. Arbitrary but fixed, which + // is what makes two runs over an unchanged machine emit identical findings. sortKey string enabled string disabledBy string @@ -408,6 +408,56 @@ func (b *browserScan) add(id string, occ occurrence) bool { return true } +// stateRank orders the states the way the fold's union loop resolves them: +// enabled in any profile wins, and a disabled profile still beats one whose +// state could not be read. +func stateRank(state string) int { + switch state { + case model.BrowserExtEnabled: + return 2 + case model.BrowserExtDisabled: + return 1 + default: + return 0 + } +} + +// lessOccurrence orders one extension's occurrences so the first one describes +// access the machine really has. The block is taken whole from the first, so +// these keys rank whole profiles and never a field. +// +// State ranks first, by the same rule the union loop uses. That is an +// invariant rather than a preference: the first occurrence is a maximum by +// state, so its own state equals the state the loop resolves, and the version, +// store and permissions under it belong to a profile that really is in it. +func lessOccurrence(a, b occurrence) bool { + if ra, rb := stateRank(a.enabled), stateRank(b.enabled); ra != rb { + return ra > rb + } + // Breadth, not count: granted in one profile outranks twenty + // narrow patterns in another. + if ba, bb := hasBroadHosts(a.block.HostPermissions), hasBroadHosts(b.block.HostPermissions); ba != bb { + return ba + } + // Content scripts come from the manifest and usually match across profiles, + // but profiles can sit on different versions. A nil list is nothing read + // rather than nothing injected, and ranks as not broad either way. + sa := a.block.ScriptableHostPermissions != nil && hasBroadHosts(*a.block.ScriptableHostPermissions) + sb := b.block.ScriptableHostPermissions != nil && hasBroadHosts(*b.block.ScriptableHostPermissions) + if sa != sb { + return sa + } + if la, lb := len(a.block.HostPermissions), len(b.block.HostPermissions); la != lb { + return la > lb + } + if la, lb := len(a.block.Permissions), len(b.block.Permissions); la != lb { + return la > lb + } + // Last and total: this is what makes two runs over an unchanged machine + // emit identical findings. + return a.sortKey < b.sortKey +} + // fold reduces every extension's occurrences to one finding. func (b *browserScan) fold(browserID string) []model.BrowserExtensionFinding { ids := make([]string, 0, len(b.occurrences)) @@ -419,7 +469,7 @@ func (b *browserScan) fold(browserID string) []model.BrowserExtensionFinding { out := make([]model.BrowserExtensionFinding, 0, len(ids)) for _, id := range ids { occs := b.occurrences[id] - sort.SliceStable(occs, func(i, j int) bool { return occs[i].sortKey < occs[j].sortKey }) + sort.SliceStable(occs, func(i, j int) bool { return lessOccurrence(occs[i], occs[j]) }) f := occs[0].block f.BrowserID = browserID diff --git a/internal/detector/browserext/detector_test.go b/internal/detector/browserext/detector_test.go index de8d3800..c26e6fdc 100644 --- a/internal/detector/browserext/detector_test.go +++ b/internal/detector/browserext/detector_test.go @@ -592,15 +592,141 @@ func TestDetect_FoldsOneExtensionSeenInTwoProfiles(t *testing.T) { if f.DisabledBy != "" { t.Errorf("disabled_by = %q, want none on an enabled extension", f.DisabledBy) } - // The first-sorting profile owns the block, so both fields come from it. - if f.Version != "1.0.0" || len(f.Permissions) != 1 || f.Permissions[0] != "storage" { - t.Errorf("version/permissions = %q/%v, want one profile's whole record", f.Version, f.Permissions) + // The block comes from the profile the state describes, not from the + // first-sorting one, so both fields come from the enabled profile. + if f.Version != "2.0.0" || len(f.Permissions) != 1 || f.Permissions[0] != "tabs" { + t.Errorf("version/permissions = %q/%v, want the enabled profile's whole record", f.Version, f.Permissions) } if got := coverageFor(t, info, browserChrome); got.ProfileCount != 2 { t.Errorf("profile_count = %d, want 2", got.ProfileCount) } } +// TestDetect_GrantedProfileOwnsTheAccessLists is the shape that made the ranking +// necessary: the same extension in two profiles, host access withheld in the +// first-sorting one and granted in the other. Reporting the withheld profile +// says the machine has no broad access when it does, which is the wrong +// direction for the reader to be wrong in. +func TestDetect_GrantedProfileOwnsTheAccessLists(t *testing.T) { + home := tempHome(t) + localState(t, chromeRoot(home), "Default", "Profile 1") + securePrefs(t, chromeRoot(home), "Default", `"`+idA+`": { + "location": 1, "withholding_permissions": true, + "manifest": {"name": "Example Blocker", "version": "1.0.0"}, + "granted_permissions": {"api": ["storage"], "explicit_host": [""]} + }`) + securePrefs(t, chromeRoot(home), "Profile 1", `"`+idA+`": { + "location": 1, + "manifest": {"name": "Example Blocker", "version": "1.0.0"}, + "granted_permissions": {"api": ["storage"], "explicit_host": [""]} + }`) + + got := findingsFor(scanHome(t, home), browserChrome) + if len(got) != 1 { + t.Fatalf("findings = %d, want one", len(got)) + } + if len(got[0].HostPermissions) != 1 || got[0].HostPermissions[0] != "" { + t.Errorf("host_permissions = %v, want the granted profile's access", got[0].HostPermissions) + } + // It declares no content scripts in either profile, so an empty list here is + // the right answer and a filled one would be a different wrong one. + if got[0].ScriptableHostPermissions == nil || len(*got[0].ScriptableHostPermissions) != 0 { + t.Errorf("scriptable_host_permissions = %v, want the empty list it declared", got[0].ScriptableHostPermissions) + } +} + +// TestLessOccurrence covers the ranking that decides which profile's record one +// finding carries, key by key. State first, then breadth of access, and the path +// only once nothing about the access itself separates them. +func TestLessOccurrence(t *testing.T) { + list := func(entries ...string) *[]string { return &entries } + occ := func(sortKey, state string, hosts []string, scriptable *[]string, perms ...string) occurrence { + return occurrence{sortKey: sortKey, enabled: state, block: model.BrowserExtensionFinding{ + Permissions: perms, + HostPermissions: hosts, + ScriptableHostPermissions: scriptable, + }} + } + specific := []string{"https://a.internal/*", "https://b.internal/*", "https://c.internal/*"} + + // In every row the first occurrence is the one that should own the block, and + // it sorts second by path so the old rule would have picked the other. + tests := []struct { + name string + first occurrence + second occurrence + }{{ + name: "enabled wins however narrow, because the row says enabled", + first: occ("b", model.BrowserExtEnabled, nil, nil), + second: occ("a", model.BrowserExtDisabled, []string{""}, nil), + }, { + name: "a disabled profile still beats one whose state could not be read", + first: occ("b", model.BrowserExtDisabled, nil, nil), + second: occ("a", model.BrowserExtStateUnknown, []string{""}, nil), + }, { + name: "breadth is not count", + first: occ("b", model.BrowserExtEnabled, []string{""}, nil), + second: occ("a", model.BrowserExtEnabled, specific, nil), + }, { + name: "http and https together are the whole web", + first: occ("b", model.BrowserExtEnabled, []string{"http://*/*", "https://*/*"}, nil), + second: occ("a", model.BrowserExtEnabled, specific, nil), + }, { + name: "one half of the pair is not broad, so count decides", + first: occ("b", model.BrowserExtEnabled, specific, nil), + second: occ("a", model.BrowserExtEnabled, []string{"https://*/*"}, nil), + }, { + name: "all disabled: breadth still decides, so a broad grant is reported", + first: occ("b", model.BrowserExtDisabled, []string{""}, nil), + second: occ("a", model.BrowserExtDisabled, specific, nil), + }, { + name: "broad content scripts break a tie the host lists do not", + first: occ("b", model.BrowserExtEnabled, []string{""}, list("")), + second: occ("a", model.BrowserExtEnabled, []string{""}, list()), + }, { + name: "an unrecorded scriptable list is not a broad one", + first: occ("b", model.BrowserExtEnabled, []string{""}, list("")), + second: occ("a", model.BrowserExtEnabled, []string{""}, nil), + }, { + name: "with neither broad, the wider host list wins", + first: occ("b", model.BrowserExtEnabled, specific, nil), + second: occ("a", model.BrowserExtEnabled, specific[:1], nil), + }, { + name: "a runtime-granted API permission beats a path comparison", + first: occ("b", model.BrowserExtEnabled, specific, nil, "storage", "tabs"), + second: occ("a", model.BrowserExtEnabled, specific, nil, "storage"), + }, { + name: "everything equal: the path decides, and it is total", + first: occ("a", model.BrowserExtEnabled, specific, nil), + second: occ("b", model.BrowserExtEnabled, specific, nil), + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if !lessOccurrence(tt.first, tt.second) { + t.Error("the occurrence that describes the machine's access did not sort first") + } + if lessOccurrence(tt.second, tt.first) { + t.Error("both orderings compared less, so the ordering is not a strict one") + } + + // The invariant: whichever way round the two arrive, the block the + // fold takes belongs to a profile in the state the fold resolves. + // It is what stops this comparator and that union loop drifting. + for _, occs := range [][]occurrence{{tt.first, tt.second}, {tt.second, tt.first}} { + b := &browserScan{occurrences: map[string][]occurrence{idA: occs}} + out := b.fold(browserChrome) + if len(out) != 1 { + t.Fatalf("findings = %d, want one", len(out)) + } + if winner := b.occurrences[idA][0]; winner.enabled != out[0].EnabledState { + t.Errorf("block came from a %q profile on a %q row", winner.enabled, out[0].EnabledState) + } + } + }) + } +} + // TestDetect_DisabledCauseComesFromADisabledProfile is the trap in the reduction: // the profile that owns the record may be an enabled one, and reading the cause // off it would attach an enabled profile's empty cause to a disabled row. diff --git a/internal/detector/browserext/gecko_test.go b/internal/detector/browserext/gecko_test.go index 86956375..c9a8161f 100644 --- a/internal/detector/browserext/gecko_test.go +++ b/internal/detector/browserext/gecko_test.go @@ -511,6 +511,11 @@ func TestGecko_TwoDataDirectoriesDeduplicate(t *testing.T) { if got[0].EnabledState != model.BrowserExtEnabled { t.Errorf("enabled_state = %q, want enabled: it runs in one of the two", got[0].EnabledState) } + // The reduction is shared with chromium, so the record comes from the + // directory whose state the row describes, not the first-sorting one. + if got[0].Version != "2.0" { + t.Errorf("version = %q, want the enabled directory's record", got[0].Version) + } if c := coverageFor(t, info, browserFirefox); c.ProfileCount != 2 { t.Errorf("profile_count = %d, want both directories' profiles counted together", c.ProfileCount) } diff --git a/internal/model/browserext.go b/internal/model/browserext.go index 608670ac..ea9661d3 100644 --- a/internal/model/browserext.go +++ b/internal/model/browserext.go @@ -264,7 +264,9 @@ type BrowserExtensionFinding struct { ScriptableHostPermissions *[]string `json:"scriptable_host_permissions,omitempty"` // What the extension declared it collects, as far as the user's grant records - // it. Gecko only. A list holding "none" is a positive declaration that it + // it. Written by gecko, the only engine that records the concept, though the + // wire carries it on any engine. A list holding "none" is a positive + // declaration that it // collects nothing, which is not the same answer as an empty list, where // nothing was declared at all. DataCollection []string `json:"data_collection,omitempty"`