Skip to content

Remove ineffective arbitrary-want setting - #85

Open
behinddwalls wants to merge 2 commits into
tobi:mainfrom
behinddwalls:preetam/issue-77-arbitrary-wants
Open

behinddwalls wants to merge 2 commits into
tobi:mainfrom
behinddwalls:preetam/issue-77-arbitrary-wants

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Sep 30, 2026 •

Copy link
Copy Markdown

Summary

Why?

Partial-clone lazy fetches require direct object wants, while the configuration claimed they could be disabled even though both upload-pack engines always accepted them.

What?

Remove the inert git.allow_any_sha1_in_want option and update tests and integrity documentation to describe arbitrary object wants as part of partial-clone support.

Test Plan

✅ cargo test -p walgit-server --test e2e partial_clone_tree_zero_and_depth_with_filter -- --exact
✅ cargo fmt --all -- --check
✅ git diff --check

Issue

Closes #77

## Summary

### Why?

Partial-clone lazy fetches require direct object wants, while the configuration claimed they could be disabled even though both upload-pack engines always accepted them.

### What?

Remove the inert `git.allow_any_sha1_in_want` option and update tests and integrity documentation to describe arbitrary object wants as part of partial-clone support.

## Test Plan

⚠️ Rust tests were not run because `cargo` is unavailable in this environment; `git diff --check` passes.

## Issue

Closes tobi#77
@0bserver07

Copy link
Copy Markdown
Contributor

CI is waiting on approval for this stack, so I ran the whole thing locally on top of main. Tests, e2e and the sims all pass. Clippy fails on one lint that comes from #86 (manual_let_else in middleware.rs), so that one would go red in CI.

#85, #87, #89, #91 and #92 look good to me and could each go in on their own if rebased on main. #87 needs that rebase anyway since its check sits in the middle of #86's lines. For this one it might be worth a line in the docs saying read access already lets anyone fetch any stored object by id, unreachable ones included, so nobody reads the removal as dropping a protection.

Clarify that repository read access already permits fetching stored objects by ID, including unreachable objects, so removing the ineffective configuration switch does not widen authorization.
@behinddwalls

Copy link
Copy Markdown
Author

Added the clarification in b0b18b1 that a principal with repository read access could already fetch any stored object by ID, including objects unreachable from advertised refs. Removing git.allow_any_sha1_in_want therefore does not widen authorization; it only removes a non-functional configuration switch. The targeted partial-clone test passes.

[addressed by agent]

@behinddwalls

Copy link
Copy Markdown
Author

Reworked the reviewed branches as independent changes on main: #87 is now 77ade75, #89 is 129fcc2, #91 is a200384, and #92 is ef1120d; #85 was already based on main at b0b18b1. I also removed the stale stack sections from all five PR descriptions. Each rewritten branch now contains only its own change, and its targeted test plus formatting/diff checks pass. #86, #88, and #90 were left untouched.

[addressed by agent]

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

git.allow_any_sha1_in_want=false does not block arbitrary object wants

2 participants