fix(security): re-probe hardening instead of latching a transient failure

The per-process attempt cap added for the read-path storm was a permanent
latch: one AV scan, momentary lock or %TEMP% blip and every later credential
write in that session went unhardened, silently, on a host where hardening
would now succeed. Same defect class as #17858's computer-use host, and
worse here because what stops happening is security hardening on credential
files and nothing said so.

The retry budget now bounds the *rate*, not the lifetime: at most three
attempts per path per minute, re-probing in every later window, forever. The
transition is announced in both directions — `throttled` once per window on
entry, `recovered` when a rate-limited path hardens again — so a host stuck
in the degraded state is diagnosable rather than merely quiet. The reporter
type covers both, and the main process ends the `recovered` span
successfully rather than failing it.

Extracted to secure-path-hardening-retry-budget.ts, which keeps
secure-file.ts under its line cap without a max-lines disable.

Also confirms the second flagged risk rather than assuming it: a real
unwritable %TEMP% is now covered by a test proving verification fails
closed, reports at the `verify` stage, and still leaves the ACL applied —
so that path loses proof, not protection, and with the lifetime cap gone it
can no longer combine into a permanent-off state.

src/main/observability/index.ts

@@ -49,7 +49,7 @@ import {
   type UploadBundleResult
 } from './diagnostic-bundle-upload'
 import { setActiveSink, startSpan } from './tracer'
