refactor(lib): trim CWE-fix follow-up comments and unused helpers across auth, webhook, gitauth, gitssh

Modified dvcs/gitauth/service.rs +2 −15 View dvcs/gitauth/service.rs

@@ -93,8 +93,6 @@

9393 required: RepoAccess,

9494 ) -> GitAuthResult<ResolvedRepo> {

9595 let result: GitAuthResult<ResolvedRepo> = async {

96 // A scoped machine credential (access key) caps what this actor may

97 // do even when the account itself would be entitled to more.

9896 if let Some(max) = actor.credential_max

9997 && !max.at_least(required)

10098 {

@@ -119,12 +117,7 @@

119117 result

120118 }

121119

122 /// Verify a credential for the account looked up by username.

123 ///

124 /// Returns the effective `user_id` (always the account's own uid — a

125 /// credential can never authenticate as a different account) plus, when the

126 /// credential is a scoped access key, the access ceiling it grants.

127 async fn verify_credentials(

120 async fn verify_credentials(

128121 &self,

129122 user: &users::Model,

130123 password: &str,

@@ -146,12 +139,7 @@

146139 }

147140 }

148141

149 /// Match `presented` against one of `user`'s access keys.

150 ///

151 /// Only keys owned by `user` are considered, so a key minted for another

152 /// account can never satisfy this username. The key row is returned so its

153 /// scope can cap the actor's access.

154 async fn verify_access_key(

142 async fn verify_access_key(

155143 &self,

156144 user: &users::Model,

157145 presented: &str,

@@ -167,7 +155,6 @@

167155 }

168156}

169157

170/// Map an access-key scope to the maximum git access it may grant.

171158fn access_key_max(scope: user_access_keys::AccessKeyScope) -> RepoAccess {

172159 use user_access_keys::AccessKeyScope as S;

173160 match scope {

Modified dvcs/gitssh/handler/command.rs +0 −5 View dvcs/gitssh/handler/command.rs

@@ -44,11 +44,6 @@

4444 };

4545 service_label = service.as_subcommand();

4646

47 // Build the actor with no pre-granted repo scopes so that

48 // `repo::permission::effective_access` evaluates the real

49 // space_role / collaborator / is_public checks instead of

50 // short-circuiting on an unconditional RepoRead scope (C3:

51 // any SSH user could clone any private repo).

5247 let actor = match &self.operator {

5348 Some(u) => ActorContext {

5449 user_id: Some(u.uid),

Modified lib/auth/features/access_token.rs +0 −6 View lib/auth/features/access_token.rs

@@ -1,11 +1,5 @@

11use sha2::{Digest, Sha256};

22

3/// Hash an access-key token with its per-key salt.

4///

5/// Format is `sha256(token ∥ salt)` hex, matching the value stored in

6/// `user_access_key.token_hash` when a key is created. Kept here (rather than

7/// duplicated in the `user` and `gitauth` crates) so creation and verification

8/// can never drift apart.

93pub fn hash(token: &str, salt: &str) -> String {

104 let mut h = Sha256::new();

115 h.update(token.as_bytes());

Modified lib/webhook/dispatcher.rs +5 −44 View lib/webhook/dispatcher.rs

@@ -20,15 +20,12 @@

2020use tracing::{debug, warn};

2121use uuid::Uuid;

2222

23/// Terminal outcome of a delivery attempt, distinguishing a successful HTTP

24/// response from a failure with a response (4xx/5xx) or a transport error.

2523#[derive(Debug, Clone)]

2624enum DeliveryOutcome {

2725 Success { status: i32, body: Option<String> },

2826 Failed { status: Option<i32>, body: Option<String> },

2927}

3028

31/// Retry parameters derived from [`WebhookConfig`]; passable in isolation for tests.

3229#[derive(Debug, Clone)]

3330pub struct RetryPolicy {

3431 pub max_attempts: u32,

@@ -46,12 +43,6 @@

4643 }

4744}

4845

49/// Async, retry-capable webhook dispatcher.

50///

51/// Implements [`gitsync::WebhookTx`] so it can replace [`gitsync::NoopWebhookTx`].

52/// Each (webhook, ref_update) is dispatched on a spawned task; the trait method

53/// returns `Ok(())` immediately so the gitsync consumer is never blocked.

54/// Final outcomes (Success or Failed) are recorded exactly once per pair.

5546#[derive(Clone)]

5647pub struct WebhookDispatcher {

5748 db: Db,

@@ -63,15 +54,12 @@

6354}

6455

6556impl WebhookDispatcher {

66 /// Low-level constructor. Useful for unit tests that need to inject a custom

67 /// `reqwest::Client` or `RetryPolicy`.

68 #[must_use]

57 #[must_use]

6958 pub fn new(db: Db, http: reqwest::Client, retry: RetryPolicy) -> Self {

7059 Self::with_concurrency(db, http, retry, 32)

7160 }

7261

73 /// Constructor with an explicit concurrency ceiling.

74 #[must_use]

62 #[must_use]

7563 pub fn with_concurrency(

7664 db: Db,

7765 http: reqwest::Client,

@@ -88,16 +76,11 @@

8876 }

8977 }

9078

91 /// Build an HTTP client from [`WebhookConfig`] and return a ready dispatcher.

92 /// Returns an error only if `reqwest::Client::builder().build()` fails, which

93 /// is unlikely for valid configs but reported honestly regardless.

94 pub fn from_config(cfg: &WebhookConfig, db: Db) -> Result<Self, reqwest::Error> {

79 pub fn from_config(cfg: &WebhookConfig, db: Db) -> Result<Self, reqwest::Error> {

9580 Self::from_config_shared(cfg, db)

9681 }

9782

98 /// Convenience for tests that want a dispatcher without configuring an HTTP

99 /// client: builds a fresh `reqwest::Client` with the workspace defaults.

100 #[must_use]

83 #[must_use]

10184 pub fn with_defaults(db: Db) -> Self {

10285 let cfg = WebhookConfig::default();

10386 Self::from_config_shared(&cfg, db).expect("default webhook config always builds")

@@ -127,10 +110,7 @@

127110 ))

128111 }

129112

130 /// Wait for all currently-spawned dispatch tasks to finish. New calls to

131 /// [`Self::dispatch`] after this are recorded inline as `Failed` (without

132 /// spawning) to preserve the single-row invariant. Idempotent.

133 pub async fn drain(&self) {

113 pub async fn drain(&self) {

134114 self.draining.store(true, Ordering::Release);

135115 let mut set = {

136116 let mut guard = self.in_flight.lock().await;

@@ -186,9 +166,6 @@

186166 let payload = serde_json::to_vec(&build_push_payload(task, &repo, update))?;

187167 let signature = sign_hmac_sha256(&payload, &webhook.secret);

188168 if draining {

189 // Drain started: do not spawn. Record the outcome inline so

190 // operators still see a row, marked as failed with the

191 // drain reason.

192169 let outcome = DeliveryOutcome::Failed {

193170 status: None,

194171 body: Some("dispatcher draining; new deliveries rejected".into()),

@@ -215,15 +192,9 @@

215192 let retry = self.retry.clone();

216193 let concurrency = self.concurrency.clone();

217194 let in_flight = self.in_flight.clone();

218 // Try to register with the JoinSet without awaiting on the mutex; if

219 // the dispatcher is currently being drained (lock contended by drain)

220 // fall back to a detached task so we never block the consumer.

221195 let mut guard = match in_flight.try_lock() {

222196 Ok(g) => g,

223197 Err(_) => {

224 // Drain holds the lock; register nothing, run detached so the

225 // consumer is never blocked. The associated row is still

226 // written by the task itself.

227198 tokio::spawn(async move {

228199 let Ok(_permit) = concurrency.acquire_owned().await else {

229200 return;

@@ -246,7 +217,6 @@

246217 }

247218}

248219

249/// Run the retry loop. Returns the terminal outcome to be recorded.

250220async fn run_retry(

251221 http: &reqwest::Client,

252222 url: &str,

@@ -280,7 +250,6 @@

280250 }

281251 continue;

282252 }

283 // Terminal 4xx. Record and stop.

284253 counter!("webhook.deliveries", "result" => "failed").increment(1);

285254 return DeliveryOutcome::Failed { status, body };

286255 }

@@ -305,7 +274,6 @@

305274 })

306275}

307276

308/// One HTTP attempt. Returns (status_code, response_body, parsed_retry_after).

309277async fn post_once(

310278 http: &reqwest::Client,

311279 url: &str,

@@ -336,13 +304,10 @@

336304 Ok((Some(status), Some(body), retry_after))

337305}

338306

339/// Parse RFC 7231 Retry-After into a Duration (seconds). Both delta-seconds and

340/// HTTP-date forms are accepted. Returns None on any parse failure.

341307fn parse_retry_after_seconds(value: &str) -> Option<Duration> {

342308 if let Ok(secs) = value.trim().parse::<u64>() {

343309 return Some(Duration::from_secs(secs));

344310 }

345 // HTTP-date form: e.g. "Wed, 21 Oct 2015 07:28:00 GMT".

346311 if let Ok(ts) = time::OffsetDateTime::parse(

347312 value.trim(),

348313 &time::format_description::well_known::Rfc2822,

@@ -359,11 +324,7 @@

359324 code == 408 || code == 429 || (500..600).contains(&code)

360325}

361326

362/// Full-jitter exponential backoff: cap = min(max_delay, base * 2^(attempt-1));

363/// sleep = rand(0..=cap).

364327fn backoff_delay(retry: &RetryPolicy, attempt: u32, retry_after: Option<Duration>) -> Duration {

365 // Honor Retry-After when present, capped to max_delay to defend against

366 // misbehaving / hostile servers; fall back to full-jitter exponential.

367328 if let Some(s) = retry_after {

368329 return s.min(retry.max_delay).max(retry.base_delay);

369330 }