P2 mobile firewall scope (#8639)

* fix(mobile): validate Windows firewall remote scope

* fix(review): simplify string-guard ternary to boolean AND

The ternary returned only boolean literals, so cond ? f() : false is
equivalent to cond && f() (addressScopeIsSufficient returns boolean).

Co-authored-by: Orca <help@stably.ai>

* fix(mobile): accept dotted-netmask firewall scopes and lock in fail-safe edges

- Parse dotted-netmask CIDR (192.168.0.0/255.255.255.0) via a contiguous-mask
  check, failing closed on holey masks.
- Factor subnetFromParsed so CIDR parsing no longer re-parses the address.
- Document why single-host (/32, /128) subnets and family-specific keywords with
  an unknown interface family fail closed, and add regression tests covering
  those deliberate false-deny edges plus policy-defined keywords (Intranet, DNS).

Co-authored-by: Orca <help@stably.ai>

* Add explanatory comment on why firewall scope check isn't unioned

Documents the fail-safe rationale behind checking coverage per rule
instead of merging rule scopes, so future edits don't "fix" this
into a less conservative union check.

---------

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing 2026-07-13 19:02:52 -07:00 committed by GitHub
parent 302b97029a
commit 01d7cd779f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 435 additions and 7 deletions

View File

@ -0,0 +1,155 @@
import { describe, expect, it } from 'vitest'
import { hasSufficientWindowsFirewallRemoteScope } from './windows-firewall-remote-scope'
type RuleScope = { remoteAddresses: unknown }
function rule(remoteAddresses: unknown): RuleScope {
return { remoteAddresses }
}
describe('Windows firewall remote-address scope', () => {
it.each([['Any'], ['any'], ['LocalSubnet']])('accepts the documented %s scope', (scope) => {
expect(hasSufficientWindowsFirewallRemoteScope([rule(scope)], undefined, undefined)).toBe(true)
})
it.each([['Any4'], ['LocalSubnet4']])(
'accepts address-family-specific %s on the selected IPv4 interface',
(scope) => {
expect(hasSufficientWindowsFirewallRemoteScope([rule([scope])], '192.168.0.108', 24)).toBe(
true
)
}
)
it('keeps address-family-specific keywords on the selected interface family', () => {
expect(hasSufficientWindowsFirewallRemoteScope([rule(['Any6'])], 'fd7a:115c:a1e0::5', 64)).toBe(
true
)
expect(
hasSufficientWindowsFirewallRemoteScope([rule(['LocalSubnet6'])], 'fd7a:115c:a1e0::5', 64)
).toBe(true)
expect(
hasSufficientWindowsFirewallRemoteScope([rule(['LocalSubnet6'])], '192.168.0.108', 24)
).toBe(false)
})
it.each([
['192.168.0.0/24', '192.168.0.108', 24],
['192.168.0.0-192.168.0.255', '192.168.0.108', 24],
['fd7a:115c:a1e0::/64', 'fd7a:115c:a1e0::5', 64],
['fd7a:115c:a1e0::-fd7a:115c:a1e0:0:ffff:ffff:ffff:ffff', 'fd7a:115c:a1e0::5', 64]
])('accepts explicit scope %s covering the selected local subnet', (scope, address, prefix) => {
expect(hasSufficientWindowsFirewallRemoteScope([rule([scope])], address, prefix)).toBe(true)
})
it.each([
['192.168.1.0/24', '192.168.0.108', 24],
['192.168.0.64/26', '192.168.0.108', 24],
['192.168.0.108', '192.168.0.108', 24],
['fd7a:115c:a1e1::/64', 'fd7a:115c:a1e0::5', 64],
['Internet', '192.168.0.108', 24]
])('rejects restrictive or unsupported scope %s', (scope, address, prefix) => {
expect(hasSufficientWindowsFirewallRemoteScope([rule([scope])], address, prefix)).toBe(false)
})
it('does not infer explicit scope coverage without selected interface subnet data', () => {
expect(
hasSufficientWindowsFirewallRemoteScope([rule(['192.168.0.0/24'])], undefined, undefined)
).toBe(false)
expect(
hasSufficientWindowsFirewallRemoteScope([rule(['192.168.0.0/24'])], '192.168.0.108', 40)
).toBe(false)
expect(
hasSufficientWindowsFirewallRemoteScope([rule(['100.64.0.0/10'])], '100.64.1.20', 32)
).toBe(false)
})
it.each([
undefined,
null,
[],
{},
[rule(undefined)],
[rule([])],
[rule([''])],
[rule(['not-an-address'])],
[{ remoteAddresses: [42] }]
])('rejects malformed or empty structured output %#', (rules) => {
expect(hasSufficientWindowsFirewallRemoteScope(rules, '192.168.0.108', 24)).toBe(false)
})
it('evaluates each rule independently instead of merging partial ranges', () => {
expect(
hasSufficientWindowsFirewallRemoteScope(
[rule(['192.168.0.0-192.168.0.127']), rule(['192.168.0.128-192.168.0.255'])],
'192.168.0.108',
24
)
).toBe(false)
expect(
hasSufficientWindowsFirewallRemoteScope(
[rule(['192.168.1.0/24']), rule(['192.168.0.0/24'])],
'192.168.0.108',
24
)
).toBe(true)
})
it('accepts PowerShell single-object and single-string JSON shapes', () => {
expect(
hasSufficientWindowsFirewallRemoteScope(
{ remoteAddresses: '192.168.0.0/24' },
'192.168.0.108',
24
)
).toBe(true)
})
it('accepts dotted-netmask CIDR with a contiguous mask and rejects a holey one', () => {
expect(
hasSufficientWindowsFirewallRemoteScope(
[rule(['192.168.0.0/255.255.255.0'])],
'192.168.0.108',
24
)
).toBe(true)
expect(
hasSufficientWindowsFirewallRemoteScope(
[rule(['192.168.0.0/255.0.255.0'])],
'192.168.0.108',
24
)
).toBe(false)
})
it('treats a single-host (/32) interface as coverable only by Any/LocalSubnet keywords', () => {
// A /32 subnet is just the desktop itself, so an explicit range cannot prove
// the phone (a different host) is allowed — only the keywords can.
expect(hasSufficientWindowsFirewallRemoteScope([rule(['Any'])], '100.64.1.20', 32)).toBe(true)
expect(hasSufficientWindowsFirewallRemoteScope([rule(['LocalSubnet'])], '100.64.1.20', 32)).toBe(
true
)
expect(
hasSufficientWindowsFirewallRemoteScope([rule(['100.64.0.0/10'])], '100.64.1.20', 32)
).toBe(false)
})
it('fails address-family keywords closed when the interface family is unknown', () => {
expect(hasSufficientWindowsFirewallRemoteScope([rule(['Any'])], undefined, undefined)).toBe(true)
expect(hasSufficientWindowsFirewallRemoteScope([rule(['Any4'])], undefined, undefined)).toBe(
false
)
expect(hasSufficientWindowsFirewallRemoteScope([rule(['Any6'])], undefined, undefined)).toBe(
false
)
})
it.each([['Intranet'], ['DNS'], ['DHCP'], ['DefaultGateway'], ['PlayToDevice']])(
'fails the policy-defined %s keyword closed rather than assuming subnet coverage',
(scope) => {
expect(hasSufficientWindowsFirewallRemoteScope([rule([scope])], '192.168.0.108', 24)).toBe(
false
)
}
)
})

View File

@ -0,0 +1,216 @@
type IpVersion = 4 | 6
type ParsedIpAddress = {
version: IpVersion
bits: 32 | 128
value: bigint
}
type IpRange = {
version: IpVersion
start: bigint
end: bigint
}
export function hasSufficientWindowsFirewallRemoteScope(
ruleScopes: unknown,
localAddress: unknown,
localPrefixLength: unknown
): boolean {
const rules = Array.isArray(ruleScopes) ? ruleScopes : [ruleScopes]
const localSubnet = parseSubnet(localAddress, localPrefixLength)
// Why: coverage is checked per scope, never unioned across rules — this
// advisory check fails safe, so accepting fragmented rules only adds risk.
return rules.some((rule) => ruleHasSufficientScope(rule, localSubnet))
}
function ruleHasSufficientScope(rule: unknown, localSubnet: IpRange | null): boolean {
if (!isRecord(rule)) {
return false
}
const addresses = Array.isArray(rule.remoteAddresses)
? rule.remoteAddresses
: [rule.remoteAddresses]
return addresses.some(
(address) => typeof address === 'string' && addressScopeIsSufficient(address, localSubnet)
)
}
function addressScopeIsSufficient(scope: string, localSubnet: IpRange | null): boolean {
const normalized = scope.trim().toLowerCase()
if (normalized === 'any' || normalized === 'localsubnet') {
return true
}
if (
normalized === 'any4' ||
normalized === 'any6' ||
normalized === 'localsubnet4' ||
normalized === 'localsubnet6'
) {
// Why: these keywords cover a single family, so they need the selected
// interface's family; an unknown family (null subnet) fails closed.
return localSubnet?.version === (normalized.endsWith('4') ? 4 : 6)
}
if (!localSubnet) {
return false
}
// Why: without a phone IP we can only prove coverage when the rule spans the
// whole selected subnet; a single-host subnet (/32, /128 VPN/Tailscale) is just
// the desktop, so start !== end blocks a desktop-only rule from a false-allow.
const explicitRange = parseIpRange(scope)
return (
localSubnet.start !== localSubnet.end &&
explicitRange?.version === localSubnet.version &&
explicitRange.start <= localSubnet.start &&
explicitRange.end >= localSubnet.end
)
}
function parseSubnet(address: unknown, prefixLength: unknown): IpRange | null {
if (typeof address !== 'string' || typeof prefixLength !== 'number') {
return null
}
const parsed = parseIpAddress(address)
return parsed ? subnetFromParsed(parsed, prefixLength) : null
}
function subnetFromParsed(parsed: ParsedIpAddress, prefixLength: number): IpRange | null {
if (!Number.isInteger(prefixLength) || prefixLength < 0 || prefixLength > parsed.bits) {
return null
}
const hostBits = BigInt(parsed.bits - prefixLength)
const hostMask = hostBits === 0n ? 0n : (1n << hostBits) - 1n
const start = parsed.value & ~hostMask
return { version: parsed.version, start, end: start | hostMask }
}
// Why: Windows also accepts dotted-netmask CIDR (192.168.0.0/255.255.255.0);
// convert a contiguous mask to a prefix length and fail closed on holey masks.
function maskPrefixLength(maskText: string, version: IpVersion): number | null {
const mask = parseIpAddress(maskText)
if (!mask || mask.version !== version) {
return null
}
const fullMask = (1n << BigInt(mask.bits)) - 1n
const hostPart = ~mask.value & fullMask
if ((hostPart & (hostPart + 1n)) !== 0n) {
return null
}
let hostBits = 0
for (let remaining = hostPart; remaining > 0n; remaining >>= 1n) {
hostBits += 1
}
return mask.bits - hostBits
}
function parseIpRange(scope: string): IpRange | null {
const trimmed = scope.trim()
const dashIndex = trimmed.indexOf('-')
if (dashIndex >= 0) {
if (dashIndex !== trimmed.lastIndexOf('-')) {
return null
}
const start = parseIpAddress(trimmed.slice(0, dashIndex))
const end = parseIpAddress(trimmed.slice(dashIndex + 1))
if (!start || !end || start.version !== end.version || start.value > end.value) {
return null
}
return { version: start.version, start: start.value, end: end.value }
}
const slashIndex = trimmed.indexOf('/')
if (slashIndex >= 0) {
if (slashIndex !== trimmed.lastIndexOf('/')) {
return null
}
const address = parseIpAddress(trimmed.slice(0, slashIndex))
if (!address) {
return null
}
const suffix = trimmed.slice(slashIndex + 1)
const prefixLength = /^\d+$/.test(suffix)
? Number(suffix)
: maskPrefixLength(suffix, address.version)
return prefixLength === null ? null : subnetFromParsed(address, prefixLength)
}
const address = parseIpAddress(trimmed)
return address ? { version: address.version, start: address.value, end: address.value } : null
}
function parseIpAddress(input: string): ParsedIpAddress | null {
const trimmed = input.trim()
const bracketed = trimmed.startsWith('[') || trimmed.endsWith(']')
if (bracketed && !(trimmed.startsWith('[') && trimmed.endsWith(']'))) {
return null
}
const address = (bracketed ? trimmed.slice(1, -1) : trimmed).split('%', 1)[0] ?? ''
return address.includes(':') ? parseIpv6(address) : parseIpv4(address)
}
function parseIpv4(address: string): ParsedIpAddress | null {
const octets = address.split('.')
if (octets.length !== 4 || octets.some((octet) => !/^\d{1,3}$/.test(octet))) {
return null
}
const values = octets.map(Number)
if (values.some((octet) => octet > 255)) {
return null
}
const value = values.reduce((result, octet) => (result << 8n) | BigInt(octet), 0n)
return { version: 4, bits: 32, value }
}
function parseIpv6(address: string): ParsedIpAddress | null {
const expandedAddress = expandEmbeddedIpv4(address)
if (!expandedAddress) {
return null
}
const halves = expandedAddress.split('::')
if (halves.length > 2) {
return null
}
const left = splitIpv6Half(halves[0] ?? '')
const right = splitIpv6Half(halves[1] ?? '')
if (!left || !right) {
return null
}
const hasCompression = halves.length === 2
const missingGroups = 8 - left.length - right.length
if ((!hasCompression && missingGroups !== 0) || (hasCompression && missingGroups < 1)) {
return null
}
const groups = [...left, ...Array<string>(missingGroups).fill('0'), ...right]
const value = groups.reduce((result, group) => (result << 16n) | BigInt(`0x${group}`), 0n)
return { version: 6, bits: 128, value }
}
function expandEmbeddedIpv4(address: string): string | null {
if (!address.includes('.')) {
return address
}
const lastColon = address.lastIndexOf(':')
const ipv4 = parseIpv4(address.slice(lastColon + 1))
if (lastColon < 0 || !ipv4) {
return null
}
const high = ((ipv4.value >> 16n) & 0xffffn).toString(16)
const low = (ipv4.value & 0xffffn).toString(16)
return `${address.slice(0, lastColon)}:${high}:${low}`
}
function splitIpv6Half(half: string): string[] | null {
if (half === '') {
return []
}
const groups = half.split(':')
return groups.every((group) => /^[\da-f]{1,4}$/i.test(group)) ? groups : null
}
function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value)
}

