Let the core own the size and timestamp grammar; bindings forward text - #276
Merged
Merged
Conversation
calculate_time_offset converted both instants with
duration_since(UNIX_EPOCH).unwrap_or_default(), which turns any instant
before the epoch into the epoch itself. The timestamp grammar accepts
such instants ("1969-07-20T20:17:00Z" parses), so the sandbox ran a
clock months away from the one requested, with no error. Floor to
signed seconds instead.
Signed-off-by: Cong Wang <cwang@multikernel.io>
The CLI and the profile loader each carried a private jiff parser for time_start, and the profile renderer converted back through whole non-negative seconds, so a profile naming a pre-1970 instant came back with no time_start at all and a sub-second one came back rounded down. Expose parse_timestamp and format_timestamp next to ByteSize::parse so the core holds both directions of the grammar in one place, route the CLI and profile through them, and drop the CLI's jiff dependency. A binding that needs to read a timestamp now has a core routine to reach instead of a reason to write its own. Signed-off-by: Cong Wang <cwang@multikernel.io>
A binding holding "512M" or an RFC 3339 instant had nowhere in the core to hand it: max_memory, max_disk and time_start take the parsed type, so the binding had to parse first and ended up owning a grammar of its own. Add max_memory_spec, max_disk_spec and time_start_spec, which read the same grammar as the CLI and the profile. Builder setters cannot fail, so a value that does not parse is held and returned by build(), the same point where net and HTTP rule specs are reported. The first rejection wins: a later valid call must not quietly paper over a value the caller got wrong. Signed-off-by: Cong Wang <cwang@multikernel.io>
max_memory and max_disk took a byte count and time_start whole epoch seconds, so every binding whose users write "512M" or an RFC 3339 instant had to parse it first. Both did, and both drifted from the core: they accepted "1.5G" and "1T", which no flag or profile accepts, and the Go one took bare epoch counts and refused anything before 1970. Take the text and hand it to the builder's *_spec setters, so the core reads the one grammar and a refused value comes back from sandlock_sandbox_build with the core's message. The epoch-seconds form also could not carry a sub-second or pre-1970 start; the text form carries both. A null string frees the builder and returns null, as an out-of-range BranchAction does: skipping it instead would build a sandbox without the limit the caller asked for. Signed-off-by: Cong Wang <cwang@multikernel.io>
go/internal/policy parsed MaxMemory, MaxDisk and TimeStart itself, by rules copied from the Python SDK rather than from the core: it took "1.5G" and "1T", which the CLI and profiles refuse, took a bare epoch count, which a profile refuses, and refused a pre-1970 instant, which a profile takes. The same field meant different things depending on which surface carried it. Now that the C ABI takes the text, pass it through unchanged and let the build report anything the core refuses. TimeStart is RFC 3339 only. Signed-off-by: Cong Wang <cwang@multikernel.io>
The SDK parsed max_memory and max_disk with its own regex, which took "1.5G" and "1T" though no flag or profile does, and time_start never worked as text at all: the builder called int() on it, so any RFC 3339 string raised ValueError. Numbers were truncated to whole seconds, and time_start_timestamp quietly read a naive time as UTC. Forward the values as text and let the build report what the core refuses. time_start takes an RFC 3339 string or a datetime, which is rendered with isoformat(); a naive datetime has no offset and is refused by the core like any offset-less string, rather than being pinned to a zone the caller never named. parse_memory_size, memory_bytes and time_start_timestamp go with the grammar they carried. Signed-off-by: Cong Wang <cwang@multikernel.io>
The C ABI carries an HTTP port as a u16. Go held HTTPPorts as []int and converted with C.uint16_t, and Python passed through ctypes.c_uint16; both truncate silently, so port 70000 intercepted traffic on 4464. Make the Go field []uint16 so the compiler refuses what the ABI cannot carry, and have Python refuse an out-of-range port before it reaches ctypes, which offers no such check of its own. Signed-off-by: Cong Wang <cwang@multikernel.io>
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.
Problem
The C ABI took
max_memory/max_diskas a byte count andtime_startas whole epoch seconds, so every binding whose users write"512M"or an RFC 3339 instant had to parse it first. Both SDKs did, and both drifted from the core:"1.5G","0.5K"and"1T", which no CLI flag or profile accepts.TimeStarttook a bare epoch count (a profile refuses it) and refused a pre-1970 instant (a profile takes it).time_startcrashed withValueError(the builder calledint()on it), and naive datetimes were silently read as UTC.calculate_time_offset, and the profile renderer dropped it entirely.HTTPPorts []intand Python'sc_uint16silently wrapped port 70000 to 4464.Fix
The core owns the grammar; bindings forward text.
time: compute the offset with signed, floored seconds.core: one publicparse_timestamp/format_timestampnext toByteSize::parse, used by the CLI and profile;sandlock-clidropsjiff.sandbox:max_memory_spec,max_disk_spec,time_start_specbuilder setters. A value that does not parse is held and returned bybuild()naming the knob; the first rejection wins.ffi: the three setters takeconst char *; a refused value comes back fromsandlock_sandbox_buildinerr_msg, a null string returns a null builder.go: deletego/internal/policy, forward the strings.python: deleteparse_memory_size,memory_bytes,time_start_timestamp; forward the strings (datetimeviaisoformat()).bindings: GoHTTPPortsbecomes[]uint16; Python refuses ports outside 0 to 65535.Breaking changes
max_memory,max_disk,time_starttake strings.TimeStartis RFC 3339 only;HTTPPortsis[]uint16;MaxMemory/MaxDiskfollow the core grammar.time_startisdatetime | str(no numbers; naive datetimes refused); sizes follow the core grammar.Not in scope
Python's
cpu_pct()still clampsmax_cpuitself, and other numeric fields (e.g. process limits) can still wrap at the ctypes/cgo boundary.Testing
New Rust unit tests (time, profile round trip, builder spec setters),
crates/sandlock-ffi/tests/size_time_spec.rs, Go tests for refused values and a 1969 start, Python tests for accepted/refused spellings, a 1969 run, and port range. Go suite green; 427 Python tests pass locally.🤖 Generated with Claude Code