Let a declared unit carry a file-descriptor limit (#78)

[sehkone]

What this does

UnitTemplate gains limit_nofile: Option<u64>, the one resource-limit field the record has a recorded need for. A package that sets it renders LimitNOFILE=<value> in [Service]; a package that omits it renders exactly the bytes it renders today, and the unit inherits the host's soft limit as every unit does now.

The shapes deliberately not taken: not a map of arbitrary Limit* keys, not a raw [Service] body, and no spelling for infinity, -1, or any other sentinel for "no limit" — absence is that case, so leaving a service unbounded stays the host's decision. u64::MAX is refused for that last reason: it is the numeric value of Linux's RLIM_INFINITY, which is both the sentinel written out and a value systemd's own rlimit parser rejects, so a unit carrying it would render fine and then fail to load.

Closes #78

Changes

• src/module_spec.rs — the field, with #[serde(default, skip_serializing_if = "Option::is_none")] like its optional neighbours, documented in their form. #[serde(deny_unknown_fields)] is untouched. The canonical-layout rustdoc — the single owner of the directive order — now names LimitNOFILE= in its [Service] position. validate_unit rejects Some(0) with a new ModuleSpecError::ZeroLimitNofile and Some(u64::MAX) with a new ModuleSpecError::InfiniteLimitNofile; None and every value systemd can load pass.
• src/render.rs — LimitNOFILE= is emitted immediately after RestartSec= and before the first sandbox boolean, and not at all when the field is None.
• src/manifest.rs — MANIFEST_FORMAT_VERSION and MAX_MANIFEST_FORMAT_VERSION each move from 4 to 5. MIN_MANIFEST_FORMAT_VERSION stays at 3: the field is optional and its absence is today's behaviour, so every payload already published at 3 stays readable and installable and no product has to republish a release asset that is otherwise correct. The three constants' doc comments already state that rule in general terms, so this change repoints the values and leaves the prose alone.
• src/verify.rs — the doc comment on check_format_version said the injected trust-set floor was observable only above the implemented range, which stopped being true once MIN and MAX diverged. Rewritten for the range as it now stands, and covered by a test that puts an injected floor inside the range.
• assets/test-fixtures/unsigned-container/manifest.json — one byte, "format_version":4 → 5. The fixture pins the unsigned writer's absolute manifest bytes, and the producer's stamped version is one of the derived values it exists to catch; a deliberate bump is the case where it is updated rather than investigated.
• CHANGELOG.md — one ### Added entry. A dependent pins this crate at a git rev and reads the changelog to know what moved since its own pin, and the manifest format version it will now be stamped at is exactly that kind of change.

Tests

New:

• the_production_renderer_reproduces_a_limit_setting_anchor_byte_for_byte — the review anchor with the field set, goldened whole, so the entire diff against the existing anchor is one line in one place.
• the_limit_sits_between_restart_sec_and_the_first_sandbox_boolean — the neighbours asserted as adjacent lines, so the directive drifting to the end of the section still fails.
• a_unit_setting_no_limit_renders_no_limit_directive_at_all — matches on Limit, not LimitNOFILE: an absent field must put no resource directive in the unit at all.
• a_present_limit_nofile_must_be_a_limit_systemd_can_load — None, Some(1), Some(8000) and Some(u64::MAX - 1) validate; Some(0) and Some(u64::MAX) each fail with their own variant.
• limit_nofile_round_trips_only_when_it_is_present — the wire anchor plus the key, round-tripped to the same bytes; an explicit null re-serializes back to no key; a sibling limit_nproc, an "infinity" string and a -1 are each refused.
• the_read_path_refuses_a_unit_declaring_an_unloadable_limit — both limit rules reached through the manifest validator a consumer that links no producer actually meets, not only through validate called directly. The RuleCheck alias the sibling read-path test declared inside its own body moves to the test module, since there are now two users.
• a_manifest_at_the_floor_version_still_decodes_validates_and_renders_without_a_limit — a manifest at MIN_MANIFEST_FORMAT_VERSION carrying a spec, taken all the way to rendered bytes rather than stopping at the decode, since the promise the floor makes is about the whole read path.
• an_injected_floor_inside_the_implemented_range_refuses_only_below_itself — a package at the build's floor is refused under a trust set's floor raised to the producer's version, while the producer's own version still verifies under it.

Changed:

• the_producer_writes_the_ceiling_and_the_floor_is_no_higher → the_accepted_range_is_open_below_the_producer_and_closed_at_it, tightening MIN <= MANIFEST to MIN < MANIFEST and pinning both it and MAX == MANIFEST as compile-time assertions, so a later bump that closed the window fails the build rather than one test run.
• the_range_accepts_the_unmoved_floor_as_well_as_the_current_version → every_version_in_the_window_is_accepted_including_the_floor, which loops the whole MIN..=MAX range instead of just its two ends, so the versions between them are covered too.
• the_canonical_directive_order_holds_for_a_record_using_every_optional_field now sets limit_nofile too, since it claims to use every optional field.
• the_wire_form_deserializes_into_the_expected_record_and_back_to_the_same_bytes asserts limit_nofile is None on an anchor that sets none, which is also the assertion that an unchanged package's bytes did not move.
• The round-trip fixture that carries a unit spec through the manifest now sets the field, so it crosses the manifest as a value rather than only as an absence.

Verified the position is actually pinned by moving the emission below the sandbox booleans: the_limit_sits_between_restart_sec_and_the_first_sandbox_boolean and the_canonical_directive_order_holds_for_a_record_using_every_optional_field both go red. the_production_renderer_reproduces_a_limit_setting_anchor_byte_for_byte does not, and cannot: it is built on the review anchor, which sets no sandbox boolean, so LimitNOFILE= is the last line of [Service] either way. That golden pins the rendered bytes of a limit-setting unit; the adjacency between the directive and its neighbours is pinned by the other two.

Test plan

• A template with limit_nofile: Some(n) renders LimitNOFILE=n in [Service] on the line after RestartSec= and before the first sandbox boolean
• A template with limit_nofile: None renders no Limit line anywhere in the unit
• A byte-for-byte golden covers a unit that sets the limit, in the form the existing anchor goldens use
• The existing anchor goldens for templates that set no limit pass unchanged, asserting the serialized bytes did not move
• validate rejects Some(0) with the new ZeroLimitNofile variant and accepts None and Some(1)
• validate rejects Some(u64::MAX), Linux's RLIM_INFINITY, with the new InfiniteLimitNofile variant, so absence stays the only unbounded case
• limit_nofile serializes only when present and deserializes back to the same value, and an unknown sibling key is still rejected by deny_unknown_fields
• A manifest at MIN_MANIFEST_FORMAT_VERSION still decodes, validates and renders; one above MAX_MANIFEST_FORMAT_VERSION is refused with UnsupportedManifestFormat
• MANIFEST_FORMAT_VERSION == MAX_MANIFEST_FORMAT_VERSION and MIN_MANIFEST_FORMAT_VERSION < MANIFEST_FORMAT_VERSION are pinned, so a later change cannot close the window by accident
• cargo fmt --check with group_imports=StdExternalCrate, both clippy runs at -D warnings, cargo doc at -D warnings, and cargo test with and without test-support

[sehkone]

[Reviewer Round 1]

Review finding

• P1 — Reject u64::MAX for limit_nofile. src/module_spec.rs:467 rejects only zero, and the new test explicitly accepts Some(u64::MAX) at src/module_spec.rs:706. That value renders as LimitNOFILE=18446744073709551615, but systemd parses LimitNOFILE with its rlimit-u64 parser and rejects values greater than or equal to RLIM_INFINITY; on Linux, this makes u64::MAX invalid. The manifest therefore passes validation and rendering but produces a unit systemd cannot load. Reject this sentinel (and update the test) so absence remains the only no-limit representation, as required. systemd rlimit parser

Otherwise, the change follows the issue well: it preserves the closed record, renders at the required position, keeps old manifests readable, and includes focused read-path and byte-order tests. PR #80’s Closes #78 linkage and test plan are present; I found no review-thread issue creation.

[sehkone]

[Review Verdict Round 1: NOT_APPROVED]

[sehkone]

[Author Round 1]

Pushed as 3b5f953.

Dispositions

P1 — Reject u64::MAX for limit_nofile — Fixed.

The finding is correct on both counts I checked it against. On Linux RLIM_INFINITY is (rlim_t)-1 — numerically u64::MAX — and systemd's rlimit_parse_u64 returns -ERANGE for u >= RLIM_INFINITY, so LimitNOFILE=18446744073709551615 renders cleanly and then fails to load the unit, moving the failure from the producer to the host at install time. It is also the infinity sentinel the issue forbids, written out in the one spelling a u64 field cannot refuse structurally — so accepting it contradicted "Do not represent infinity, -1, or any sentinel for 'no limit'. Absence is that case."

What changed:

• src/module_spec.rs — new ModuleSpecError::InfiniteLimitNofile, named in the existing taxonomy's shape alongside ZeroLimitNofile, and validate_unit now refuses Some(u64::MAX). The two rules read as one match with a comment recording why each value is refused. The field's rustdoc changes from "Non-zero when present" to "neither zero nor u64::MAX", with the RLIM_INFINITY reason stated where a package author will read it.
• a_present_limit_nofile_must_be_non_zero → a_present_limit_nofile_must_be_a_limit_systemd_can_load. Some(u64::MAX) moves out of the accepted set into the rejected one, and Some(u64::MAX - 1) takes its place so the top of the usable range is still covered — the rule is one forbidden sentinel, not a ceiling.
• the_read_path_refuses_a_unit_declaring_a_zero_limit → ..._an_unloadable_limit, now table-driven over both values, so the new rule reaches a consumer through the manifest validator the same way the zero rule does. Its RuleCheck alias moved from inside the sibling read-path test's body to the test module, since there are two users now; that also settles the clippy::type_complexity the inline fn pointer type tripped.
• CHANGELOG.md — the existing ### Added entry now names both refusals as one sentence about why absence is the only unbounded case. No new bullet: no user of the last release saw the earlier form of this field.

Nothing else in the review round to push back on — the reviewer raised one item and it stood up.

CI checks all pass locally: cargo fmt --check with group_imports=StdExternalCrate, both clippy runs at -D warnings, cargo doc at -D warnings, and cargo test with and without test-support (469 + 3 / 469 + 4, zero failures). PR #80's body was updated to match — the "not taken" paragraph, the module_spec.rs change bullet, both renamed tests, and a new test-plan line for the sentinel rule; Closes #78 is intact and no other issue declaration was added.

[sehkone]

[Reviewer Round 2]

[Reviewer Round 2: APPROVED]

No remaining findings.

Round 1 P1 is resolved: u64::MAX is rejected as InfiniteLimitNofile in src/module_spec.rs, with direct and manifest read-path coverage. The field remains closed and optional, renders in the required position, and preserves old manifests via the unchanged floor and updated ceiling in src/manifest.rs. PR linkage and test plan are present.

[sehkone]

[Review Verdict Round 2: APPROVED]

[sehkone]

Suggested squash commit

Title

Let a declared unit carry a file-descriptor limit

Body

A package could describe its systemd unit's ordering, environment,
restart policy and sandboxing, but not the descriptors it needs, so a
service whose store outgrows the soft limit systemd hands a unit left
an operator to raise it on the host. The package was then not the whole
description of what the service needs, which is the property the
declared spec exists to provide.

`UnitTemplate` gains one typed `limit_nofile`, not a map of arbitrary
`Limit*` keys and not a raw `[Service]` body: one recorded need does
not justify a general escape hatch into systemd's directive space.
Absence keeps today's behaviour of inheriting the host's soft limit and
is the only spelling for an unbounded service, so leaving one unbounded
stays the host's decision rather than a package's. A declared zero is
refused, and so is `u64::MAX` — the numeric value of Linux's
`RLIM_INFINITY`, which systemd's own rlimit parser rejects, so a unit
carrying it would render fine and then fail to load.

The producer's manifest format version and the ceiling both move,
because `deny_unknown_fields` makes an older build refuse a manifest
carrying the new key and the version is what makes that refusal legible
as `UnsupportedManifestFormat`. The floor stays put: the field is
optional and its absence is what every already published payload
means, so none has to be rebuilt and republished to keep working.

A doc comment on `check_format_version` still described the floor and
the ceiling as equal, which they had already stopped being. It is
rewritten for the range as it now stands, and a test now puts an
injected trust-set floor inside that range rather than only above it.

Closes #78