View File

@ -24,7 +24,9 @@ describe('windows mobile firewall', () => {
it('inspects the exact executable, port, and selected interface profile', async () => {
const runPowerShell = vi.fn().mockResolvedValue(
JSON.stringify({
ruleAllowed: true,
matchingRuleScopes: [{ remoteAddresses: ['192.168.0.0/24'] }],
localAddress: '192.168.0.108',
localPrefixLength: 24,
privateFirewallEnabled: true,
networkCategory: 'Private'
})
@ -47,6 +49,29 @@ describe('windows mobile firewall', () => {
expect(script).toContain("C:\\Users\\O''Brien\\Orca\\Orca.exe")
expect(script).toContain("$profile -match 'Private'")
expect(script).toContain("Get-NetIPAddress -IPAddress '192.168.0.108'")
expect(script).toContain('Get-NetFirewallAddressFilter')
expect(script).toContain('remoteAddresses = @($addressFilter.RemoteAddress')
expect(script).toContain('$localPrefixLength = [int]$ip.PrefixLength')
})
it('does not accept a qualifying rule whose remote-address scope excludes the phone subnet', async () => {
const runPowerShell = vi.fn().mockResolvedValue(
JSON.stringify({
matchingRuleScopes: [{ remoteAddresses: ['192.168.1.0/24'] }],
localAddress: '192.168.0.108',
localPrefixLength: 24,
privateFirewallEnabled: true,
networkCategory: 'Private'
})
)
await expect(
inspectWindowsMobileFirewall(6768, '192.168.0.108', environment(runPowerShell))
).resolves.toMatchObject({
supported: true,
ruleAllowed: false,
inspectionAvailable: true
})
})
it('does not support non-Windows or unpackaged development builds', async () => {
@ -81,6 +106,22 @@ describe('windows mobile firewall', () => {
})
})
it('returns an actionable status for malformed or empty PowerShell output', async () => {
for (const stdout of ['', 'not json']) {
await expect(
inspectWindowsMobileFirewall(
6768,
undefined,
environment(vi.fn().mockResolvedValue(stdout))
)
).resolves.toMatchObject({
supported: true,
ruleAllowed: false,
inspectionAvailable: false
})
}
})
it('repairs only Orca mobile pairing on private networks after elevation', async () => {
const runPowerShell = vi.fn().mockResolvedValue('{"launched":true,"exitCode":0}')
await expect(repairWindowsMobileFirewall(6769, environment(runPowerShell))).resolves.toEqual({

View File

@ -5,6 +5,7 @@ import type {
WindowsMobileFirewallStatus,
WindowsNetworkCategory
} from '../../shared/windows-mobile-firewall'
import { hasSufficientWindowsFirewallRemoteScope } from './windows-firewall-remote-scope'
const FIREWALL_RULE_NAME = 'Orca.MobilePairing'
const FIREWALL_RULE_DISPLAY_NAME = 'Orca Mobile Pairing'
@ -22,7 +23,9 @@ export type WindowsMobileFirewallEnvironment = {
}
type FirewallInspection = {
ruleAllowed: boolean
matchingRuleScopes?: unknown
localAddress?: unknown
localPrefixLength?: unknown
privateFirewallEnabled: boolean
networkCategory: string
}
@ -51,7 +54,11 @@ export async function inspectWindowsMobileFirewall(
return {
supported: true,
port,
ruleAllowed: result.ruleAllowed === true,
ruleAllowed: hasSufficientWindowsFirewallRemoteScope(
result.matchingRuleScopes,
result.localAddress,
result.localPrefixLength
),
privateFirewallEnabled: result.privateFirewallEnabled !== false,
networkCategory: parseNetworkCategory(result.networkCategory),
inspectionAvailable: true
@ -152,12 +159,16 @@ function buildInspectionScript(port: number, executablePath: string, address?: s
? `
try {
$ip = Get-NetIPAddress -IPAddress ${quotePowerShell(address)} -ErrorAction Stop | Select-Object -First 1
$localAddress = [string]$ip.IPAddress
$localPrefixLength = [int]$ip.PrefixLength
$profile = Get-NetConnectionProfile -InterfaceIndex $ip.InterfaceIndex -ErrorAction Stop | Select-Object -First 1
if ($profile) { $networkCategory = [string]$profile.NetworkCategory }
} catch {}`
: ''
// Why: NetSecurity filter properties are stable across localized Windows
// display output and keep every rule's address scope independent.
return `$ErrorActionPreference = 'Stop'
$ruleAllowed = $false
$matchingRuleScopes = @()
$rules = @(Get-NetFirewallApplicationFilter -Program ${quotePowerShell(executablePath)} -ErrorAction SilentlyContinue | Get-NetFirewallRule | Where-Object { $_.Enabled -eq 'True' -and $_.Direction -eq 'Inbound' -and $_.Action -eq 'Allow' })
foreach ($rule in $rules) {
$portFilter = $rule | Get-NetFirewallPortFilter
@ -165,16 +176,21 @@ foreach ($rule in $rules) {
$profile = [string]$rule.Profile
$portMatches = @($portFilter.LocalPort | Where-Object { [string]$_ -eq 'Any' -or [string]$_ -eq '${port}' }).Count -gt 0
if (($protocol -eq 'Any' -or $protocol -eq 'TCP' -or $protocol -eq '6') -and ($profile -eq 'Any' -or $profile -match 'Private') -and $portMatches) {
$ruleAllowed = $true
$addressFilter = $rule | Get-NetFirewallAddressFilter
$matchingRuleScopes += [pscustomobject]@{
remoteAddresses = @($addressFilter.RemoteAddress | ForEach-Object { [string]$_ })
}
}
}
$privateFirewallEnabled = [bool](Get-NetFirewallProfile -Name Private).Enabled
$networkCategory = 'Unknown'${addressLookup}
[pscustomobject]@{
ruleAllowed = $ruleAllowed
matchingRuleScopes = @($matchingRuleScopes)
localAddress = $localAddress
localPrefixLength = $localPrefixLength
privateFirewallEnabled = $privateFirewallEnabled
networkCategory = $networkCategory
} | ConvertTo-Json -Compress`
} | ConvertTo-Json -Depth 4 -Compress`
}
function buildRepairScript(port: number, executablePath: string): string {