Skip to content

gpt: reject '0<unit>' partition start values that crash sgdisk - #1284

Open
yottanami wants to merge 2 commits into
nix-community:masterfrom
yottanami:agent/53-start-0M-sgdisk-check
Open

gpt: reject '0<unit>' partition start values that crash sgdisk#1284
yottanami wants to merge 2 commits into
nix-community:masterfrom
yottanami:agent/53-start-0M-sgdisk-check

Conversation

@yottanami

Copy link
Copy Markdown

sgdisk treats a bare "0" for start as "next available range", but "0" directly followed by a size unit (0M, 0K, 0G, 0T, 0P) is parsed as a literal zero-byte offset instead — which collides with the GPT header/table area and makes sgdisk fail with a confusing error instead of doing what the user expects (see the errors quoted in the issue).

This adds a type check on the start option (lib/types/gpt.nix), following the same pattern used by the size option a few lines above, that rejects this specific footgun at Nix-eval time with a clear description, while still allowing a bare "0" and any non-zero offset (e.g. "1M", "2048", "-1M").

Checked all in-repo usages of start on gpt-typed partitions (examples + tests) — they all use non-zero offsets like "1M" already, so this shouldn't affect any existing configuration.

I don't have a local Nix environment to run nix flake check / the test suite against this change — flagging that so a maintainer/CI can verify the syntax and formatting (nixfmt) directly.

Fixes #702

sgdisk treats a bare '0' as 'next available range', but '0' directly
followed by a size unit (0M, 0K, 0G, 0T, 0P) is parsed as a literal
zero-byte offset instead, which collides with the GPT header/table
area and makes sgdisk fail with a confusing error instead of doing
what the user expects.

Add a type check on the start option (same pattern as the size option
a few lines above) that rejects this specific footgun at eval time,
while still allowing a bare '0' and any non-zero offset.

Fixes nix-community#702
The repo's jsonTypes check (lib/default.nix's typesSerializerLib) runs
option definitions through a stub lib for doc generation that doesn't
implement addCheck, so the previous commit's direct
lib.types.addCheck call crashed that check with 'attribute addCheck
missing'.

Guard the lookup with 'or' so the doc-generation stub degrades to a
no-op (same as any other unrestricted str option there) while real
evaluation still gets the genuine addCheck and its validation.
@yottanami

Copy link
Copy Markdown
Author

Pushed a follow-up commit: the jsonTypes CI check failed because lib/default.nix's doc-generation stub lib doesn't implement lib.types.addCheck (confirmed by reading typesSerializerLib). Guarded the lookup with or so the stub degrades to a no-op there while real evaluation still gets full addCheck validation.

Also noting: the treefmt failures on this PR (openTempFileWithDefaultPermissions: permission denied) appear to be a pre-existing CI infra issue unrelated to this change — the same failure shows up on #1280, an unrelated open PR from another contributor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sgdisk behaves confusingly when specifying partitions.${name}.start = "0M"

1 participant