refactor(lib): trim verbose doc comments across dvcs/gitsmart, dvcs/gitssh, lib/repo, lib/issues

Modified dvcs/gitsmart/handler/smart.rs +0 −5 View dvcs/gitsmart/handler/smart.rs

@@ -83,11 +83,6 @@

8383 let bare = state.bare_path(&resolved);

8484 let is_receive = service == GitService::ReceivePack;

8585 if is_receive {

86 // Reject the receive-pack payload BEFORE letting git touch any

87 // refs. The body carries pkt-lines, the first non-flush of which

88 // is "<old-oid> <new-oid> <refname>\0" — we extract those and

89 // evaluate the protection policy. Anything malformed is left to

90 // git which will surface its own protocol error.

9186 if let Some(actor_uid) = actor.user_id {

9287 let updates = parse_receive_pack_refs(&input);

9388 if !updates.is_empty() {

Modified dvcs/gitsmart/pre_receive.rs +0 −53 View dvcs/gitsmart/pre_receive.rs

@@ -1,14 +1,3 @@

1//! Pre-receive validation for git receive-pack.

2//!

3//! This module gates the actual `git receive-pack` invocation. It enforces

4//! branch and tag protection policies before any ref is mutated, so an

5//! attacker cannot `git push --force` past a protected ref or delete a

6//! protected branch.

7//!

8//! The validator is transport-agnostic: both the HTTP `gitsmart` path and

9//! the SSH `gitssh` path call [`validate_pre_receive`] before spawning the

10//! git process.

11

121use db::conn::sea_orm::{

132 ColumnTrait, DatabaseConnection, EntityTrait, QueryFilter,

143};

@@ -21,14 +10,12 @@

2110

2211use crate::error::{SmartError, SmartResult};

2312

24/// Per-update verdict emitted by [`validate_pre_receive`].

2513#[derive(Debug, Clone, PartialEq, Eq)]

2614pub enum RefVerdict {

2715 Accept,

2816 Reject(String),

2917}

3018

31/// Outcome of [`validate_pre_receive`].

3219#[derive(Debug, Clone)]

3320pub struct PreReceiveVerdict {

3421 pub per_ref: Vec<(RefUpdate, RefVerdict)>,

@@ -46,38 +33,19 @@

4633 }

4734}

4835

49/// Validate every ref-update against the repo's protection policy.

50///

51/// Rules applied (H6 — branch / tag protection at push time):

52/// - Branch deletion (`new_oid` is the zero OID): rejected for any branch

53/// that has a matching protection row. The DB schema has no

54/// `enable_delete` field, so deletion is always refused under protection.

55/// - Branch update where `enable_push = false`: rejected.

56/// - Branch force-push (non-fast-forward, i.e. `old_oid != OID::zero` and

57/// `new_oid != OID::zero` and not a fast-forward): rejected unless the

58/// matching policy has `enable_force_push = true`.

59/// - Tag update matching a `tag_protection_policy.name_pattern`: rejected

60/// unless the actor's `user_id` is in the policy's

6136/// `allowlist_user_ids` CSV.

62/// - Branches without a matching protection row: accepted.

63///

64/// The caller (transport) is responsible for ensuring the actor has at

65/// least `RepoAccess::Write` for the repo before calling this.

6637pub async fn validate_pre_receive(

6738 db: &DatabaseConnection,

6839 repo_id: Uuid,

6940 actor_user_id: Uuid,

7041 refs: &[RefUpdate],

7142) -> SmartResult<PreReceiveVerdict> {

72 // Confirm the repo exists; protects against silent typos in the path.

7343 let _repo = repo_entity::Entity::find_by_id(repo_id)

7444 .one(db)

7545 .await

7646 .map_err(|e| SmartError::Internal(format!("db: {e}")))?

7747 .ok_or_else(|| SmartError::NotFound(format!("repo {repo_id}")))?;

7848

79 // Pre-fetch protection rows once (full set scoped to the repo) and the

80 // actor's collaboration mode for `is_admin_endpoint`-style checks.

8149 let branch_policies = policy_entity::Entity::find()

8250 .filter(policy_entity::Column::RepoId.eq(repo_id))

8351 .all(db)

@@ -130,9 +98,6 @@

13098 if let Some(tag) = r.refname.strip_prefix("refs/tags/") {

13199 return judge_tag(tag, tag_policies, actor_user_id);

132100 }

133 // Non-branch / non-tag refs (notes, etc.) are not protected by branch /

134 // tag policies. Allow them to pass; transport layers may add their own

135 // gates for these.

136101 RefVerdict::Accept

137102}

138103

@@ -170,8 +135,6 @@

170135 }

171136

172137 if !policy.enable_push {

173 // Even admin collaborators can't bypass an `enable_push = false`

174 // rule — the row's intent is "no pushes".

175138 return RefVerdict::Reject(format!(

176139 "pushes to protected branch `{}` are not allowed",

177140 branch

@@ -213,26 +176,10 @@

213176 RefVerdict::Accept

214177}

215178

216/// Cheap fast-forward heuristic: parent-of-of-of-of is true when `new_oid`

217/// is the same OID as `old_oid` (no-op ref update) or when the object id

218/// string is `0{40}` (delete). True non-FF detection would require walking

219/// the commit graph; for H6 we treat all non-equal updates as potential

220/// force-pushes and let the `enable_force_push` flag decide.

221///

222/// The planner's intent (H6 plan, section 6.2): reject non-FF updates

223/// unless `enable_force_push = true`. Refining fast-forward detection is

224/// tracked as a follow-up but is not required to close H6.

225179fn is_fast_forward(old: &str, new: &str) -> bool {

226180 old == new || old == "0".repeat(40) || new == "0".repeat(40)

227181}

228182

229/// Parse the leading ref-update lines from a receive-pack pkt-line stream.

230///

231/// Format (per pkt-line): `<old-oid SP> <new-oid SP> <refname> NUL`,

232/// optionally followed by a capability list on the first line. Anything

233/// that doesn't match the schema is silently skipped — the worst case is

234/// we let git see a payload we'd have rejected; the best case is we

235/// caught a force-push early.

236183pub fn parse_receive_pack_refs(input: &[u8]) -> Vec<RefUpdate> {

237184 let lines = match crate::protocol::pktline::decode_stream(input) {

238185 Ok(lines) => lines,

Modified dvcs/gitssh/handler/channel.rs +0 −10 View dvcs/gitssh/handler/channel.rs

@@ -5,10 +5,6 @@

55

66use crate::handler::GitSshHandler;

77

8/// Per-channel buffering cap for a single SSH channel. Matches the HTTP-side

9/// `MAX_RPC_BODY` cap (`dvcs/gitsmart/handler/smart.rs`) so a malicious client

10/// cannot exhaust server memory by streaming bytes into a channel before

11/// EOF triggers the git spawn.

128pub const MAX_SSH_CHANNEL_BUF: usize = 1 << 30;

139

1410impl GitSshHandler {

@@ -26,8 +22,6 @@

2622 limit = MAX_SSH_CHANNEL_BUF,

2723 "ssh channel buffer overflow; dropping channel"

2824 );

29 // Mark the channel as poisoned so the EOF handler refuses to

30 // spawn git, then ask russh to close the channel.

3125 state.buffer_overflow = true;

3226 return Err(russh::Error::Disconnect);

3327 }

@@ -67,7 +61,6 @@

6761 channel = ?channel,

6862 "refusing to spawn git for ssh channel that exceeded buffer cap"

6963 );

70 // Surface the rejection to the client as a non-zero exit.

7164 let _ = handle.exit_status_request(channel, 1).await;

7265 let _ = handle.eof(channel).await;

7366 let _ = handle.close(channel).await;

@@ -80,9 +73,6 @@

8073 Some(crate::handler::git_op::GitOperation::ReceivePack { .. })

8174 );

8275

83 // H6: enforce branch / tag protection BEFORE spawning git. The

84 // buffer here is the same pkt-line stream `gitsmart::rpc` would

85 // receive; we reuse the same parser.

8676 if is_receive

8777 && let (Some(repo_id), Some(actor_uid)) = (repo_id, actor_id)

8878 {

Modified dvcs/gitssh/handler/mod.rs +1 −5 View dvcs/gitssh/handler/mod.rs

@@ -36,11 +36,7 @@

3636 pub session_permit: Option<OwnedSemaphorePermit>,

3737 pub operator: Option<users::Model>,

3838 pub channels: HashMap<ChannelId, ChannelState>,

39 /// True when this handler was admitted by `try_acquire_owned` returning

40 /// `None` — i.e. the SSH session limit is already saturated. The handler

41 /// rejects every authentication attempt so the client disconnects

42 /// quickly without consuming a real session slot.

43 pub over_capacity: bool,

39 pub over_capacity: bool,

4440}

4541

4642impl GitSshHandler {

Modified lib/issues/features/authz.rs +0 −10 View lib/issues/features/authz.rs

@@ -1,12 +1,5 @@

11//! Authorization helpers for the issues crate.

2//!

32//! `authorize_issue` ensures the caller can mutate a issue belonging to a given

4//! channel. The check walks `channel -> space -> active member`, mirroring the

5//! pattern used by `lib/repo/features/authz.rs`. Site admins bypass the check.

6//!

7//! Space-less channels (top-level channels where `channel.space_id IS NULL`)

8//! only accept the channel owner (`channels.created_by`) and site admins.

9

103use db::channel::channels as channel_entity;

114use db::conn::sea_orm::*;

125use db::space::space_members as member_entity;

@@ -44,11 +37,8 @@

4437 Ok(member.status == member_entity::MemberStatus::Active)

4538}

4639

47/// Verify the caller may mutate a issue that belongs to `channel_id`.

48///

4940/// - Site admins: always allowed.

5041/// - Channel owner (top-level / space-less channel): allowed.

51/// - Channel with `space_id`: caller must be an active member of that space.

5242pub async fn authorize_issue(

5343 svc: &IssueService,

5444 actor: Uuid,

Modified lib/repo/features/authz.rs +0 −15 View lib/repo/features/authz.rs

@@ -260,8 +260,6 @@

260260 }

261261}

262262

263/// A single ref-update line from a git receive-pack payload, parsed from the

264/// pkt-line stream (`<old-oid> <new-oid> <refname>\0`).

265263#[derive(Debug, Clone, PartialEq, Eq)]

266264pub struct RefUpdate {

267265 pub old_oid: String,

@@ -269,11 +267,6 @@

269267 pub refname: String,

270268}

271269

272/// `true` if `name` matches `pattern` using a tiny fnmatch-style glob:

273/// `*` matches any (possibly empty) run of characters; `?` matches one

274/// character; everything else is a literal byte. The implementation is

275/// linear-time and zero-allocation, sufficient for branch-name matching

276/// against user-supplied patterns.

277270pub fn glob_match(pattern: &str, name: &str) -> bool {

278271 let pat: Vec<char> = pattern.chars().collect();

279272 let txt: Vec<char> = name.chars().collect();

@@ -281,7 +274,6 @@

281274 match (pat.first(), txt.first()) {

282275 (None, None) => true,

283276 (Some('*'), _) => {

284 // Try matching zero chars, then one more each step.

285277 rec(&pat[1..], txt)

286278 || (!txt.is_empty() && rec(pat, &txt[1..]))

287279 }

@@ -293,10 +285,6 @@

293285 rec(&pat, &txt)

294286}

295287

296/// Resolve the active branch-protection policy for a `(repo_id, branch_name)`

297/// pair. Matching order:

298/// 1. Exact `branch_name` match.

299/// 2. `branch_pattern` glob (any row whose non-null `branch_pattern` matches).

300288pub async fn find_protection_for_branch(

301289 db: &DatabaseConnection,

302290 repo_id: Uuid,

@@ -304,7 +292,6 @@

304292) -> RepoResult<Option<db::repo::repo_branch_protection_policy::Model>> {

305293 use db::repo::repo_branch_protection_policy as policy_entity;

306294

307 // Step 1: exact match.

308295 let exact = policy_entity::Entity::find()

309296 .filter(policy_entity::Column::RepoId.eq(repo_id))

310297 .filter(policy_entity::Column::BranchName.eq(branch_name))

@@ -314,8 +301,6 @@

314301 return Ok(exact);

315302 }

316303

317 // Step 2: pattern fallback. We fetch pattern rows and match in-process

318 // because glob semantics are application-defined (not stored in the DB).

319304 let pattern_rows = policy_entity::Entity::find()

320305 .filter(policy_entity::Column::RepoId.eq(repo_id))

321306 .filter(policy_entity::Column::BranchPattern.is_not_null())