[04/06] feat(core): add functions exec cell lifecycle - #6
Draft
HeavenllyDemon wants to merge 1 commit into
Draft
Conversation
HeavenllyDemon
force-pushed
the
feat/functions-exec-03-secure-nested-tools
branch
from
August 18, 2026 13:15
e0b8dd3 to
ac51a52
Compare
HeavenllyDemon
force-pushed
the
feat/functions-exec-04-engine-lifecycle
branch
from
August 18, 2026 13:15
c079fa2 to
25fdee8
Compare
HeavenllyDemon
changed the title
HeavenllyDemon
added a commit
that referenced
this pull request
Aug 22, 2026
…urity record
Third security review round on Task 10's sandbox minimization — a convergence
re-review confirmed fix round 2's ship-gate CONCLUSION (Test B never returns
-10814) but reproduced it on the production shape (bundled+embedded, which
neither of this task's own tests cover) and ran the control round 2 never did:
unsandboxed the same LSOpenCFURLRef call returns -10814; sandboxed it returns
-10661 (9 cells: 5 sandboxed / 4 unsandboxed). A log-stream capture shows the
open dying at _LSAgentGetConnection on denied com.apple.lsd.modifydb/.mapdb/a
quarantine-resolution service, before handler resolution; adding those names to
a scratch profile flips the sandboxed reading to -10814; removing the
launchservicesd grant changes nothing. So the sandbox — specifically the
absence of those lsd.*/quarantine-resolution grants, not the presence or
absence of launchservicesd — is what protects this open path. Doc/comment-only
by design; one code delta (F5) removes an inert allowlist entry.
Three mandatory corrections (falsified claims in the durable security record):
F1 (main.swift, OfficeSandboxTests.swift, both copies) — the prior claim that
-10661 is "a LOCAL, client-side rejection... before ever needing to ask lsd"
was falsified, not merely unhedged. Replaced with the measured mechanism,
cited as the re-review's own cells (not reproduced here): the sandbox gates
this open path via the absence of lsd.mapdb/.modifydb/quarantine-resolution,
independent of the launchservicesd grant. Recorded as the pair it has to be
stated as: launchservicesd remains necessary for the original hang fix
(document-open bootstrap) and is irrelevant to this specific open path —
stating only one half risks a future tightening pass dropping it and silently
reintroducing the hang. Two upgrades: Test A is now a load-bearing containment
pin for this open path (covers 2 of the re-review's 3 named denials; the third,
quarantine-resolution, is an honest, disclosed gap — no test in this repo pins
it); the registered-scheme follow-up is now largely closed by mechanism, marked
as inference, not a run cell. Disclosed the bare-vs-bundled coverage gap in
this task's own tests as a coverage note, not a defect.
F2 (LOKBridge.swift) — the N2 comment's "a repo-wide grep turns up zero
fixtures or tests... so today this allowlist is inert" was false, not merely
unverified. Three committed fixtures exist (legacy-doc.doc, legacy-ppt.ppt,
legacy-xls.xls, each verified real CFB magic bytes) and
testKnownLimitationLegacyBinaryImportDoesNotOpenInThisVendorBuild opens all
three through this exact helper, bypassing the app's own routing. The
allowlist is load-bearing today: dropping xls flips the test to the gate's own
refusal reason (wrong error, red); dropping doc/ppt means the helper survives
instead of dying (red). The "not reachable via app routing" half was correct
and is unchanged.
I1 residual (office-helper.sb) — a paragraph three below the fix-round-2 hedge
restated the same untraced mechanism as fact ("MacSpellChecker constructs
SUCCESSFULLY in the bare-exec cells... NSApplicationLoad()'s own
bundle-identity-dependent behavior was [the right lever]"), contradicting its
own hedge. Corrected: the bare-exec cells complete, consistent with
construction succeeding but never traced directly; Experiment A's negative
result licenses only "neither registry seed changed the outcome," not the
bundle-identity mechanism claim.
Four optional one-liners, taken:
F3 (release.ts) — M4's tripwire was satisfiable by comment text: the .sb file's
own prose (a few paragraphs up) literally contains the substring
"(deny network*)". Replaced the two-clause substring check with a full
byte-identity comparison against the repo source (Buffer.equals), which
subsumes it entirely.
F4 (office-helper.sb) — the canonical comment never reflected round 2's own
Test A/Test B results and still framed the open-request question as fully
open. Added a forward-pointing addendum naming both, and the re-review's
finding, without rewriting the Experiment C section's own historical prose.
F5 (LOKBridge.swift) — removed xlsb from cfbNativeLegacyExtensions: it is a
post-2007 OPC/ZIP+BIFF12 package, never OLE2/CFB, so it never belonged in a
"genuine CFB expected here" allowlist. Leaving it in would have silently
disabled the CFB guard for .xlsb the day T9's concern #6 ships real support.
No fixture or test exercises xlsb today, so this is unexercised — the one real
code delta this round, latent-only.
F6 (LOKBridge.swift) — fixed a self-contradictory comment ("fails OPEN, not
closed" immediately followed by describing a wrongly-REFUSED outcome, which is
failing closed, not open).
Re-verified: OfficeSandboxTests 9/9, OfficeHelperLiveTests 35/35 (incl. the
legacy-binary and both CFB tests), OfficeRuntimeLiveTests 28/28 — 72/72, 0
failures. release.ts change bun-build-checked clean. Zero observable behavior
change; F5 is the only code delta and is unexercised by any fixture or test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge order
4 of 5. Depends on #5.
Scope
Verification
bun test test/functions-exec— 36 passbunx tsc -p tsconfig.json --noEmit— only the six pre-existing TS18048 errors intest/agent/approvals.test.ts