ffi: take sizes and timestamps as strings; both SDKs stop parsing - #187
Open
dzerik wants to merge 3 commits into
Open
ffi: take sizes and timestamps as strings; both SDKs stop parsing#187dzerik wants to merge 3 commits into
dzerik wants to merge 3 commits into
Conversation
Closes the second half of multikernel#174. The SDK carried its own TOML parser and its own grammars, and they had already diverged from the core in two places you found: `parse_memory_size` accepted fractions and a `T` suffix that `ByteSize::parse` rejects, and `time_start` went through `int()`, so an RFC 3339 stamp worked in the CLI and raised through the SDK. A third grammar sat unused in the dataclass, `time_start_timestamp`, with naive-means-UTC semantics. `sandlock_profile_parse` takes TOML text and returns canonical JSON with every micro-grammar already resolved: mounts as `{virt, host, ro}` objects, sizes as integer bytes, `time_start` as epoch seconds. The SDK's remaining job is a field-for-field copy into its dataclass, so introspection, `dataclasses.replace` and preset composition keep working. Unknown keys are rejected on both sides, so future drift fails at load time instead of mis-parsing silently. `sandbox_to_json` was not reusable as-is: it re-emits mounts as `V:H:ro` spec strings, which would have put string parsing straight back into the SDK. The canonical form emits structured mounts instead. Its `ro` is the effective setting for the virtual path, not the flag written on one spec: the core keys read-only mounts by virtual path (`Sandbox::fs_mount_ro` is a list of virtual paths), so two specs sharing a virtual path share one verdict, and reporting the written flag would describe a policy no layer applies. Public Python API changes, deliberately and without shims: max_memory: str | int | None -> int | None max_disk: str | None -> int | None time_start: float | str | None -> float | None fs_mount: Mapping[str, str] -> Sequence[Mount] `Mount(virt, host, ro)` is new and mirrors the canonical field names. `fs_mount` becoming a sequence is what lets a read-only mount be expressed at all from Python; it reaches the C ABI through the `fs_mount_ro` setter added in multikernel#180. `tomli` is gone from the dependencies. Two more changes to the same surface, both consequences of the SDK no longer holding an opinion of its own: - `on_error` loaded from a profile now defaults to COMMIT where it defaulted to ABORT. The canonical form always resolves both branch actions, and the SDK copies what it is handed, so a profile that says nothing about the error path gets the core's answer rather than the dataclass's second opinion. Deliberate, since the CLI, a profile and the Go SDK have always meant COMMIT for that policy, but it changes what happens to a COW branch for a profile already in use, and it changes it silently. Only the profile path moves; the dataclass default is untouched here. - `parse_memory_size`, `Sandbox.memory_bytes()` and `Sandbox.time_start_timestamp()` are removed. The first two were the SDK's byte-size grammar and its accessor, the third the unused third grammar named above. Nothing replaces them: the resolved value is the field. Two core changes came out of this rather than the SDK: - Rebuilding a builder from a parsed profile ran `extend_net_allow_for_http` a second time over an allowlist that already held its derived entries, so the helper is now idempotent, with a test. - `ByteSize::parse` multiplies with `checked_mul`. The unchecked multiply wrapped in release builds, so `memory = "17179869184G"` parsed cleanly and installed a ceiling of zero bytes, with nothing reported anywhere and the guest SIGKILLed on its first allocation. It is an out-of-range error now. Verified against the CLI message for message on every grammar: the same profile loads identically, or fails identically, through both paths, with one gap left open and pinned rather than papered over. `sandlock_sandbox_builder_time_start` takes a `uint64` of seconds, so a stamp the core keeps in full loads from a profile and then cannot be handed to a builder: `"2026-01-01T00:00:00.5Z"` and any instant before 1970 are what that costs. The SDK refuses them by name instead of wrapping a negative value through an unsigned setter, and `test_time_start_the_c_abi_cannot_carry_is_refused_loudly` holds it there. Closing the gap means changing that setter's signature, which is a later commit in this series.
A builder setter returns Self, not Result, so it has no channel for a value
the core cannot accept. The C ABI answered that by coercing. An on_exit
discriminant with no variant became Commit through the fall-through arm of a
match. An unrecognized protection discriminant was a documented no-op. Every
string setter ran its argument through `to_str().unwrap_or("")`, except the two
mount setters, which dropped the whole call instead. Each of those runs a
configuration the caller never wrote, and says nothing while doing it.
SandboxBuilder now carries a pending-error latch. `reject` records a reason a
surface diagnosed itself, `reject_error` records one the core's own parser
produced, and `build()` returns it instead of a Sandbox. The setter contract
is otherwise untouched, which is why the bindings do not move: this commit
changes nothing under go/ or python/src. The three python/tests files it does
touch move because of the zero checks below, not because of binding work.
Three decisions worth naming.
The latch holds a String, not a SandboxError. SandboxBuilder is Clone and
SandboxError is not; making it Clone would widen a public error type for the
benefit of one private field. `reject_error` keeps the parser's own text
rather than the wrapped Display, because build() puts the reason back into
SandboxError::Invalid, so a value refused through the C ABI reads exactly as
it reads on the command line instead of as a doubled "invalid sandbox:
invalid sandbox: ...".
The check sits in `build_unchecked`, not in `build`. `build_unchecked` is
public and is what sandlock-oci calls (crates/sandlock-oci/src/policy.rs:463).
A check in `build` alone would let the one caller that deliberately skips
cross-section validation also skip the caller's own rejected input, which is
not the invariant it asked to skip.
Clone carries the latch. Dropping it there would make `.clone().build()` a
laundering channel for a value the core has already refused.
First write wins. The earliest bad input is the one that explains whatever
follows it, so later rejections are dropped and the message names the caller's
first mistake rather than its last.
What now reports instead of coercing:
- on_exit and on_error, on an unrecognized discriminant. BranchAction gains
#[repr(u8)] with explicit discriminants and a `from_repr`, so the values
the bindings pass as a u8 are a written-down contract rather than the
fall-through arm of a match. Serde is unaffected: a data-less enum
serializes by variant name, not by discriminant.
- allow_degraded and disable, on an unrecognized protection. The no-op was
documented, which meant a binding built against a newer header was told
nothing when an older library did not recognize the protection it asked to
be degradable: the caller believed it had opted out, and the protection
stayed strict.
- 22 string setters, through one `setter_arg` helper: a null pointer, and
bytes that are not UTF-8. Those two stay the C ABI's own verdicts because
they are representation problems the core cannot see once the value is a
&str; the grammar's verdict still comes from the core untouched.
`unwrap_or("")` is reachable without any bug in the caller, since a path
read off readdir() is an arbitrary byte string on Linux, and the empty
path it produces is a prefix of every guest path. The coercion survives
only in the entry points that take no builder and so have nothing to latch
a reason on.
The three-argument setters report per half (`env_var key`, `fs_mount_ro
host path`), so the message names the pointer to fix. fs_mount and
fs_mount_ro are the two that had a coercion of their own shape: a private
`mount_pair` helper answered "add no mount" for a null, non-UTF-8 or empty
path, so the caller who asked for a read-only subtree got a writable one
and the caller who asked for a host directory got nothing there. They go
through the latch now, which is what makes the sentence above true of
every `*const c_char` builder setter rather than of most of them.
Zero and the empty set, in the same commit and for the same reason. The latch
stops a surface from inventing a value the caller did not write; these stop a
surface from having to invent a verdict the core would not give. Both have to
be in place before a binding can be reduced to forwarding, and neither is
visible in a binding's own diff. The max_open_files check that was already
here said as much in its comment, which claimed a binding "must" filter zero
itself; that comment is corrected here too, and corrected to what is true
today rather than to what the series is heading for. Python already forwards
whatever is not None, zero included. Go still filters (`if s.MaxOpenFiles > 0`
in go/sandlock_linux.go), so a Go caller who writes zero still gets no cap and
no diagnosis; reducing Go to forwarding needs its fields to spell "unset"
without using the value, which is a later commit.
- max_processes = 0: the supervisor compares proc_count >= limit, so a limit
of zero denies every fork with EAGAIN no matter how few processes are
alive, and the workload reads "Resource temporarily unavailable" from its
first subprocess with nothing naming the setting.
- num_cpus = 0: reaches the synthetic procfs as an empty /proc/cpuinfo and
an affinity mask with no bits, so the guest reads nproc = 0.
- max_memory = 0: zero is the sentinel the supervisor already carries for
"no ceiling" (max_memory.map(..).unwrap_or(0) in Sandbox::run, read back
as > 0 by the synthetic /proc/meminfo), but the memory handler is
registered on is_some(). An explicit zero therefore installs a ceiling of
zero and SIGKILLs the loader's first anonymous mmap while /proc/meminfo
reports the sandbox unlimited. The two readings cannot both stand, and
refusing the value is what lets the sentinel keep meaning "unset".
max_disk is deliberately not the same: zero is its documented spelling of
"unlimited", and one reading is all it has.
- cpu_cores = []: an affinity mask with no bits, which sched_setaffinity(2)
refuses with EINVAL. confine_child skipped the call for an empty set
instead, so the pinning the caller asked for silently did not happen and
the sandbox ran on every core; that branch is deleted now that the value
cannot reach it. Unlike gpu_devices, where an empty list is the spelling
of "every device present", there is no cpu set an empty list could stand
for, because "every core" is what omitting the field already means.
- an empty virtual or host path in fs_mount and fs_mount_ro. This is the
check that came back from the C ABI: `mount_pair` was making a policy
judgement the core's own profile grammar already makes when it splits a
VIRTUAL:HOST spec, and making it in the one place that could not report
it. Neither half has a reading as "unset", and an empty virtual path is a
prefix of every guest path, so ChrootCtx::is_mounted would match the whole
tree and short-circuit can_read and can_write.
Confinement::try_from listed on_exit and on_error among the fields a
confinement cannot honour. A confinement has no branch to act on: it is
applied in place, and fs_storage and workdir, the two knobs that create one,
are already refused above it. The check only ever refused a field that could
not have changed the outcome, and it did so by comparing against two hardcoded
actions rather than against what build() resolves an unset field to, so a
caller who said nothing about the error path was refused a confinement its
policy allowed.
The C ABI is unchanged: no signature, no struct and no discriminant value
moves. include/sandlock.h changes by 171 lines (141 added, 30 removed) and every
one of them is inside a comment block; with comment lines stripped the header
is byte-identical to its parent. The added ones are the four new refusals
written down where a binding author reads them: max_memory = 0,
max_processes = 0, num_cpus = 0 and an empty cpu_cores are now in the doc
comment of the setter that carries each, along with max_open_files = 0, which
was already refused and had never been documented anywhere. The same rows in
docs/sandbox-reference.md say the same thing.
Tests: crates/sandlock-ffi/tests/builder_pending_error.rs covers the latch
itself (both branch-action setters, survival across later valid calls,
first-write-wins, Clone, build_unchecked, and null, non-UTF-8 and per-half
arguments across every string setter). tests/fs_mount.rs had four tests
pinning the drop-silently behaviour of the mount setters; they become one that
pins the report, over both setters and all six unusable inputs. In
protection.rs the two tests that asserted the no-op now assert the report, and
the third, which checked that a later valid call still took effect, becomes
the first-write-wins case while keeping the memory-safety property it was
really watching. sandbox/tests.rs covers the confinement change from both
sides, and builder.rs covers `reject_error` directly: it has no caller yet,
since the four that use it arrive with the string setters in a later commit,
so the test drives it with a real ByteSize::parse error and asserts the built
message is the parser's own text rather than a doubled wrapping.
Closes multikernel#175.
Three builder setters change shape:
sandlock_sandbox_builder_max_memory(b, uint64_t bytes)
-> sandlock_sandbox_builder_max_memory(b, const char *size)
sandlock_sandbox_builder_max_disk(b, uint64_t bytes)
-> sandlock_sandbox_builder_max_disk(b, const char *size)
sandlock_sandbox_builder_time_start(b, uint64_t epoch_secs)
-> sandlock_sandbox_builder_time_start(b, const char *timestamp)
plus one new symbol, sandlock_sandbox_builder_time_start_epoch(b, int64_t
seconds, uint32_t nanoseconds). The core parses the strings with the same
routines that read [limits].memory, [limits].disk and
[determinism].time_start in a profile and --max-memory, --max-disk and
--time-start on the command line. A value it refuses is latched by the
mechanism from the previous commit and comes back from
sandlock_sandbox_build as the core's own message.
Why the signatures had to move. "The bindings stop parsing" is not
implementable over a uint64_t setter: a binding whose users write "512M" has
to own a byte-size parser to have anything to pass, and a binding whose users
write a timestamp has to own a timestamp parser. So both of them did. The Go
one said what that costs, in its own words:
// ParseMemory parses a human-friendly size string into bytes. It accepts
// a plain integer (bytes) or a value suffixed with K, M, G, or T (case
// insensitive), e.g. "512M", "1G", "100K". Mirrors the Python SDK's
// parse_memory_size so the two SDKs agree byte-for-byte.
Two bindings held in agreement with each other, and neither of them with the
core they were feeding. That parser accepted "1.5G", "0.5K", "1T" and "512T";
ByteSize::parse takes a decimal integer with a K, M or G suffix and nothing
else, so no flag and no profile has ever accepted any of them. The entries
marked sdk_only in tests/grammar-corpus.json are exactly that gap.
ParseTimeStart had the mirror-image defect on the other grammar: it took a
bare epoch count, which a profile refuses, and refused a pre-1970 stamp, which
a profile takes.
time_start_epoch is not a second grammar and is not scope creep. It is the
numeric door to the same instant, and what needs it is the SDK's own field:
Sandbox.time_start takes str, int or float, because an epoch count is how a
caller who starts from datetime.timestamp() or from an arithmetic offset
already holds the value. A float is not text, and there is no reading of it
that makes it text. With only the string setter, the SDK would have to render
"1767225600.5" back into "2026-01-01T00:00:00.500000000Z" before it could
forward it: an RFC 3339 *writer* in the binding, one line under the RFC 3339
reader this commit just deleted from it, and with its own way to be wrong
about offsets, leap-second spelling and sub-second digits. The epoch setter
takes the pair the value already is (_epoch_split only moves the fractional
part into its own field, flooring the way sandlock_profile_parse does), and
_b_time_start_from picks the door by type: text goes to the string setter
untouched, a number to this one (_sdk.py:877). Either way the grammar is read
exactly once, by the core.
A consumer of sandlock_profile_parse also lands here, but that is a
consequence, not the reason: CanonicalTimestamp carries rfc3339 now, so such a
consumer does still hold text it could forward. It reaches for the epoch door
because {seconds, nanoseconds} is what it is holding, not because it has
nothing else.
Core changes that carry it:
- profile::parse_time_start, parse_timestamp and the new
time_start_from_epoch take a label naming the knob that carried the value,
the way NetRule::parse_allow names --net-allow, so one grammar can be
reached from four surfaces and still name the right one in a diagnosis.
The CLI's private jiff parser is deleted and sandlock-cli drops its jiff
dependency.
- time_start_from_epoch refuses a remainder at or above a full second rather
than carrying it into the seconds: the canonical form normalizes, so a
caller that has not is working from a different contract.
- CanonicalTimestamp gains an rfc3339 field, for the same reason
CanonicalNetRule carries spec. A consumer whose scalar type cannot hold
both halves was otherwise pushed back into a lossy one: a double has about
238ns of spacing at 2026 epoch values, so "...T00:00:00.9999999Z" rounded
up to the next whole second and the same profile ran one second later
through the Python SDK than through the CLI.
- time.rs: calculate_time_offset resolved both instants with
duration_since(UNIX_EPOCH).unwrap_or_default(), which collapses every
pre-epoch instant to the epoch. The grammar accepts one
("1969-07-20T20:17:00Z" parses), so the sandbox ran a clock the caller
never asked for, 14182980 seconds off, with no error anywhere. The new
epoch_seconds floors in both directions, matching CanonicalTimestamp.
Go. go/internal/policy is deleted, both files, 131 lines. MaxMemory, MaxDisk
and TimeStart are forwarded verbatim. Every numeric cap is forwarded whenever
it is set, zero included, because the core now has a verdict on zero for each
of them and filtering it here would replace that verdict with a silently
ignored field; the fields that cannot express "unset" in the value itself are
therefore pointers, with a Ptr helper since Go has no address-of for a
literal. CPUCores and GPUDevices distinguish nil from empty, so an empty GPU
list can still mean "every GPU present". The bool setters are called
unconditionally: sending them only when true leaves the false side
inexpressible and pins the binding to today's core defaults. What remains
binding-local is the one verdict Go must reach on its own, an interior NUL in
a Go string, which cannot survive the conversion to a C string, plus an empty
command, which the C ABI's create family has no error channel to report.
Python. The size and timestamp fields accept the core's spellings and forward
them; the __post_init__ guards that used to refuse "512M" and
"2026-01-01T00:00:00Z" as "profile syntax" are gone, along with _bytes_limit
and _epoch_seconds. uid and gid become one User value, mirroring the core's
RunAs: an unprivileged user namespace maps a single pair, so a uid without a
gid is not a policy the core can apply, and now not a state the SDK can spell.
on_exit and on_error default to None and let the core decide. The /lib64 entry
is no longer dropped from fs_readable behind the caller's back; the MCP
default policy, which is the caller that needed it, filters its own system
paths and says so.
Tests: tests/grammar-corpus.json is one corpus above all four surfaces, read
by python/tests/test_setter_grammar_parity.py (flag, profile, C ABI setter via
ctypes, Python SDK) and go/grammar_parity_linux_test.go. A corpus pasted into
each language would be the failure being fixed, one table per surface, free to
drift. go/core_verdict_linux_test.go covers the verdicts Go stopped reaching
itself.
BREAKING CHANGES
Go SDK:
1. Name string -> *string. nil auto-generates "sandbox-{pid}"; the empty
string is now a value the core refuses, not a spelling of "auto".
2. OnExit, OnError BranchAction -> *BranchAction. nil means unset.
3. BranchActionDefault is removed, which renumbers BranchActionCommit from 1
to 0, BranchActionAbort from 2 to 1 and BranchActionKeep from 3 to 2. The
constants are the ABI discriminants now and travel unshifted.
4. MaxProcesses, MaxCPU, MaxOpenFiles, NumCPUs -> *uint32, *uint8, *uint32,
*uint32. Use sandlock.Ptr.
5. UID *int and GID *int -> User *RunAs, with uint32 ids.
6. HTTPPorts []int -> []uint16. The C ABI carries a u16, so a wider Go type
wrapped silently on the way down (70000 arriving as 4464).
7. TimeStart no longer accepts a bare unix-seconds string. RFC 3339 only,
which in exchange now carries sub-second precision, an explicit offset
and instants before 1970.
8. MaxMemory and MaxDisk narrow to the core's grammar: "1.5G", "0.5K", "1T"
and "512T" are refused.
9. CPUCores: a non-nil empty slice used to be skipped silently and is now
refused. nil is unset.
10. go/internal/policy is gone, with ParseMemory and ParseTimeStart. Internal
to the module, so not importable from outside it, listed for
completeness.
Python SDK:
11. uid and gid -> user: User | None. User(uid, gid).
12. Sandbox.cpu_pct() is removed.
13. on_exit and on_error default to None instead of BranchAction.COMMIT and
BranchAction.ABORT.
14. max_processes defaults to None instead of 64. The effective cap is
unchanged, since the core's own default is 64; it is simply written down
once now.
15. SyscallEvent.category widens from str to str | int. A category this SDK
does not know is reported as the number rather than mapped to "file".
16. A missing /lib64 is no longer dropped from fs_readable behind the
caller's back. The old behaviour answered a portability question for the
caller, silently, for one hardcoded path; the caller is the one who knows
whether a missing /lib64 is a portability detail or a typo. Read the note
below this list before updating, because what replaces the drop is worse
than it sounds.
17. max_memory, max_disk and time_start accept the core's string spellings.
Widening, not narrowing, but the TypeError that used to reject them is
gone.
C ABI:
18. sandlock_sandbox_builder_max_memory(b, uint64_t bytes) -> (b, const char
*size).
19. sandlock_sandbox_builder_max_disk(b, uint64_t bytes) -> (b, const char
*size).
20. sandlock_sandbox_builder_time_start(b, uint64_t epoch_secs) -> (b, const
char *timestamp). The unit changes with the type: this took seconds and
now takes a stamp, so a caller that has a number wants item 21 rather
than sprintf.
21. sandlock_sandbox_builder_time_start_epoch(b, int64_t seconds, uint32_t
nanoseconds) is new. Additive, listed because it is what a caller of
items 18 to 20 is being pointed at.
A C caller passing an integer where a pointer is now expected is a
constraint violation and gets a diagnostic, which is how items 18 to 20
are meant to be found. The one spelling that slips through is a literal
0, a valid null pointer constant: it compiles, and the C ABI then reports
it from sandlock_sandbox_build as a NULL argument rather than acting on
it. No struct, no discriminant and no other signature moves.
Canonical profile document (sandlock_profile_parse, and the Rust
CanonicalProfile behind it):
22. CanonicalTimestamp gains a required rfc3339 field. It has no serde
default and the struct denies unknown fields, so a document written by an
older build no longer deserializes, and Rust code that constructs the
struct itself no longer compiles. A consumer that validates the key set of
what it is handed, as python/src/sandlock/_profile.py does, has to learn
the key.
23. CanonicalTimestamp loses Copy, because it now holds a String. It moves
where it used to copy. Clone, PartialEq and Eq are unaffected.
Silent at compile time. Most of the list above stops a build or a test run.
Three do not, and they are the ones to read before updating:
- Item 3. A caller who derives a BranchAction from a number rather than
naming a constant, sandlock.BranchAction(n) over a stored or unmarshalled
value, still compiles and now selects a different action, with nothing
anywhere reporting it. Writing the literal straight into the field is
caught, but only because item 2 made that field a pointer at the same
time, which is an accident of packaging rather than a guard.
- Item 13. A Python policy that says nothing about the error path used to
abort the branch's writes and now commits them, which is what the CLI, a
profile and the Go SDK have always done for the same policy. The old
default was a second opinion about a field the core already has one about;
that is why it went, but a caller relying on it gets no warning.
- Item 15. An `event.category == "file"` comparison on an unknown category
used to be true and is now false.
Items 7, 8 and 9 also compile cleanly, but each fails loudly at sandbox-build
time with a message from the core naming the knob and the value.
Item 16 is not in that group, and calling it one would be a nicer story than
the truth. build() does not check that a readable path exists; nothing in the
core does, outside fs_read_if_exists, which nothing on this path calls. The
rule is installed in the child, where Landlock has to open the path, so a
policy naming a missing /lib64 builds, runs, and comes back as
Result(success=False, error="sandlock_create failed") with the path nowhere
in it. Under chroot it is skipped without a word instead (landlock.rs skips a
rule whose in-jail path is absent). So this trades a silent drop of one
hardcoded path for a loud-but-anonymous failure on any of them, which is the
right trade for who owns the decision and a plain regression in diagnosis.
Filter system paths on the way in where the set varies by host: the MCP
default policy, the caller that needed the old behaviour, does exactly that
now and says why, and python/examples, go/examples and the test fixtures that
name /lib64 do the same. The fixtures are part of the change rather than a
detail of it: they passed on arm64 only because the SDK was dropping the path
for them, so leaving them alone would have kept the old behaviour alive in the
one place that is supposed to observe the new one. Making the core name the
path it could not grant is worth doing and is not in this commit.
Contributor
|
Not a review, just a note: I am planning to cut the release in a few days, since this PR is fairly large, I'd suggest to defer it to the next release. WDYT? |
Contributor
Author
|
Agreed, defer it. Answered at length in #185, since the three are one stack and the same answer covers them. Short version: this PR shows +9874/-1574 because it carries #185 and #186 underneath it; what is new here is +4327/-797 across 46 files, about 70 percent of it tests. This is also the one that carries the breaking changes, so it is the last thing that should go near a release. Review order whenever you get to the batch: #185, then #186, then this. |
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
2 participants
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.
Third of three. Stacked on #185 and #186, so this branch carries both of
those commits too: review the third one,
ffi: take sizes and timestamps as strings, and merge the other two first.Completes the arc the first two PRs start: after this, neither SDK owns a
grammar. The core owns all of them, and a binding forwards.
What changes
plus one new symbol,
sandlock_sandbox_builder_time_start_epoch(b, int64_t seconds, uint32_t nanoseconds).The core parses the strings with the same routines that read
[limits].memory,[limits].diskand[determinism].time_startin aprofile, and
--max-memory,--max-diskand--time-starton the commandline. A value it refuses is latched by the mechanism from the previous PR and
comes back from
sandlock_sandbox_buildas the core's own message.Why the signatures had to move
"The bindings stop parsing" is not implementable over a
uint64_tsetter: abinding whose users write
"512M"has to own a byte-size parser to haveanything to pass. So both of them did. The Go one said what that costs, in its
own words:
Two bindings held in agreement with each other, and neither of them with the
core they were feeding. That parser accepted
"1.5G","0.5K","1T"and"512T";ByteSize::parsetakes a decimal integer with a K, M or G suffixand nothing else, so no flag and no profile has ever accepted any of them. The
entries marked
sdk_onlyintests/grammar-corpus.jsonare exactly that gap.ParseTimeStarthad the mirror-image defect on the other grammar: it took abare epoch count, which a profile refuses, and refused a pre-1970 stamp, which
a profile takes.
Why
time_start_epochis not a second grammarIt is the numeric door to the same instant, and what needs it is the SDK's own
field:
Sandbox.time_starttakesstr,intorfloat, because an epochcount is how a caller who starts from
datetime.timestamp()already holds thevalue. A float is not text. With only the string setter, the SDK would have to
render
"1767225600.5"back into"2026-01-01T00:00:00.500000000Z"before itcould forward it: an RFC 3339 writer in the binding, one line under the RFC
3339 reader this commit just deleted from it, with its own ways to be wrong
about offsets, leap-second spelling and sub-second digits. The epoch setter
takes the pair the value already is, and
_b_time_start_frompicks the door bytype. Either way the grammar is read exactly once, by the core.
Core changes that carry it
profile::parse_time_start,parse_timestampand the newtime_start_from_epochtake a label naming the knob that carried the value,the way
NetRule::parse_allownames--net-allow, so one grammar can bereached from four surfaces and still name the right one in a diagnosis. The
CLI's private jiff parser is deleted and
sandlock-clidrops itsjiffdependency.
CanonicalTimestampgains anrfc3339field, for the same reasonCanonicalNetRulecarriesspec. A consumer whose scalar type cannot holdboth halves was otherwise pushed into a lossy one: a double has about 238ns
of spacing at 2026 epoch values, so
"...T00:00:00.9999999Z"rounded up tothe next whole second and the same profile ran one second later through the
Python SDK than through the CLI.
time.rs:calculate_time_offsetresolved both instants withduration_since(UNIX_EPOCH).unwrap_or_default(), which collapses everypre-epoch instant to the epoch. The grammar accepts one
(
"1969-07-20T20:17:00Z"parses), so the sandbox ran a clock the callernever asked for, 14182980 seconds off, with no error anywhere.
Bindings
Go.
go/internal/policyis deleted, both files, 131 lines.MaxMemory,MaxDiskandTimeStartare forwarded verbatim. Every numeric cap isforwarded whenever it is set, zero included, because the core now has a
verdict on zero for each of them; the fields that cannot express "unset" in
the value itself are therefore pointers, with a
Ptrhelper.CPUCoresandGPUDevicesdistinguish nil from empty, so an empty GPU list can still mean"every GPU present". What remains binding-local is the one verdict Go must
reach on its own: an interior NUL in a Go string, which cannot survive the
conversion to a C string.
Python. The size and timestamp fields accept the core's spellings and
forward them; the
__post_init__guards that used to refuse"512M"aregone.
uidandgidbecome oneUservalue, mirroring the core'sRunAs:an unprivileged user namespace maps a single pair, so a uid without a gid is
not a policy the core can apply, and now not a state the SDK can spell.
BREAKING CHANGES
Go SDK.
Name string->*string;OnExit/OnError->*BranchAction;BranchActionDefaultremoved, which renumbersCommit1->0,Abort2->1,Keep3->2;MaxProcesses/MaxCPU/MaxOpenFiles/NumCPUsbecome pointers;UID/GID->User *RunAs;HTTPPorts []int->[]uint16(the C ABIcarries a u16, so a wider Go type wrapped silently, 70000 arriving as 4464);
TimeStartis RFC 3339 only, in exchange carrying sub-second precision andpre-1970 instants;
MaxMemory/MaxDisknarrow to the core's grammar; anon-nil empty
CPUCoresis refused rather than skipped.Python SDK.
uid/gid->user: User | None;Sandbox.cpu_pct()removed;
on_exit/on_errordefault toNone;max_processesdefaults toNone(the effective cap is unchanged at 64, it is simply written down once);SyscallEvent.categorywidens tostr | int; a missing/lib64is no longerdropped from
fs_readablebehind the caller's back; the size and timestampfields accept the core's spellings.
C ABI. The three signatures above, plus the additive
time_start_epoch. No struct, no discriminant and no other signature moves.Canonical profile document.
CanonicalTimestampgains a requiredrfc3339field, with no serde default on a struct that denies unknown fields,so an older document no longer deserializes. It loses
Copybecause it nowholds a
String;Clone,PartialEqandEqare unaffected.The three that are silent at compile time
Most of the list above stops a build or a test run. These do not, and they are
the ones to read before updating:
BranchActionrenumbering. A caller who derives one from a numberrather than naming a constant,
sandlock.BranchAction(n)over a stored orunmarshalled value, still compiles and now selects a different action, with
nothing reporting it. Writing the literal straight into the field is caught,
but only because the same change made that field a pointer, which is an
accident of packaging rather than a guard.
on_errordefault. A policy that says nothing about the errorpath used to abort the branch's writes and now commits them, which is what
the CLI, a profile and the Go SDK have always done for the same policy. A
caller relying on the old default gets no warning.
SyscallEvent.category. Anevent.category == "file"comparison on anunknown category used to be true and is now false.
A C caller passing an integer where a pointer is now expected is a constraint
violation and gets a diagnostic, which is how the signature changes are meant
to be found. The one spelling that slips through is a literal
0, a validnull pointer constant: it compiles, and the C ABI then reports it from
sandlock_sandbox_buildas a NULL argument rather than acting on it.One honest regression
Dropping the
/lib64filter is not in the "fails loudly" group, and callingit one would be a nicer story than the truth.
build()does not check that areadable path exists; nothing in the core does, outside
fs_read_if_exists,which nothing on this path calls. The rule is installed in the child, where
Landlock has to open the path, so a policy naming a missing
/lib64builds,runs, and comes back as
Result(success=False, error="sandlock_create failed")with the path nowhere in it. Under chroot it is skipped without aword instead.
So this trades a silent drop of one hardcoded path for a loud-but-anonymous
failure on any of them: the right trade for who owns the decision, and a plain
regression in diagnosis. Filter system paths on the way in where the set
varies by host, which is what the MCP default policy and
python/examplesdonow. Making the core name the path it could not grant is worth doing and is
not in this PR.
Testing
tests/grammar-corpus.jsonis one corpus above all four surfaces, read bypython/tests/test_setter_grammar_parity.py(flag, profile, C ABI setter viactypes, Python SDK) and
go/grammar_parity_linux_test.go. A corpus pastedinto each language would be the failure being fixed, one table per surface,
free to drift.
go/core_verdict_linux_test.gocovers the verdicts Go stoppedreaching itself.
cargo test -p sandlock-core --lib: 747 pass. Integration: 394. Go: green.The cbindgen header is regenerated here and matches CI's gate.