Skip to content

feat: range decryption API on top of the fee package (FIL-570) - #4

Merged
bajtos merged 19 commits into
mainfrom
fil-570-stream-api
Aug 18, 2026
Merged

feat: range decryption API on top of the fee package (FIL-570)#4
bajtos merged 19 commits into
mainfrom
fil-570-stream-api

Conversation

@bajtos

@bajtos bajtos commented Aug 3, 2026

Copy link
Copy Markdown
Member

Adds range decryption to the root fee package, so a caller can decrypt one plaintext byte range of a stored FEE blob without fetching or decrypting the whole object, and without reaching into fee/aesstream or the COSE/wrap sub-packages to do it.

The range path reuses the envelope-decode and recipient-unwrap logic that Decrypt uses (matchRecipient, RecipientUnwrapper.unwrap), which is why it lives inside the fee package rather than a sub-package, and adds only the range piece: locating the chunk-aligned ciphertext span and handing it to aesstream's range primitive instead of the whole stream.

Entry points

  • DecryptRange(blob, blobSize, unwrap, off, length) decodes the envelope header from the blob, recovers the CEK, and reads only the ciphertext chunks the range overlaps.
  • DecryptRangeWithCEK is the external-CEK counterpart. It accepts the envelope parameters as an optional caller-supplied metadata. When provided, it reads no envelope at all: the only I/O left is the ciphertext the range overlaps.
  • PlaintextSize(blob, blobSize) answers an object's decrypted size from the envelope header alone, with no key material, for HEAD responses and suffix ranges. BodyDescriptor.PlaintextSize(blobSize) answers the same question from cached metadata, reading nothing.

