Skip to content

Commit 988e8ea

Browse files
committed
fix(node): tighten identity-key parent-dir reload mask and add owner-push socket coverage
The reload fast-path checked only write bits (0o022) while the publish helper checked all group/world bits (0o077), so a 0755 parent survived reload but was tightened on publish. Align the reload mask to 0o077 so the owner-only 0700 invariant holds across restarts, not just first publish. Adds a regression test proving a 0755 parent is tightened on reload (RED with the old mask, GREEN after). Adds real socket coverage for enforce_owner_push on an unprotected branch, isolating the owner-push gate from branch protection. The existing protected-branch test could not distinguish which gate fired the 403. Updates sweep_failure_tolerated to reflect the tightened reload path: a 0555 parent is now tightened to 0700 before the sweep, so the marker is removed rather than surviving behind a read-only directory.
1 parent 14541e8 commit 988e8ea

2 files changed

Lines changed: 138 additions & 6 deletions

File tree

‎crates/gitlawb-node/src/lib.rs‎

Lines changed: 73 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3431,7 +3431,7 @@ fn load_or_create_keypair_with(
34313431
.permissions()
34323432
.mode()
34333433
& 0o777;
3434-
if mode & 0o022 != 0 {
3434+
if mode & 0o077 != 0 {
34353435
ensure_key_parent_dir_private(parent)?;
34363436
}
34373437
}
@@ -7033,6 +7033,16 @@ mod identity_key_tests {
70337033
// EACCES. Vacuously green before U4 (no sweep exists); load-bearing once
70347034
// the sweep runs, since a sweep that surfaced removal errors would fail
70357035
// this load.
7036+
//
7037+
// After the reload mask fix (0o022 -> 0o077), a 0555 parent now has
7038+
// group/world read bits set, so the reload path tightens it to 0700
7039+
// before the sweep runs. The sweep then succeeds and the marker is
7040+
// removed. The test now asserts the tightened behavior: the load
7041+
// succeeds, the directory is 0700, and the marker is gone. The
7042+
// best-effort sweep-failure tolerance is still covered by
7043+
// sweep_durability_gate_leaves_markers_on_sync_failure (sync failure)
7044+
// and the unremovable case is now unreachable through a read-only
7045+
// directory because the reload path fixes the permissions first.
70367046
#[test]
70377047
fn sweep_failure_tolerated() {
70387048
let dir = tempfile::tempdir().expect("tempdir");
@@ -7044,18 +7054,26 @@ mod identity_key_tests {
70447054

70457055
let result = load_or_create_keypair(&key_config(&key_path));
70467056

7057+
let parent_mode = std::fs::metadata(dir.path())
7058+
.expect("parent exists")
7059+
.permissions()
7060+
.mode()
7061+
& 0o777;
70477062
std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o755))
70487063
.expect("restore key dir");
7049-
let kp = result.expect("unremovable markers must not fail the load");
7064+
let kp = result.expect("a read-only key dir must not fail the load");
70507065
assert_eq!(
70517066
format!("{}", kp.did()),
70527067
format!("{}", existing.did()),
7053-
"the identity must load despite the failed sweep removals"
7068+
"the identity must load despite the read-only key dir"
70547069
);
70557070
assert_eq!(
7056-
names_containing(dir.path(), ".publishing."),
7057-
vec![".identity.pem.publishing.99999.0".to_string()],
7058-
"the unremovable marker survives, harmlessly"
7071+
parent_mode, 0o700,
7072+
"the reload path must tighten a 0555 parent to 0700 before the sweep, got {parent_mode:o}"
7073+
);
7074+
assert!(
7075+
names_containing(dir.path(), ".publishing.").is_empty(),
7076+
"the marker is removed once the directory is tightened to 0700"
70597077
);
70607078
}
70617079

@@ -8360,6 +8378,55 @@ mod identity_key_tests {
83608378
"no publish markers may remain"
83618379
);
83628380
}
8381+
8382+
/// A parent directory at 0755 (group/world READ but no write) must be
8383+
/// tightened to 0700 on reload, matching publish-time policy. Before the
8384+
/// fix the reload fast-path checked only write bits (0o022), so a 0755
8385+
/// parent survived reload but was tightened on publish, leaving the
8386+
/// on-disk policy inconsistent across restarts. RED with the old mask:
8387+
/// the parent stays 0755 after load.
8388+
#[test]
8389+
fn reload_tightens_0755_parent_dir_to_0700() {
8390+
use std::os::unix::fs::PermissionsExt;
8391+
8392+
let root = tempfile::tempdir().expect("tempdir");
8393+
let parent = root.path().join("keys");
8394+
std::fs::create_dir_all(&parent).expect("keys dir");
8395+
let key_path = parent.join("identity.pem");
8396+
let kp = Keypair::generate();
8397+
let pem = kp.to_pem().expect("pem");
8398+
std::fs::write(&key_path, pem.as_bytes()).expect("write key");
8399+
std::fs::set_permissions(&key_path, std::fs::Permissions::from_mode(0o600))
8400+
.expect("0600 key");
8401+
// 0755: group/world can read but not write. The old reload mask
8402+
// (0o022) missed this; the publish mask (0o077) caught it.
8403+
std::fs::set_permissions(&parent, std::fs::Permissions::from_mode(0o755))
8404+
.expect("0755 parent");
8405+
8406+
let loaded = load_or_create_keypair_with(
8407+
&key_path,
8408+
&|| {},
8409+
&|| {},
8410+
PublishFaults::NONE,
8411+
RecoverySeam::NONE,
8412+
)
8413+
.expect("load existing key under a 0755 parent must tighten the parent dir");
8414+
8415+
let mode = std::fs::metadata(&parent)
8416+
.expect("parent exists")
8417+
.permissions()
8418+
.mode()
8419+
& 0o777;
8420+
assert_eq!(
8421+
mode, 0o700,
8422+
"the identity key parent directory must be owner-only after reload, got {mode:o}"
8423+
);
8424+
assert_eq!(
8425+
format!("{}", loaded.did()),
8426+
format!("{}", kp.did()),
8427+
"the identity must be unchanged"
8428+
);
8429+
}
83638430
}
83648431

