feat: Implementation offsets in paths.json by chrisburr · Pull Request #2565 · conda/rattler

chrisburr · 2026-07-07T21:08:41Z

Description

Follow up to #2032 that is needed for the (about to be opened) draft RattlerFS PR. See the original PR for details.

Closes #2032

How Has This Been Tested?

See the testing in the coming RattlerFS and Pixi PRs.

AI Disclosure

This PR contains AI-generated content.
- I have tested any AI-generated content in my PR.
- I take responsibility for any AI-generated content in my PR.

This is mostly the work of @Dagmar-Dinjens. The AI part has come as part of many many sessions working on the downstream parts of this so there isn't a clear prompt.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added sufficient tests to cover my changes.

baszalmstra · 2026-07-08T08:35:22Z

I think this will require a Conda Enhancement Proposal (CEP), but I would be happy to merge this in rattler already. Do you think you can take care of writing a CEP?

chrisburr · 2026-07-08T09:04:00Z

Yeah I can add that to my list, though it proabably won't happen for a week or two.

baszalmstra · 2026-07-08T14:54:42Z

+    let mut last_match = 0;
+
+    for &offset in offsets {
+        destination.write_all(&source_bytes[last_match..offset])?;


If the offsets are corrupted for whatever reason this could panic.

Fable original comment:

offsets comes straight from a package's paths.json (attacker-controllable) and is never validated. Empty binary groups panic at group.split_at(group.len() - 1) (usize underflow); out-of-range/out-of-order offsets panic at &source[src_start..src_end]. A package with "offsets": [[1000, 2000]] on a 10-byte file kills the FUSE/NFS read thread → whole mount dies. The identical arithmetic in the install-time variants panics the installer. Needs a validation pass (sorted, in-bounds, spaced ≥ placeholder length) at build/mount time.

chrisburr · 2026-07-09T11:36:11Z

I'm about to push an amended version of this PR based on the review above and the CEP that I've prepared. Though it has exposed a messy edge case described here: https://github.com/conda/ceps/pull/179/changes#r3551264320

Correct the doc comments on `Offsets`, `PrefixPlaceholder::offsets` and `PrefixPlaceholder::shebang_length` to match the draft CEP "Prefix placeholder offsets in paths.json" (conda#2565): - `offsets` exclude occurrences inside the shebang region, and for binary files the group's last value is the NUL terminator offset or the file size when the final C string is unterminated at end-of-file. - `shebang_length` is present if and only if `offsets` is present, `file_mode` is text, and the file starts with `#!` — independent of whether the first line contains the placeholder. - The `Offsets` shape is determined normatively by `file_mode`, not inferred from the JSON structure.

Bring install-time prefix replacement into line with the draft CEP "Prefix placeholder offsets in paths.json" (conda#2565). `offsets` are absolute byte positions that exclude the shebang region and are spliced uniformly on every platform; the shebang region (first `shebang_length` bytes) is transformed separately. - Plumb `shebang_length` from `PrefixPlaceholder` into the offset functions and use it as the region boundary instead of re-deriving it from content, validating it equals first-0x0A-index + 1. - Add the missing non-rewriting-target rule: on Windows (e.g. a noarch package) the shebang region gets plain placeholder replacement rather than being left untouched. On Unix it is rewritten by the shebang rules (region minus its trailing newline, newline copied through verbatim). - Report producer non-conformance (in-region offset, shape/mode mismatch, out-of-range or unordered offsets, missing placeholder bytes, empty binary outer list, `#!` file without `shebang_length`) as a distinguishable `OffsetReplaceError::InconsistentMetadata`. The offset functions validate before writing anything, so `link_file` falls back to the search-based path (with a warning) using the still-empty destination instead of failing the install. - Fix a pre-existing panic in the search-based `copy_and_replace_textual_ placeholder`: a `#!` file with no newline fed an empty line into `replace_shebang`, tripping its `starts_with("#!")` assertion. - Fix the binary test that recorded the NUL terminator one byte past the actual `\x00`, and align the suite with the CEP's informative test-vector list (short/long Unix prefixes, non-rewriting target, no trailing newline, empty offsets, multiple in-shebang occurrences, over-long shebang with no occurrence, unterminated final C string, and the fallback for non-conformant input).

Add read-only `offsets` and `shebang_length` getters to the py-rattler `PrefixPlaceholder` so consumers can inspect the CEP fields (conda#2565). `offsets` returns `None`, a `list[int]` for text-mode files, or a `list[list[int]]` for binary-mode files (grouped by C string).

chrisburr · 2026-07-14T06:25:11Z

I think this will require a Conda Enhancement Proposal (CEP), but I would be happy to merge this in rattler already. Do you think you can take care of writing a CEP?

@baszalmstra How does this fit into the potentially open question in the CEP about encodings? I don't mind iterating here a couple of times but I guess you don't want to deal with backwards compatibility issues if it changes too much.

baszalmstra · 2026-07-14T12:33:52Z

@chrisburr Since this code is not breaking (its an addition) I would be happy to iterate here. As long as we mark the fields experimental_ we warn people that this can break. Once the CEP is merged, we make the fields stable.

(We only need to prefix the fields experimental_ in rust, we can leave the serialized form as is).

chrisburr · 2026-07-14T13:06:49Z

(We only need to prefix the fields experimental_ in rust, we can leave the serialized form as is).

Doesn't it become breaking if it's in people's caches and a future version of rattler doesn't understand the outdated schema? I see how experimental_offsets would avoid that problem but not if it's only in rust.

baszalmstra · 2026-07-14T13:40:15Z

Yeah that is true but as far as I understand we wont be adding it to any caches right now?

We can also mark those fields in serde as non-failing. E.g. just use a default value if parsing fails.

improved permission checking, improved error message, improved slice usage in link.rs, Still have to look at the v2 backwards incompatibility comment

Finalise the offset-based prefix replacement: a structured Offsets enum (text vs binary), memchr-based NUL scanning, and the `copy_and_replace_placeholders_with_offsets` family, so install (and other consumers) can replace the install prefix at known byte offsets instead of rescanning each file. The `offsets` field on PathsEntry/PrefixPlaceholder is optional and skipped when absent, so paths.json stays backward-compatible (paths_version unchanged).

Correct the doc comments on `Offsets`, `PrefixPlaceholder::offsets` and `PrefixPlaceholder::shebang_length` to match the draft CEP "Prefix placeholder offsets in paths.json" (conda#2565): - `offsets` exclude occurrences inside the shebang region, and for binary files the group's last value is the NUL terminator offset or the file size when the final C string is unterminated at end-of-file. - `shebang_length` is present if and only if `offsets` is present, `file_mode` is text, and the file starts with `#!` — independent of whether the first line contains the placeholder. - The `Offsets` shape is determined normatively by `file_mode`, not inferred from the JSON structure.

Bring install-time prefix replacement into line with the draft CEP "Prefix placeholder offsets in paths.json" (conda#2565). `offsets` are absolute byte positions that exclude the shebang region and are spliced uniformly on every platform; the shebang region (first `shebang_length` bytes) is transformed separately. - Plumb `shebang_length` from `PrefixPlaceholder` into the offset functions and use it as the region boundary instead of re-deriving it from content, validating it equals first-0x0A-index + 1. - Add the missing non-rewriting-target rule: on Windows (e.g. a noarch package) the shebang region gets plain placeholder replacement rather than being left untouched. On Unix it is rewritten by the shebang rules (region minus its trailing newline, newline copied through verbatim). - Report producer non-conformance (in-region offset, shape/mode mismatch, out-of-range or unordered offsets, missing placeholder bytes, empty binary outer list, `#!` file without `shebang_length`) as a distinguishable `OffsetReplaceError::InconsistentMetadata`. The offset functions validate before writing anything, so `link_file` falls back to the search-based path (with a warning) using the still-empty destination instead of failing the install. - Fix a pre-existing panic in the search-based `copy_and_replace_textual_ placeholder`: a `#!` file with no newline fed an empty line into `replace_shebang`, tripping its `starts_with("#!")` assertion. - Fix the binary test that recorded the NUL terminator one byte past the actual `\x00`, and align the suite with the CEP's informative test-vector list (short/long Unix prefixes, non-rewriting target, no trailing newline, empty offsets, multiple in-shebang occurrences, over-long shebang with no occurrence, unterminated final C string, and the fallback for non-conformant input).

Add read-only `offsets` and `shebang_length` getters to the py-rattler `PrefixPlaceholder` so consumers can inspect the CEP fields (conda#2565). `offsets` returns `None`, a `list[int]` for text-mode files, or a `list[list[int]]` for binary-mode files (grouped by C string).

- "Check intra-doc links" (`-D rustdoc::private-intra-doc-links`): the public `copy_and_replace_textual_placeholder_offsets` doc linked to the private `replace_shebang` fn; demote it to a plain code span. - Windows-x86_64: `test_replace_long_prefix_in_text_file_offsets` hardcoded `shebang_length: Some(44)`, which is wrong when the `shebang_test.txt` fixture is checked out with CRLF (the carriage return shifts the first newline to 45). Derive `shebang_length` from the file contents instead.

The offsets CEP (conda/ceps#179) was restructured on Jul 15: `offsets` is no longer a bare list of positions but a list of offset groups, each recording the occurrences under one encoding: "offsets": [{"encoding": "utf-8", "ranges": [10, 45]}] Installers do not agree on which encodings of the placeholder they replace (conda searches UTF-8 plus UTF-16/UTF-32 variants; rattler and libmamba search UTF-8 only), so each installer applies exactly the groups its own search-based replacement covers and one package serves every installer. - `Offsets` becomes `OffsetRanges` inside a new `OffsetGroup` carrying an `OffsetEncoding`; unknown encoding names and unrecognized group members parse (they must not fail the whole paths.json) but mark the metadata as unusable. - `select_utf8_offset_ranges` validates the CEP's structural rules and picks the UTF-8 group; rattler skips the other defined encodings, whose occurrences its own search would not have replaced either. - The installer treats invalid metadata as inconsistent (falling back to search-based replacement) and copies files with no UTF-8 group through unchanged apart from text shebang handling; pre-CEP flat offsets deserialize as absent rather than failing the parse. - Adds the CEP's new test vector 9 (a binary file with occurrences under more than one encoding) and the Examples-section entries as tests.

Shim branch for the pixi-rattlerfs integration, re-cut from the rattler_build_core-v0.2.9 publish tag (whose rattler requirements match the fork's 0.48/0.30): PrefixPlaceholder gained `offsets` and `shebang_length` fields; construct them as None ("publisher did not populate"), so installers locate occurrences by searching, exactly as for pre-CEP packages. Recording real offsets at build time is the eventual goal but out of scope for this shim.

…ental_ Per review discussion on conda#2565: until conda/ceps#179 is finalized the schema may still change, so mark the Rust API as unstable by renaming the PrefixPlaceholder fields to `experimental_offsets` and `experimental_shebang_length` (py-rattler properties follow suit). The serialized form is unchanged — serde renames keep `offsets` and `shebang_length` on the wire, as the CEP specifies. One self-contained commit so it can be reverted wholesale once the CEP merges and the fields become stable.

chrisburr · 2026-07-17T14:58:01Z

Indeed, I've added an issue for adding support for writing paths.json when the CEP is finished: #2607

In the meantime I think this is ready.

baszalmstra

Can you make the comments less verbose, and generally less AIy, strip them from emdashes for instance.

One thing I dont understand is that this CEP adds support for other encodings but it looks like this code only deals with UTF-8? Why not also add support for the other encodings?

baszalmstra · 2026-07-21T08:20:55Z

+    /// treated like corrupt metadata (fall back to searching). Unrecognized
+    /// members are not preserved on re-serialization.
+    #[serde(skip)]
+    pub has_unknown_members: bool,


Would it make sense to store these extra keys in here? So we can report an error?

baszalmstra · 2026-07-21T08:22:39Z

+/// occurrences by searching the file contents (and MAY report a warning).
+#[derive(Debug, thiserror::Error)]
+#[error("{0}")]
+pub struct InvalidOffsetsError(String);


I would prefer to use an enum.

baszalmstra · 2026-07-21T08:23:58Z

+/// Per the CEP, an installer applies exactly the groups whose encodings its own search-based
+/// replacement covers. rattler's search-based replacement covers UTF-8 only, so the UTF-8 group's
+/// ranges (selected and structurally validated by [`select_utf8_offset_ranges`]) are spliced by
+/// [`copy_and_replace_textual_placeholder_offsets`] or
+/// [`copy_and_replace_cstring_placeholder_offsets`]; groups for the other defined encodings are
+/// skipped, since their occurrences would not have been replaced by the search either. Valid
+/// metadata without a UTF-8 group means there is nothing to splice: the file is copied through
+/// unchanged apart from the shebang handling of text files.


Is that because rattler doenst impement utf-16 replacement? We could implement that for binary files couldnt we?

baszalmstra · 2026-07-21T08:25:05Z

+    for group in groups {
+        // Each group lists the prefix offsets followed by the NUL terminator position.
+        let Some((&nul_pos, prefix_offsets)) = group.split_last() else {
+            return Err(OffsetReplaceError::inconsistent(
+                "binary offset group is empty",
+            ));
+        };
+        if prefix_offsets.is_empty() {
+            return Err(OffsetReplaceError::inconsistent(
+                "binary offset group has no prefix offsets",
+            ));
+        }
+        if nul_pos > source_bytes.len() {
+            return Err(OffsetReplaceError::inconsistent(format!(
+                "NUL offset {nul_pos} is out of range for content of length {}",
+                source_bytes.len()
+            )));
+        }
+        for &offset in prefix_offsets {
+            if offset < prev_end {
+                return Err(OffsetReplaceError::inconsistent(
+                    "binary offsets are not sorted / c-string ranges overlap",
+                ));
+            }
+            let end = offset
+                .checked_add(old_prefix.len())
+                .filter(|&end| end <= nul_pos)
+                .ok_or_else(|| {
+                    OffsetReplaceError::inconsistent(format!(
+                        "offset {offset} does not fit before its NUL terminator {nul_pos}"
+                    ))
+                })?;
+            if &source_bytes[offset..end] != old_prefix {
+                return Err(OffsetReplaceError::inconsistent(format!(
+                    "placeholder bytes are not present at recorded offset {offset}"
+                )));
+            }
+            prev_end = end;
+        }
+        prev_end = nul_pos;
+    }


Maybe we should move this to a seperate function?

Same for the text variant

chrisburr · 2026-07-21T12:51:53Z

One thing I don't understand is that this CEP adds support for other encodings but it looks like this code only deals with UTF-8? Why not also add support for the other encodings?

That changes the path which scans for the prefix which felt like it was out of scope at this point, especially as it's a much less trivial scan than the current one used in rattler. It should definitely be a follow up.

- Turn `InvalidOffsetsError` into an enum with typed variants. - Store the names of unrecognized offset-group members (`unknown_members` replaces `has_unknown_members`) so errors can report them. - Extract the offset validation into separate functions for the text and binary variants. - Trim doc comments. Prefix replacement still applies the UTF-8 group only; supporting the other encodings requires extending the search-based replacement first and is left as a follow-up.

chrisburr mentioned this pull request Jul 7, 2026

feat: add rattler_vfs crate #2566

Draft

7 tasks

chrisburr force-pushed the feat/paths-offsets branch from 494c6dd to 2fb4bc6 Compare July 7, 2026 23:14

baszalmstra requested changes Jul 8, 2026

View reviewed changes

chrisburr mentioned this pull request Jul 9, 2026

CEP XXXX: Prefix placeholder offsets in paths.json conda/ceps#179

Open

10 tasks

DagmarFiep and others added 15 commits July 17, 2026 14:48

paths version 2 support

243afe0

fixing lint error's

9281419

improved c-string function,

34c05a0

improved permission checking, improved error message, improved slice usage in link.rs, Still have to look at the v2 backwards incompatibility comment

fixed some warnings & the python lint run error's

c2b26e4

implemented keeping version 1 and the executable field

963058f

ran lint again

558a458

rename snap file

c0b9300

formatting in the py-rattler paths_json

e711a17

PyPrefixPlaceholder empty offstes field flag

e18358e

chrisburr force-pushed the feat/paths-offsets branch from 1938194 to 560c953 Compare July 17, 2026 12:56

chrisburr mentioned this pull request Jul 17, 2026

Include offsets when writing paths.json #2607

Open

2 tasks

baszalmstra reviewed Jul 21, 2026

View reviewed changes

Dagmar-Dinjens mentioned this pull request Jul 23, 2026

feat(exec): mount temporary environments via rattler_vfs #2618

Draft

7 tasks

Uh oh!

Conversation

chrisburr commented Jul 7, 2026

Description

How Has This Been Tested?

AI Disclosure

Checklist:

Uh oh!

baszalmstra commented Jul 8, 2026

Uh oh!

chrisburr commented Jul 8, 2026

Uh oh!

Uh oh!

baszalmstra Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

chrisburr commented Jul 9, 2026

Uh oh!

chrisburr commented Jul 14, 2026

Uh oh!

baszalmstra commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisburr commented Jul 14, 2026

Uh oh!

baszalmstra commented Jul 14, 2026

Uh oh!

chrisburr commented Jul 17, 2026

Uh oh!

baszalmstra left a comment

Choose a reason for hiding this comment

Uh oh!

baszalmstra Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

baszalmstra Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

baszalmstra Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

baszalmstra Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

baszalmstra Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

chrisburr commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

baszalmstra commented Jul 14, 2026 •

edited

Loading