RangeReader exposes Len (the clamped range length) and Size (the whole object's plaintext size), so an HTTP consumer can fill in Content-Length and Content-Range before any ciphertext is read, plus CiphertextSpan so a remote-backed caller can prefetch the span in a single range request.

Cacheable envelope parameters

The envelope is a fixed prefix of every stored object, so a store that keeps its own metadata beside the blob can record what a range decrypt needs from it and skip the header read. BodyDescriptor is those four values: envelope length, base nonce, chunk size, and the Enc_structure AAD. Every one is non-secret, already in the clear at the front of the blob; the CEK is deliberately not among them.

The AAD is stored whole rather than rebuilt from the protected header it contains, because the Enc_structure's context string differs between a COSE_Encrypt and a recipient-less COSE_Encrypt0. Caching the protected header alone would need a companion flag recording which form was written; the finished bytes keep BodyDescriptor identical for both. Validate is the all-or-nothing check a store applies before persisting a record, and DecryptRangeWithMaterial calls it so a bad value fails with ErrIncompleteMaterial rather than as an authentication error further down.

A stale or corrupted record cannot serve wrong plaintext. All four values are either bound into every chunk's GCM tag or decide which bytes are read under which nonce, so a wrong one fails with aesstream.ErrCorrupted. The one check the cached path gives up is ErrSizeMismatch: with no envelope to consult, nothing cross-checks blobSize against the declared chunk count.

Breaking change: Encrypt and EncryptWithCEK now return (io.ReadCloser, BodyDescriptor, error). The descriptor is complete on return, before any plaintext is read, so a writer can record it while the upload is still streaming. Callers with no use for it discard the second result.

Envelope header probe

The header is located by probing a small prefix of the blob with cose.Decode, which reports its exact encoded length. The probe grows only when the decode failed because the input ran out mid-item, and is bounded at 1 MiB. To make that distinction available, cose.Decode and cose.DecodeReader now wrap io.ErrUnexpectedEOF in ErrMalformed for a truncated leading item; every other decode failure is final, and the probe stops on it instead of re-reading at ever larger sizes.

When the envelope records a chunk count, it is cross-checked against the geometry the blob size implies (ErrSizeMismatch), catching a stale or wrong size before any plaintext is served. The count is unprotected metadata, so this is an operational check, not an integrity guarantee, and is documented as such.

Authentication scope

Every chunk a range overlaps is authenticated, so a tampered chunk fails rather than yielding corrupt plaintext. Chunks outside the range are never fetched and so never checked, which the docs spell out along with the trust placed in the supplied blob size.

Adds DecryptRange / DecryptRangeWithCEK to the root fee package, so a caller
can decrypt one plaintext byte range of a stored FEE blob without fetching or
decrypting the whole object — and without reaching into fee/aesstream or the
COSE/wrap sub-packages to do it.

The range path reuses the envelope-decode and recipient-unwrap logic that
Decrypt uses (matchRecipient, RecipientUnwrapper.unwrap), which is why it lives
inside the fee package rather than a sub-package, and adds only the range piece:
locating the chunk-aligned ciphertext span and handing it to aesstream's range
primitive instead of the whole stream.

  - RangeReader exposes Len (the clamped range length) and Size (the whole
    object's plaintext size), so an HTTP consumer can fill in Content-Length and
    Content-Range before any ciphertext is read, plus CiphertextSpan so a
    remote-backed caller can prefetch the span in a single range request.
  - PlaintextSize answers an object's decrypted size from the envelope header
    alone, with no key material — for HEAD responses and suffix ranges.
  - The envelope header is located by probing a small prefix with cose.Decode,
    which reports its exact encoded length; the probe grows only for unusually
    large envelopes and is bounded at 1 MiB.
  - When the envelope records a chunk count, it is cross-checked against the
    geometry the blob size implies (ErrSizeMismatch), catching a stale or wrong
    size before any plaintext is served. The count is unprotected metadata, so
    this is an operational check, not an integrity guarantee — documented as such.

openStream's body-header validation is extracted into a shared validateBody
helper so the whole-object and range paths accept exactly the same envelopes and
report the same errors. No behavior change on the existing path.

Every chunk a range overlaps is authenticated, so a tampered chunk fails rather
than yielding corrupt plaintext; chunks outside the range are never fetched and
so never checked, which the docs spell out along with the trust placed in the
supplied blob size.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CF6S9hmvMkpTkDyRvYopeN

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a byte-range decryption API to the root fee package so callers can decrypt a plaintext subrange from a stored FEE blob without fetching/decrypting the full object, while reusing existing envelope decode + recipient unwrap logic.

Changes:

  • Introduces DecryptRange, DecryptRangeWithCEK, PlaintextSize, and a streaming RangeReader backed by aesstream.SpanReader.
  • Refactors whole-object decryption to share body-header validation via a new validateBody helper.
  • Updates README and adds comprehensive tests + an example for range decryption behavior and I/O characteristics.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
README.md Updates docs/examples to describe and demonstrate fee.DecryptRange and related APIs.
range.go Implements the new range decryption + header-only plaintext size API in the root package.
range_test.go Adds extensive unit tests covering correctness, bounds, I/O span behavior, and tamper semantics.
fee.go Extracts shared envelope body-header validation to align whole-object and range paths.
example_range_test.go Adds a runnable example demonstrating DecryptRange usage and headers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md
Comment thread example_range_test.go Outdated
@bajtos
bajtos marked this pull request as draft August 4, 2026 07:00
bajtos and others added 2 commits August 4, 2026 11:06
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread range.go
A range decrypt currently has to read and CBOR-decode the envelope at the
front of the blob before it can touch a single ciphertext byte, because that
is where the base nonce, chunk size and AAD live. For a caller fronting a
remote object store that is a round trip per range request, spent re-reading
a fixed prefix that never changes.

BodyMaterial is what a caller needs to skip it: the envelope's encoded
length, base nonce, chunk size and AAD. EncryptedBlob.Material reports it
from the encrypt call (complete before any plaintext is read, so a writer can
record it while the upload streams), and DecryptRangeWithMaterial consumes it
on the read path, fetching only the chunks the range overlaps. Measured on a
3-chunk object, a 50-byte range goes from reads at {0,4096} + {201,4112} to
just {201,4112} — the header probe disappears.

Cache the AAD, not the protected header
---------------------------------------
The obvious thing to cache is the protected header, since that is what the
Enc_structure is built from. It does not work on its own. The Enc_structure
is [context, protected, external_aad], and the context differs between a
COSE_Encrypt ("Encrypt") and a recipient-less COSE_Encrypt0 ("Encrypt0") — a
distinction a bare material value has no way to record. Caching the protected
header alone therefore needs a companion flag for the envelope form, which
means an extra field here, an extra column in any store that persists this,
and a new way to get it wrong.

Caching the finished AAD sidesteps all of it. encryptStream already computes
the Enc_structure via Envelope.EncStructure, which resolves the context for
whichever form is being written, so the cached bytes are correct for both and
BodyMaterial stays form-agnostic. No flag, no reconstruction logic, and no
new cose export — the bytes go straight to aesstream.Config. Nothing is lost:
the protected header is the structure's second element, still recoverable
from the AAD. The cost is roughly 12 bytes of CBOR framing.

Callers persisting this material should store the AAD rather than the
protected header for the same reason.

Safety
------
A stale or corrupted cache cannot serve wrong data. BaseNonce and AAD are
bound into every chunk's GCM tag, and HeaderLen and ChunkSize decide which
bytes are read and under which nonce, so a drifted value fails with
aesstream.ErrCorrupted rather than emitting plausible plaintext. Validate
catches a partial record up front. The material is entirely non-secret — all
of it is already in the clear at the front of the stored blob — and
deliberately excludes the CEK.

One caveat, documented on DecryptRangeWithMaterial: with no envelope to
consult, the declared chunk-count cross-check that yields ErrSizeMismatch
cannot run, so nothing detects a blobSize that disagrees with the stored
object. A caller that records the blob's size alongside this material should
compare the two before trusting a range.

Encrypt and EncryptWithCEK now return *EncryptedBlob instead of
io.ReadCloser. It still satisfies io.ReadCloser, so ordinary use is
unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CTgvEMV8qaHdKrPswx8bHN
@bajtos
bajtos requested a review from alanshaw August 4, 2026 17:27
bajtos added 6 commits August 4, 2026 19:40
The BodyMaterial path landed in the previous commit without being mentioned
where a reader looks for it. Three places still described a range read as
always fetching the envelope: the package overview's byte-range section, the
RangeReader doc's list of constructors, and the README, which documented
DecryptRange and DecryptRangeWithCEK but not the cached-material variant.

The README gains a section on it, since that is where a caller deciding what
to persist alongside a blob will start. It states the four values, that they
are complete before any plaintext is read, why the AAD is stored rather than
the protected header it contains, and the one check the path gives up
(ErrSizeMismatch has no envelope to consult).

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Assisted-by: Claude:claude-opus-5
BodyMaterial arrived after the range API was written, so it duplicated
logic the envelope path already had. Two copies mattered, because both
underpin the claim that the envelope path and the cached-material path
accept the same inputs and fail the same way:

- the aesstream.Config literal, built field by field in openStream and
  again in spanRangeReader, now bodyParams.streamConfig
- the plaintext-size derivation, in both BodyMaterial.PlaintextSize and
  plaintextSizeFor, now plaintextSizeFrom

bodyParams stays a separate type. Merging it into BodyMaterial would
hand the whole-object path a material with no header length to put in
it: cose.DecodeReader does not report how many bytes the envelope
consumed, and Decrypt has no use for the number. Its doc comment now
records what BodyMaterial adds and why the two shapes differ.

Validate stays off the envelope paths. validateBody checks that the iv
header is present but not that it is BaseNonceSize bytes, so a short iv
keeps failing in the body cipher rather than as ErrIncompleteMaterial.

Internal only: the exported surface is unchanged.

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Assisted-by: Claude:claude-opus-5
ciphertextBytesRead never had a caller, so staticcheck flagged it
(U1000). The two assertions about how much ciphertext a range decrypt
fetched accumulate inside loops that also bounds-check every read
against the reported span, which the helper cannot do, so there was
nowhere for it to be used.

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Assisted-by: Claude:claude-opus-5
decodeHeaderAt grew its probe on every cose.Decode failure except a typ
mismatch, so a blob that is not a FEE envelope was re-read at 4 KiB, 8
KiB, ... up to 1 MiB before returning the error the very first read had
already settled. Against a remote store, a wrong object id cost nine
requests and ~2 MiB of fetches for a purely client-side mistake.

Only a prefix that stopped mid-item can be answered by reading more.
cose.Decode now also wraps io.ErrUnexpectedEOF in that case, so the
probe can tell "read more bytes" from "these bytes are complete and
wrong" and grow only for the former. That subsumes the ErrUnexpectedType
special case rather than adding a second one beside it.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Cleanups from a reuse/simplification review of the branch. No behaviour
change.

- checkCEK replaces four copies of the ErrInvalidCEK length branch, and
  errNilBlob the two independently built "nil blob reader" strings.
- plaintextSizeFor becomes envelopePlaintextSize: it sat beside
  plaintextSizeFrom and differed by one preposition.
- encryptReader folds into EncryptedBlob, its only user.
- Tests share clampLen and headerLenOf instead of spelling the clamp two
  ways and the header length three times, use bytes.Clone/slices.Clone
  and cose.TagCOSEEncrypt0 over the older idioms, and compare BodyMaterial
  in one assertion so a new field cannot go unchecked.
- recordingReaderAt.covered() goes: the assertion it fed was weaker than
  the span check two lines above it.
- The test doubles hold one *bytes.Reader rather than allocating per
  ReadAt; countingBlob is godoc-visible, so callers copy its shape.
- ExampleDecryptRange drops an unreachable empty-range branch, which the
  README handler still covers.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Encrypt and EncryptWithCEK now return (io.ReadCloser, BodyMaterial,
error), and EncryptedBlob with its Material method is gone.

The concrete *EncryptedBlob return made a typed nil reachable: a wrapper
declared as (io.ReadCloser, error) could `return fee.Encrypt(...)`, and
on an error path the interface came back non-nil holding a nil pointer,
so a caller's `if rc != nil { rc.Close() }` panicked. Returning the
interface makes that unconstructible rather than merely documented, and
BodyMaterial is a value type whose zero value is inert.

The material was always complete before any plaintext was read, so
handing it back directly says so where a method plus a doc comment had
to assert it. clone() now runs once at construction instead of per
Material call.

TestEncryptedBlobMaterialIsACopy is replaced by
TestEncryptMaterialDoesNotAliasTheStream, which pins the property that
still matters: mutate the returned material before reading a byte, and
the blob still decrypts under the pristine values. The old test only
compared two Material calls against each other.

BREAKING CHANGE: Encrypt and EncryptWithCEK gained a second result and
no longer return *EncryptedBlob, which is removed along with its
Material method.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Comment thread example_material_test.go Outdated
Comment thread fee.go Outdated
bajtos added 3 commits August 4, 2026 20:58
- name the example's stored size field blobSize, so it reads as the
  blob's size rather than the plaintext's
- keep chunkCountFor above encryptReader to cut diff churn

Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
A plaintext of exactly k*chunkSize bytes has two valid encodings: k
chunks whose last one is full, or k full chunks followed by an empty
final chunk. Both decrypt to the same bytes, aesstream reads either (the
"full + empty final" row in TestChunkLayout pins it), and whole-object
Decrypt accepts either.

envelopePlaintextSize derived the expected chunk count with
chunkCountFor, which models only the first form, so a blob in the second
form declaring k+1 chunks failed with ErrSizeMismatch. DecryptRange and
PlaintextSize were stricter than Decrypt on the same object, which is
not a distinction this API means to draw.

Compare against the count aesstream derives from the ciphertext layout
instead, via a new exported ChunkCount. chunkCountFor stays as the
producer's rule for writing the header, matching the reference
implementation, and its doc now says so.

The check keeps its purpose: a size wrong by a whole chunk or more still
trips ErrSizeMismatch. A size short by only part of the final chunk now
accounts for the same number of chunks and passes, surfacing instead as
an authentication failure when that chunk is read -- the model the docs
already state for blobs carrying no chunk count. The two lengths are
indistinguishable without reading the chunks.

Also adds the exact-multiple boundary to the cross-implementation
vectors, which had no fixture for it: exact-multiple-go is 3 full chunks,
and the pinned foc-encryption decrypts it.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
@bajtos
bajtos marked this pull request as ready for review August 4, 2026 19:15
@bajtos
bajtos requested a lite review from Copilot August 4, 2026 19:15

@bajtos bajtos left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Written by Claude (claude-opus-5) at the repo author's request.

Three findings from a quality review of this branch that were deliberately not fixed here — each is a design decision rather than a cleanup, so they are filed as line comments anchored to the code they concern rather than acted on. None of them is a correctness bug; the range math, CEK lifetimes, header probe and material caching all checked out.

Not human-reviewed line by line.

Comment thread range.go
Comment thread range.go Outdated
if err != nil {
return 0, err
}
body, err := validateBody(env)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Written by Claude (claude-opus-5), same review pass, same reason for leaving it.

PlaintextSize builds the Enc_structure AAD and throws it away. The only field used from body is chunkSize (line 231), but validateBody unconditionally runs protectedBytes() + encMode.Marshal([]any{context, bstr, bstr}) and returns an AAD nothing here reads. This is the documented HEAD / suffix-range path — "reads only the header … and needs no key material" — so a reflection-based CBOR marshal and its allocations run on every HEAD request for a value discarded one line later.

Splitting the AAD build out of validateBody fixes it, since only openStream and newRangeReader construct a cipher. The tension is real though: validateBody is deliberately the one place both paths validate and its doc leans on that. The shared alg/nonce/chunk-size checks would still carry the "both paths accept the same envelopes" property.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ash, do you have an opinion on whether we should fix this now?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I would separate these two operations - validateBody is not just validating...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 46a967b

Comment thread range.go Outdated
func spanRangeReader(blob io.ReaderAt, blobSize, headerLen int64, body bodyParams, plainSize int64, cek []byte, off, length int64) (*RangeReader, error) {
ciphertextSize := blobSize - headerLen

start, n, _, err := aesstream.CiphertextRange(ciphertextSize, body.chunkSize, off, length)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Written by Claude (claude-opus-5), same review pass. Lowest stakes of the three.

An empty range still allocates ~2× chunk size and an AES key schedule. CiphertextRange returns (0, 0, 0) here, but NewSpanReader is still called below and allocates inBuf (chunkSize+16) and outBuf (chunkSize) and expands the AES key before noticing effLen == 0 — ~512 KiB zeroed at the default chunk size, for a reader that reads nothing and returns io.EOF. This is the 416 shape the README handler drives (if r.Len() == 0).

The value needed to short-circuit is already being discarded on this line: the third result is plainLen, dropped as _, while Len() delegates to sr.Len(). Keeping it on RangeReader (it is the authoritative pre-read Content-Length anyway) and leaving sr nil when n == 0 with Read returning io.EOF is small. Allocating inBuf/outBuf lazily in aesstream.nextSpanChunk fixes it a level down and helps aesstream callers directly.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ash, do you have an opinion on whether we should fix this now?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ Sounds easy enough to guard against so yeah I'd perhaps consider it. I don't think callers will often ask for an empty range...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 5cbd1e2

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.

Suppressed comments (1)

range.go:362

  • When the envelope header exceeds the 1 MiB probe limit (maxHeaderLen) but the blob is larger, decodeHeaderAt returns an error that wraps io.ErrUnexpectedEOF, which looks like a truncated blob. This makes it hard for callers/logs to distinguish "header too large (rejected by policy)" from a genuinely truncated envelope. Consider returning a dedicated error path when probe hit maxHeaderLen but blobSize indicates more bytes exist, and avoid wrapping io.ErrUnexpectedEOF in that case.
		if !errors.Is(derr, io.ErrUnexpectedEOF) || atEnd || probe >= limit {
			return nil, 0, fmt.Errorf("fee: decoding envelope: %w", derr)
		}

The merge left two overlaps in vectors/README.md.

The producer's chunk-count formula was stated twice, once on the body
cipher bullet and once on the chunking bullet added for the trailing
empty final chunk. Keep it on the body cipher bullet and narrow the other
to what it alone says: a decoder must not re-derive the count that way,
because a stream ending in an empty final chunk holds one more chunk than
the formula gives for the same plaintext.

The framing-case count said three; exact-multiple-go makes four. While
correcting it, the "in both directions" claim turned out to cover only
two of the cases: single-chunk-go and exact-multiple-go run Go to TS
only, so the sentence now names the partial final chunk and the empty
file rather than claiming every case.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Comment thread material.go Outdated
// incorrect one.
//
// The zero value is not usable; see [BodyMaterial.Validate].
type BodyMaterial struct {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure "material" is the best naming here. Material normally refers to key material and is secret, yet this is not secret.

I'd suggest something more like BodyParams or BodyDescriptor although noting that the unexported type bodyParams already exists without the header length field, so might be confusing to have both.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to BodyDescriptor in 9f740aa

Comment thread range.go Outdated
// cek must be 32 bytes (AES-256). The caller retains ownership: it is copied into
// the body cipher but neither retained nor wiped. off and length behave exactly
// as in [DecryptRange].
func DecryptRangeWithMaterial(blob io.ReaderAt, blobSize int64, m BodyMaterial, cek []byte, off, length int64) (*RangeReader, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about instead of defining a whole new function, adding options to DecryptRangeWithCEK that allows the "material" to be provided, rather than decoded from the blob?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in eeff7ba

Comment thread range.go
Comment thread range.go Outdated
func spanRangeReader(blob io.ReaderAt, blobSize, headerLen int64, body bodyParams, plainSize int64, cek []byte, off, length int64) (*RangeReader, error) {
ciphertextSize := blobSize - headerLen

start, n, _, err := aesstream.CiphertextRange(ciphertextSize, body.chunkSize, off, length)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ Sounds easy enough to guard against so yeah I'd perhaps consider it. I don't think callers will often ask for an empty range...

Comment thread range.go Outdated
if err != nil {
return 0, err
}
body, err := validateBody(env)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I would separate these two operations - validateBody is not just validating...

Short-circuit zero-length ranges before building the span reader or
allocating chunk-sized buffers. This keeps empty HTTP-style range
requests cheap in both fee and aesstream.

Assisted-by: Copilot:gpt-5.4
bajtos added 3 commits August 10, 2026 15:36
Use descriptor terminology for the cached, non-secret envelope\nmetadata returned by Encrypt and consumed by range decryption.\nThis matches the review feedback and avoids implying that the\nvalue carries key material.\n\nAssisted-by: GPT-5.4:gpt-5.4
Make the external-CEK range API cover both envelope-decoding
and cached-descriptor reads through one entry point.

This keeps the range reader behavior, examples, and docs aligned
while preserving the no-header-read path for cached metadata.

Assisted-by: Copilot:gpt-5.4
Keep header-only validation separate from AAD reconstruction so
PlaintextSize avoids building Enc_structure bytes it does not use.

Return validated body parameters and rebuilt AAD separately in the
body-cipher paths, and split the cached descriptor accessors to match.

Assisted-by: Copilot:gpt-5.4
@bajtos
bajtos requested review from frrist and a balanced review from Copilot August 10, 2026 14:53
@bajtos
bajtos requested a review from alanshaw August 10, 2026 14:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.

Suppressed comments (2)

descriptor.go:18

  • The PR description documents this feature under the names BodyMaterial, DecryptRangeWithMaterial(blob, blobSize, m, cek, off, length), and ErrIncompleteMaterial, but the code implements them as BodyDescriptor, the optional desc *BodyDescriptor argument of DecryptRangeWithCEK, and ErrIncompleteDescriptor. The code is internally consistent; the description appears to predate the rename. Consider updating the description so the permanent PR record matches the shipped API surface.
// BodyDescriptor is everything a range decrypt needs from a FEE envelope, so a
// caller that cached it can serve a byte range without fetching or decoding the
// envelope header at all. It is returned by [Encrypt] / [EncryptWithCEK] at
// encryption time and consumed by [DecryptRangeWithCEK] as its desc argument.

descriptor.go:69

  • Minor/optional: the BodyDescriptor methods use the receiver name m (a leftover from the type's earlier BodyMaterial name). Elsewhere in the package receivers are derived from the type name — e.g. r for RangeReader (range.go:65), b for bodyParams (fee.go:553), e for encryptReader (fee.go:438), u for the unwrappers (unwrapper.go:44). Renaming the receiver to d across the file would keep this consistent with the established convention.
func (m BodyDescriptor) Validate() error {

@bajtos
bajtos requested a review from hannahhoward August 10, 2026 15:19
@bajtos bajtos self-assigned this Aug 18, 2026

@alanshaw alanshaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can we rename the error? Otherwise GTG.

Comment thread descriptor.go Outdated
Assisted-by: Claude:claude-fable-5
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>"
@bajtos
bajtos merged commit 72a3a35 into main Aug 18, 2026
7 checks passed
@bajtos
bajtos deleted the fil-570-stream-api branch August 18, 2026 17:33
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.

4 participants