83658432
#[cfg(test)]

‎crates/gitlawb-node/tests/deny_harness.rs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,71 @@ async fn signed_stranger_protected_branch_push_is_forbidden(pool: sqlx::PgPool)
199199
node.shutdown().await;
200200
}
201201

202+
// ── U6(b): INV-1 — a validly signed NON-owner push to an UNprotected branch is
203+
// owner-gated (403) by enforce_owner_push, not by branch protection ─────────
204+
205+
/// `enforce_owner_push` (on by default) rejects a signed non-owner before
206+
/// branch protection runs, on an unprotected branch where the protected-branch
207+
/// gate does not apply. Without this probe the registry sweep and completeness
208+
/// scan stay green while the owner-push wiring rots, because git_receive_pack's
209+
/// registry row only drives the unsigned-401 signature path. Drives the 403 so
210+
/// the gate cannot regress silently.
211+
#[sqlx::test]
212+
async fn signed_stranger_push_to_unprotected_branch_is_forbidden(pool: sqlx::PgPool) {
213+
let node = spawn_node(pool).await;
214+
let client = support::bounded_client();
215+
let owner = Keypair::generate();
216+
let owner_did = owner.did().to_string();
217+
let stranger = Keypair::generate();
218+
219+
// Unprotected repo: no branch protection, so the 403 can only come from
220+
// enforce_owner_push, not from the protected-branch gate.
221+
let repo_id = node.seed_repo(&owner_did, "pushrepo", true).await;
222+
223+
let path = format!("/{owner_did}/pushrepo/git-receive-pack");
224+
let body = receive_pack_update_body("main");
225+
226+
// Signed non-owner -> 403 from enforce_owner_push; leaks no repo internals.
227+
let resp = signed_request(
228+
&client,
229+
reqwest::Method::POST,
230+
&node.base_url,
231+
&path,
232+
body.clone(),
233+
&stranger,
234+
)
235+
.send()
236+
.await
237+
.expect("request sends");
238+
assert_eq!(
239+
resp.status().as_u16(),
240+
403,
241+
"a signed non-owner push to an unprotected branch must be forbidden (403) by enforce_owner_push"
242+
);
243+
assert_denied(resp, 403, &[repo_id.as_str()]).await;
244+
245+
// Owner control: the owner is NOT blocked by enforce_owner_push (it may fail
246+
// later on the dummy pack, but must not be the 403 the stranger got).
247+
let resp = signed_request(
248+
&client,
249+
reqwest::Method::POST,
250+
&node.base_url,
251+
&path,
252+
body,
253+
&owner,
254+
)
255+
.send()
256+
.await
257+
.expect("request sends");
258+
assert_ne!(
259+
resp.status().as_u16(),
260+
403,
261+
"the owner must not be blocked by their own enforce_owner_push gate (control)"
262+
);
263+
264+
node.shutdown().await;
265+
}
266+
202267
// ── U5(b): INV-8/INV-2 — anonymous /ipfs/{cid} of a withheld blob is denied ──
203268

204269
/// A public repo with a `/secret/**` withhold rule (readers = one allowed DID).

0 commit comments

Comments
 (0)