-import { setSecurePathHardeningFailureReporter } from '../../shared/secure-path-windows-acl'
+import { setSecurePathHardeningReporter } from '../../shared/secure-path-windows-acl'
 
 const CI_ENV_VARS = [
   'CI',
@@ -162,19 +162,29 @@ export function initObservability(): ObservabilityConsent {
  * Why route it here: Windows path hardening lives in `src/shared` and defaults to `console.warn`,
  * which reaches nothing in a packaged build — the main process is GUI-subsystem and owns no
  * console. A credential file left on inherited ACLs is exactly what a diagnostic bundle should
- * show, so the failure becomes a failed span in the trace sink.
+ * show, so it becomes a span in the trace sink.
+ *
+ * `recovered` ends successfully rather than failing: a host that climbs back out of the
+ * rate-limited state has to be as visible as one that fell into it, or the degraded state is only
+ * ever half-diagnosable.
  */
 function installSecurePathHardeningReporter(): void {
-  setSecurePathHardeningFailureReporter((failure) => {
-    startSpan('secure-path.windows-acl.failure', {
-      attributes: { targetPath: failure.targetPath, stage: failure.stage }
-    }).fail(failure.detail)
-    console.warn('[secure-path.windows-acl] failed to restrict path', failure)
+  setSecurePathHardeningReporter((entry) => {
+    const span = startSpan('secure-path.windows-acl', {
+      attributes: { targetPath: entry.targetPath, stage: entry.stage, detail: entry.detail }
+    })
+    if (entry.stage === 'recovered') {
+      span.end()
+      console.info('[secure-path.windows-acl] path hardening recovered', entry)
+      return
+    }
+    span.fail(entry.detail)
+    console.warn('[secure-path.windows-acl] failed to restrict path', entry)
   })
 }
 
 export async function shutdownObservability(): Promise<void> {
-  setSecurePathHardeningFailureReporter(null)
+  setSecurePathHardeningReporter(null)
   // Order matters: tracer first so no new pushes arrive while the local sink
   // is closing and flushing buffered lines.
   setActiveSink(null)

src/shared/secure-file.test.ts

@@ -200,35 +200,81 @@ describe('hardenSecurePath', () => {
     Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
     const targetPath = writeFailingHardenTarget()
 
-    // The read-path polling loop the storm came from.
+    // The read-path polling loop the storm came from: 25 reads, not 25 spawns.
     for (let read = 0; read < 25; read++) {
       hardenExistingSecureFile(targetPath)
       await flushAsyncAcl()
     }
 
-    expect(attemptsFor(targetPath)).toHaveLength(1)
+    expect(attemptsFor(targetPath)).toHaveLength(3)
+    // Entering the degraded state is announced once, not once per read.
+    expect(throttleReports(warn, targetPath)).toHaveLength(1)
     warn.mockRestore()
   })
 
-  it('retries a failing path once the retry floor has passed, then gives up', async () => {
+  /**
+   * A cap that never expires latches a transient failure: one AV scan or momentary lock and every
+   * later credential write in the session is unprotected, on a host where hardening would now
+   * work. The rate is bounded; the lifetime is not.
+   */
+  it('keeps re-probing a failing path in every later window', async () => {
     const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
     Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
     const targetPath = writeFailingHardenTarget()
     let clock = Date.now()
     const now = vi.spyOn(Date, 'now').mockImplementation(() => clock)
 
-    for (let read = 0; read < 10; read++) {
-      hardenExistingSecureFile(targetPath)
-      await flushAsyncAcl()
+    for (let window = 0; window < 6; window++) {
+      for (let read = 0; read < 6; read++) {
+        hardenExistingSecureFile(targetPath)
+        await flushAsyncAcl()
+      }
       clock += 61_000
     }
 
-    // Three attempts spread over ten minutes, not ten — and then silence.
-    expect(attemptsFor(targetPath)).toHaveLength(3)
+    // Bounded per window, but never abandoned: 36 reads, 3 attempts in each of 6 windows.
+    expect(attemptsFor(targetPath)).toHaveLength(18)
     now.mockRestore()
     warn.mockRestore()
   })
 
+  it('reports recovery when a previously throttled path hardens again', async () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    const info = vi.spyOn(console, 'info').mockImplementation(() => {})
+    Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+    const targetPath = writeFailingHardenTarget()
+    let clock = Date.now()
+    const now = vi.spyOn(Date, 'now').mockImplementation(() => clock)
+
+    for (let read = 0; read < 5; read++) {
+      hardenExistingSecureFile(targetPath)
+      await flushAsyncAcl()
+    }
+    expect(throttleReports(warn, targetPath)).toHaveLength(1)
+
+    // The transient condition clears; the next window's re-probe must notice.
+    clock += 61_000
+    vi.mocked(runProcess).mockImplementation((spec) => Promise.resolve(fakeIcacls(spec)))
+    hardenExistingSecureFile(targetPath)
+    await flushAsyncAcl()
+
+    expect(info).toHaveBeenCalledWith(
+      '[secure-path.windows-acl] path hardening recovered',
+      expect.objectContaining({ targetPath, stage: 'recovered' })
+    )
+    now.mockRestore()
+    info.mockRestore()
+    warn.mockRestore()
+  })
+
+  // Scoped to one path: the parent directory is hardened too, and reports its own transition.
+  function throttleReports(warn: ReturnType<typeof vi.spyOn>, targetPath: string): unknown[] {
+    return warn.mock.calls.filter((call) => {
+      const entry = call[1] as { stage?: string; targetPath?: string } | undefined
+      return entry?.stage === 'throttled' && entry.targetPath === targetPath
+    })
+  }
+
   function writeFailingHardenTarget(): string {
     const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
     tempDirs.push(userDataPath)

src/shared/secure-file.ts

@@ -16,6 +16,11 @@ import {
   SecurePathHardeningCache,
   type SecurePathHardeningCacheBounds
 } from './secure-path-hardening-cache'
+import {
+  configureHardeningRetryBudget,
+  mayAttemptHardening,
+  recordHardeningOutcome
+} from './secure-path-hardening-retry-budget'
 import {
   bestEffortRestrictWindowsPath,
   resetSecureFileWindowsUserSidForTests,
@@ -56,50 +61,15 @@ let hardenedDirectoryPathsThisProcess = new SecurePathHardeningCache<true>(
   DEFAULT_HARDENING_CACHE_BOUNDS
 )
 
-type HardeningFailureRecord = { at: number; attempts: number }
-
-/**
- * Why a retry floor and not a plain eviction: hardening fails permanently on hosts where it simply
- * cannot work — FAT32/exFAT have no ACLs, network paths and redirected profiles refuse, restricted
- * tokens lack WRITE_DAC. The env store re-hardens on the *read* path at ~2/s (#4901), so evicting
- * on every failure turns those hosts into a permanent icacls-and-log storm.
- */
-const HARDENING_RETRY_FLOOR_MS = 60_000
-const MAX_HARDENING_ATTEMPTS = 3
-
-let hardeningFailuresThisProcess = new SecurePathHardeningCache<HardeningFailureRecord>(
-  DEFAULT_HARDENING_CACHE_BOUNDS
-)
-
-function mayAttemptHardening(targetPath: string): boolean {
-  const failure = hardeningFailuresThisProcess.get(targetPath)
-  if (!failure) {
-    return true
-  }
-  if (failure.attempts >= MAX_HARDENING_ATTEMPTS) {
-    return false
-  }
-  return Date.now() - failure.at >= HARDENING_RETRY_FLOOR_MS
-}
-
-function recordHardeningOutcome(targetPath: string, restricted: boolean): void {
-  if (restricted) {
-    hardeningFailuresThisProcess.delete(targetPath)
-    return
-  }
-  const previous = hardeningFailuresThisProcess.get(targetPath)
-  hardeningFailuresThisProcess.set(targetPath, {
-    at: Date.now(),
-    attempts: (previous?.attempts ?? 0) + 1
-  })
-}
+// Bounds the retry rate for paths whose hardening keeps failing; see the module for why.
+configureHardeningRetryBudget(DEFAULT_HARDENING_CACHE_BOUNDS)
 
 function hardenSecureDirectoryOnce(dirPath: string): void {
   // Why: dir hardening stays async — re-applying it stormed the main thread (#4901); files inside are hardened synchronously anyway.
   if (hardenedDirectoryPathsThisProcess.get(dirPath)) {
     return
   }
-  // Cache before the ACL lands so concurrent writes don't restorm; a failure drops it, under the retry floor.
+  // Cache before the ACL lands so concurrent writes don't restorm; a failure drops it, under the retry budget.
   hardenedDirectoryPathsThisProcess.set(dirPath, true)
   applySecurePathRestriction(dirPath, true, process.platform, false, (restricted) => {
     if (!restricted) {
@@ -358,7 +328,7 @@ export function __resetSecureFileHardenedPathsForTests(
 ): void {
   hardenedPathsThisProcess = new SecurePathHardeningCache(bounds)
   hardenedDirectoryPathsThisProcess = new SecurePathHardeningCache(bounds)
-  hardeningFailuresThisProcess = new SecurePathHardeningCache(bounds)
+  configureHardeningRetryBudget(bounds)
 }
 
 export function __getSecureFileHardeningCacheStateForTests(): {

src/shared/secure-path-hardening-retry-budget.ts

@@ -0,0 +1,76 @@
+import {
+  SecurePathHardeningCache,
+  type SecurePathHardeningCacheBounds
+} from './secure-path-hardening-cache'
+import { reportSecurePathHardening } from './secure-path-windows-acl'
+
+type HardeningFailureRecord = { windowStartedAt: number; attempts: number }
+
+/**
+ * How often a path whose hardening keeps failing may be retried.
+ *
+ * Why a rate limit and not a lifetime cap: the env store re-hardens on the *read* path at ~2/s
+ * (#4901), so retrying every failure is a permanent icacls-and-log storm on hosts where hardening
+ * cannot work — FAT32/exFAT have no ACLs, and network paths, redirected profiles and restricted
+ * tokens refuse. But a cap that never expires latches a *transient* failure: one AV scan or
+ * momentary lock, and every later credential write in the session is unprotected, silently, on a
+ * host where hardening would now succeed. So bound the retry *rate*, never the lifetime, and
+ * announce both directions of the transition so a stuck host is diagnosable.
+ */
+const HARDENING_RETRY_WINDOW_MS = 60_000
+const MAX_HARDENING_ATTEMPTS_PER_WINDOW = 3
+
+let hardeningFailures: SecurePathHardeningCache<HardeningFailureRecord> | null = null
+
+function failures(): SecurePathHardeningCache<HardeningFailureRecord> {
+  if (!hardeningFailures) {
+    throw new Error('secure path hardening retry budget used before it was configured')
+  }
+  return hardeningFailures
+}
+
+export function configureHardeningRetryBudget(bounds: SecurePathHardeningCacheBounds): void {
+  hardeningFailures = new SecurePathHardeningCache<HardeningFailureRecord>(bounds)
+}
+
+export function mayAttemptHardening(targetPath: string): boolean {
+  const failure = failures().get(targetPath)
+  if (!failure) {
+    return true
+  }
+  // A stale window always re-probes: recovery must never require a restart to be noticed.
+  if (Date.now() - failure.windowStartedAt >= HARDENING_RETRY_WINDOW_MS) {
+    return true
+  }
+  return failure.attempts < MAX_HARDENING_ATTEMPTS_PER_WINDOW
+}
+
+export function recordHardeningOutcome(targetPath: string, restricted: boolean): void {
+  const previous = failures().get(targetPath)
+  if (restricted) {
+    failures().delete(targetPath)
+    if (previous && previous.attempts >= MAX_HARDENING_ATTEMPTS_PER_WINDOW) {
+      reportSecurePathHardening(
+        targetPath,
+        'recovered',
+        'hardening succeeded again after being rate-limited'
+      )
+    }
+    return
+  }
+  const now = Date.now()
+  const staleWindow = !previous || now - previous.windowStartedAt >= HARDENING_RETRY_WINDOW_MS
+  const attempts = staleWindow ? 1 : previous.attempts + 1
+  failures().set(targetPath, {
+    windowStartedAt: staleWindow ? now : previous.windowStartedAt,
+    attempts
+  })
+  // Fires exactly once per window: further attempts inside it are refused before they run.
+  if (attempts === MAX_HARDENING_ATTEMPTS_PER_WINDOW) {
+    reportSecurePathHardening(
+      targetPath,
+      'throttled',
+      `hardening failed ${attempts} times; retrying at most ${MAX_HARDENING_ATTEMPTS_PER_WINDOW} times per ${HARDENING_RETRY_WINDOW_MS / 1000}s until it succeeds`
+    )
+  }
+}

src/shared/secure-path-windows-acl.ts

@@ -14,9 +14,10 @@ const BUILTIN_ADMINISTRATORS_SID = 'S-1-5-32-544'
 
 const WINDOWS_SID_PATTERN = /^S-1-\d+(?:-\d+)+$/
 
-export type SecurePathHardeningFailure = {
+export type SecurePathHardeningReport = {
   targetPath: string
-  stage: 'sid-lookup' | 'reset' | 'grant' | 'verify'
+  /** `throttled` and `recovered` mark entering and leaving the rate-limited degraded state. */
+  stage: 'sid-lookup' | 'reset' | 'grant' | 'verify' | 'throttled' | 'recovered'
   detail: string
 }
 
@@ -149,20 +150,37 @@ function toIcaclsPath(targetPath: string): string {
  * installs a reporter that routes into the diagnostic trace; the console default keeps dev runs
  * and the CLI readable.
  */
-let reportFailure: (failure: SecurePathHardeningFailure) => void = (failure) => {
-  console.warn('[secure-path.windows-acl] failed to restrict path', failure)
+const consoleReporter = (entry: SecurePathHardeningReport): void => {
+  if (entry.stage === 'recovered') {
+    console.info('[secure-path.windows-acl] path hardening recovered', entry)
+    return
+  }
+  console.warn('[secure-path.windows-acl] failed to restrict path', entry)
 }
 
-export function setSecurePathHardeningFailureReporter(
-  reporter: ((failure: SecurePathHardeningFailure) => void) | null
+let reportEntry: (entry: SecurePathHardeningReport) => void = consoleReporter
+
+export function setSecurePathHardeningReporter(
+  reporter: ((entry: SecurePathHardeningReport) => void) | null
 ): void {
-  reportFailure = reporter ?? ((failure) => {
-    console.warn('[secure-path.windows-acl] failed to restrict path', failure)
-  })
+  reportEntry = reporter ?? consoleReporter
 }
 
-function report(targetPath: string, stage: SecurePathHardeningFailure['stage'], detail: string): void {
-  reportFailure({ targetPath, stage, detail: detail.trim().slice(0, 500) })
+/** Exported so the caller owning the retry budget reports degradation and recovery on this lane. */
+export function reportSecurePathHardening(
+  targetPath: string,
+  stage: SecurePathHardeningReport['stage'],
+  detail: string
+): void {
+  reportEntry({ targetPath, stage, detail: detail.trim().slice(0, 500) })
+}
+
+function report(
+  targetPath: string,
+  stage: SecurePathHardeningReport['stage'],
+  detail: string
+): void {
+  reportSecurePathHardening(targetPath, stage, detail)
 }
 
 /**

src/shared/secure-path-windows-acl.win32.test.ts

@@ -191,6 +191,45 @@ describeOnWindows('restrictWindowsPathSync against a real filesystem', () => {
     expect(readAclEntries(file)).toEqual(first)
   })
 
+  /**
+   * Verification writes a temp SDDL file. If it cannot, the ACL may well have been applied — but
+   * it cannot be *proved*, so hardening must report failure rather than assume success. Fail
+   * closed, and say so: a silently-unverifiable control is the shape of the original bug.
+   */
+  it('reports failure, loudly, when verification cannot write its descriptor', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    const file = join(root, 'unverifiable.json')
+    writeFileSync(file, '{}')
+    const realTemp = process.env.TEMP
+    const realTmp = process.env.TMP
+    // Point the descriptor save at a directory that cannot exist.
+    process.env.TEMP = join(root, 'no-such-dir', 'nested')
+    process.env.TMP = process.env.TEMP
+
+    try {
+      expect(restrictWindowsPathSync(file, false)).toBe(false)
+      expect(warn).toHaveBeenCalledWith(
+        '[secure-path.windows-acl] failed to restrict path',
+        expect.objectContaining({ stage: 'verify' })
+      )
+    } finally {
+      if (realTemp === undefined) {
+        delete process.env.TEMP
+      } else {
+        process.env.TEMP = realTemp
+      }
+      if (realTmp === undefined) {
+        delete process.env.TMP
+      } else {
+        process.env.TMP = realTmp
+      }
+      warn.mockRestore()
+    }
+
+    // And the ACL itself was still applied, so the failure is a loss of proof, not of protection.
+    expect(readAclEntries(file)).toHaveLength(3)
+  })
+
   it('reports failure for a path that does not exist', () => {
     const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
     expect(restrictWindowsPathSync(join(root, 'absent.json'), false)).toBe(false)