Skip to content

feat(sdk): add WithRequireUnprefixedManifestName to reject 0.manifest.json - #4051

Open
pflynn-virtru wants to merge 1 commit into
fix/3513-manifest-json-reader-tolerancefrom
feat/3513-require-spec-manifest-name
Open

pflynn-virtru wants to merge 1 commit into
fix/3513-manifest-json-reader-tolerancefrom
feat/3513-require-spec-manifest-name

Conversation

@pflynn-virtru

@pflynn-virtru pflynn-virtru commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

Proposed Changes

Adds WithRequireUnprefixedManifestName(), a TDFReaderOption that rejects an archive whose manifest is stored only under the legacy name 0.manifest.json. Off by default; no existing reader changes behavior.

r, err := sdk.LoadTDF(f, sdk.WithRequireUnprefixedManifestName())
// errors.Is(err, sdk.ErrPrefixedManifestName) for a 0.manifest.json archive
  • Readers accept both manifest.json and 0.manifest.json by default (fix(sdk): read the TDF manifest under the spec name manifest.json #4063). This option lets a caller accept only the name the spec defines.
  • The legacy entry is still looked up in strict mode, so a misnamed manifest reports ErrPrefixedManifestName while a missing one still reports a missing entry.
  • WithSchemaValidation(Strict) can't do this: schema validation inspects the manifest document, and the zip entry name never reaches the validator. A separate option keeps Strict schema-only.

This PR no longer stacks on #4049; it targets main directly. Names follow the format-describing convention of WithHexlessPolicyBinding (#4232) and WithUnprefixedManifestName (#4049).

Refs #3513.

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation (godoc)

Testing Instructions

  • cd sdk && go test ./... -race
  • golangci-lint run --new-from-rev=origin/main ./... in sdk: 0 new issues.
  • Tests: TestManifest_RequireUnprefixedName_{RejectsPrefixedName,ReadsUnprefixedName,MissingEntirely} (zipstream) and TDFSuite.Test_LoadTDF_RequireUnprefixedManifestName (end to end; builds the manifest.json archive by repacking, so it doesn't depend on any writer option).

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 375a94f8-ac8d-432a-aeeb-8ef90821ff4a
📥 Commits

Reviewing files that changed from the base of the PR and between 4ec7aa7 and 619dd34.

📒 Files selected for processing: 6
  • sdk/internal/zipstream/tdf3_reader.go
  • sdk/internal/zipstream/tdf3_reader_test.go
  • sdk/tdf.go
  • sdk/tdf_config.go
  • sdk/tdf_test.go
  • sdk/tdferrors.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added comp:sdk A software development kit, including library, for client applications and inter-service communicati size/s labels Sep 15, 2026
@pflynn-virtru
pflynn-virtru added this pull request to stack #4052 September 15, 2026 18:18
@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 254.304574ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 147.853125ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 423.768449ms
Throughput 235.98 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m2.360057351s
Average Latency 622.02688ms
Throughput 80.18 requests/second

@pflynn-virtru pflynn-virtru changed the title feat(sdk): add WithRequireSpecManifestName to reject off-spec archives feat(sdk): strict WithRequireSpecManifestName to reject off-spec Sep 16, 2026
@pflynn-virtru pflynn-virtru changed the title feat(sdk): strict WithRequireSpecManifestName to reject off-spec feat(sdk): strict WithRequireSpecManifestName to reject non-aligned Sep 30, 2026
….json

Readers accept both manifest.json and 0.manifest.json so archives written by
earlier releases keep working. A caller that wants to accept only the unprefixed
entry name had no way to say so.

WithRequireUnprefixedManifestName drops the fallback: an archive whose manifest
is filed under 0.manifest.json fails with ErrPrefixedManifestName instead of
being read. The prefixed entry is still looked up in that mode, so an archive
with a misnamed manifest is reported differently from one with no manifest at all.

This is deliberately separate from WithSchemaValidation rather than folded
into Strict. Schema validation inspects the manifest document; the entry name
is archive layout and never reaches the validator, so no schema setting can
express this. Overloading Strict would also start rejecting archives that pass
the schema, changing behaviour for callers already using it.

Off by default, so this adds no new failure mode to existing readers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>
@pflynn-virtru
pflynn-virtru force-pushed the feat/3513-require-spec-manifest-name branch from 35b64b9 to 619dd34 Compare October 19, 2026 22:58
@pflynn-virtru pflynn-virtru changed the title feat(sdk): strict WithRequireSpecManifestName to reject non-aligned feat(sdk): add WithRequireUnprefixedManifestName to reject 0.manifest.json Oct 19, 2026
@pflynn-virtru
pflynn-virtru marked this pull request as ready for review October 19, 2026 22:58
@pflynn-virtru
pflynn-virtru requested review from a team as code owners October 19, 2026 22:58
@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 459.782163ms
Throughput 217.49 requests/second

TDF3 Benchmark Results

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 49.156693546s
Average Latency 490.666443ms
Throughput 101.72 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • otdfctl
  • sdk
  • service
  • lib/fixtures
  • tests-bdd

See the workflow run for details.

This branch has not been deployed

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

Labels

comp:sdk A software development kit, including library, for client applications and inter-service communicati size/s

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant