azcomputerguru/guru-connect
1
0
Fork 0
Code Issues 4 Pull Requests Actions 2 Packages Projects Releases 2 Wiki Activity

style(enroll): cargo fmt --all (satisfy CI fmt gate)
All checks were successful
Build and Test / Build Agent (Windows) (pull_request) Successful in 16m35s
Details
Build and Test / Build Server (Linux) (pull_request) Successful in 19m7s
Details
Build and Test / Security Audit (pull_request) Successful in 5m27s
Details
Build and Test / Build Summary (pull_request) Successful in 26s
Details

Browse Source 
The Phase A work passed cargo check + clippy + tests locally but missed
`cargo fmt --all -- --check` (the first step of the Linux CI job): module
ordering in db/mod.rs and two trailing-comment alignments in rate_limit.rs.
No logic change. Agent build failure on the prior run was transient infra
(verified: agent crate compiles clean locally).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mike Swanson
2026-06-02 10:48:51 -07:00
parent 0f02f23765
commit 4106fc4bc4
5 changed files with 296 additions and 225 deletions
Download Patch File Download Diff File Expand all files Collapse all files

499
server/src/api/enroll.rs
View File

@@ -218,11 +218,7 @@ fn is_collision(existing: &db::machines::Machine, incoming_hostname: &str) -> bo
/// Mint a `cak_`, store its hash bound to `machine_id` + tenant, and return the
/// plaintext. Shared by the new/reuse/move active paths.
async fn mint_cak(
db: &db::Database,
machine_id: Uuid,
tenant_id: Uuid,
) -> ApiResult<String> {
async fn mint_cak(db: &db::Database, machine_id: Uuid, tenant_id: Uuid) -> ApiResult<String> {
let plaintext = agent_keys::generate_agent_key();
let key_hash = agent_keys::hash_agent_key(&plaintext);
db::agent_keys::insert_agent_key(db.pool(), machine_id, &key_hash, Some(tenant_id))
@@ -287,11 +283,16 @@ pub async fn enroll(
// site_code is not distinguishable by timing (enumeration oracle).
equalize_reject_timing(&req.enrollment_key);
state.rate_limits.enroll.record_failure(site_code, ip);
audit(db, db::events::EventTypes::ENROLL_REJECTED, ip, json!({
"reason": "unknown_site_code",
"site_code": site_code,
"machine_uid": machine_uid,
}))
audit(
db,
db::events::EventTypes::ENROLL_REJECTED,
ip,
json!({
"reason": "unknown_site_code",
"site_code": site_code,
"machine_uid": machine_uid,
}),
)
.await;
tracing::warn!("[ENROLL] unknown site_code={} from {}", site_code, ip);
// Same opaque rejection shape AND the same KDF cost as a bad key — do not
@@ -322,13 +323,21 @@ pub async fn enroll(
// has no active key" is not distinguishable by timing from a bad key.
equalize_reject_timing(&req.enrollment_key);
state.rate_limits.enroll.record_failure(site_code, ip);
audit(db, db::events::EventTypes::ENROLL_REJECTED, ip, json!({
"reason": "no_active_key",
"site_code": site_code,
"machine_uid": machine_uid,
}))
audit(
db,
db::events::EventTypes::ENROLL_REJECTED,
ip,
json!({
"reason": "no_active_key",
"site_code": site_code,
"machine_uid": machine_uid,
}),
)
.await;
tracing::warn!("[ENROLL] no active enrollment key for site_code={}", site_code);
tracing::warn!(
"[ENROLL] no active enrollment key for site_code={}",
site_code
);
return Err(ApiError::new(
StatusCode::UNAUTHORIZED,
"ENROLL_REJECTED",
@@ -347,13 +356,22 @@ pub async fn enroll(
if !enrollment_keys::verify_enrollment_key(&req.enrollment_key, &active_key.key_hash) {
state.rate_limits.enroll.record_failure(site_code, ip);
audit(db, db::events::EventTypes::ENROLL_REJECTED, ip, json!({
"reason": "bad_enrollment_key",
"site_code": site_code,
"machine_uid": machine_uid,
}))
audit(
db,
db::events::EventTypes::ENROLL_REJECTED,
ip,
json!({
"reason": "bad_enrollment_key",
"site_code": site_code,
"machine_uid": machine_uid,
}),
)
.await;
tracing::warn!("[ENROLL] bad enrollment key for site_code={} from {}", site_code, ip);
tracing::warn!(
"[ENROLL] bad enrollment key for site_code={} from {}",
site_code,
ip
);
return Err(ApiError::new(
StatusCode::UNAUTHORIZED,
"ENROLL_REJECTED",
@@ -366,7 +384,12 @@ pub async fn enroll(
// Build the label/identity params shared by the create/update paths.
let tags = effective_tags(&req.labels);
let company = req.labels.company.as_deref().map(str::trim).filter(|s| !s.is_empty());
let company = req
.labels
.company
.as_deref()
.map(str::trim)
.filter(|s| !s.is_empty());
let site_label = req
.labels
.site
@@ -386,25 +409,137 @@ pub async fn enroll(
let mut attempts = 0u8;
loop {
attempts += 1;
let existing =
match db::machines::get_machine_by_tenant_uid(db.pool(), tenant_id, machine_uid).await {
Ok(e) => e,
Err(e) => {
tracing::error!("[ENROLL] DB error on dedup lookup: {}", e);
return Err(ApiError::new(
StatusCode::INTERNAL_SERVER_ERROR,
"INTERNAL_ERROR",
"Internal server error",
));
}
};
let existing = match db::machines::get_machine_by_tenant_uid(
db.pool(),
tenant_id,
machine_uid,
)
.await
{
Ok(e) => e,
Err(e) => {
tracing::error!("[ENROLL] DB error on dedup lookup: {}", e);
return Err(ApiError::new(
StatusCode::INTERNAL_SERVER_ERROR,
"INTERNAL_ERROR",
"Internal server error",
));
}
};
match existing {
// -- Reuse / collision path -------------------------------------------
Some(existing) => {
// Collision gate: a seemingly-different live endpoint resolving to an
// existing uid -> pending, alert, NO usable cak_ minted.
if is_collision(&existing, hostname) {
// -- Reuse / collision path -------------------------------------------
Some(existing) => {
// Collision gate: a seemingly-different live endpoint resolving to an
// existing uid -> pending, alert, NO usable cak_ minted.
if is_collision(&existing, hostname) {
let params = enroll_params(
&existing.agent_id,
hostname,
machine_uid,
tenant_id,
site.id,
company,
site_label,
&tags,
"pending",
);
let machine =
db::machines::update_enrolled_machine(db.pool(), existing.id, &params)
.await
.map_err(map_update_err)?;
audit(
db,
db::events::EventTypes::ENROLL_COLLISION_PENDING,
ip,
json!({
"machine_id": machine.id,
"machine_uid": machine_uid,
"site_code": site_code,
"existing_hostname": existing.hostname,
"incoming_hostname": hostname,
"heuristic": "online_existing_with_different_hostname (PROVISIONAL)",
}),
)
.await;
// TODO(SPEC-016): wire to #dev-alerts — collision requires operator
// confirmation in the dashboard before this endpoint may activate.
tracing::warn!(
"[ENROLL] machine_uid collision -> PENDING: machine_id={} site_code={} \
existing_host={} incoming_host={} from {}",
machine.id,
site_code,
existing.hostname,
hostname,
ip
);
return Ok((
StatusCode::ACCEPTED,
Json(EnrollResponse {
machine_id: machine.id,
key: None,
enrollment_state: "pending".to_string(),
disposition: "collision_pending".to_string(),
}),
));
}
// Cross-site guard (SPEC-016 Phase A, HIGH): if this machine_uid is
// already bound to a DIFFERENT site, REFUSE — do not move it and do not
// mint a key. `machine_uid` is a raw, spoofable client string in Phase A,
// so a silent move here is a hijack vector: a holder of site B's key could
// claim a site-A machine merely by presenting its uid, repointing the row
// and minting a fresh `cak_` on site B. Deliberate moves are out of scope
// for Phase A and arrive with the Phase-B `--reassign` flow + dashboard
// (SPEC-016 §"Explicitly out of scope"); until then this is the explicit
// accidental-move / hijack guard the spec calls for.
//
// Only `Some(other_site)` is a conflict. `None` (a legacy / connect-path /
// support-code row that never had a relational site binding) is treated as
// a first-time bind to the enrolling site, NOT a cross-site move — there is
// no prior owning site to hijack from.
if let Some(existing_site) = existing.site_id {
if existing_site != site.id {
audit(
db,
db::events::EventTypes::ENROLL_SITE_CONFLICT,
ip,
json!({
"machine_id": existing.id,
"machine_uid": machine_uid,
// Record the bound site id for the operator audit trail; the
// response body below leaks nothing about the other site.
"bound_site_id": existing_site,
"attempted_site_code": site_code,
"attempted_site_id": site.id,
}),
)
.await;
// TODO(SPEC-016): wire to #dev-alerts — cross-site enroll refused
// (possible accidental move or a cross-site claim attempt).
tracing::warn!(
"[ENROLL] cross-site conflict REFUSED: machine_id={} already bound to \
a different site; attempted site_code={} from {}",
existing.id,
site_code,
ip
);
// Opaque-enough: states the machine is already enrolled elsewhere
// and how to move it deliberately, without naming the other site.
return Err(ApiError::new(
StatusCode::CONFLICT,
"ENROLL_SITE_CONFLICT",
"This machine is already enrolled at another site. A deliberate \
move requires the operator-initiated reassignment flow.",
));
}
}
// Same-site reuse (re-image / re-install) or first relational bind of a
// legacy/connect-path row (existing.site_id was NULL). No site move occurs
// on the unauthenticated path in Phase A.
let params = enroll_params(
&existing.agent_id,
hostname,
@@ -414,195 +549,123 @@ pub async fn enroll(
company,
site_label,
&tags,
"pending",
"active",
);
let machine = db::machines::update_enrolled_machine(db.pool(), existing.id, &params)
.await
.map_err(map_update_err)?;
let machine =
db::machines::update_enrolled_machine(db.pool(), existing.id, &params)
.await
.map_err(map_update_err)?;
audit(db, db::events::EventTypes::ENROLL_COLLISION_PENDING, ip, json!({
"machine_id": machine.id,
"machine_uid": machine_uid,
"site_code": site_code,
"existing_hostname": existing.hostname,
"incoming_hostname": hostname,
"heuristic": "online_existing_with_different_hostname (PROVISIONAL)",
}))
let cak = mint_cak(db, machine.id, tenant_id).await?;
audit(
db,
db::events::EventTypes::ENROLL_REUSE,
ip,
json!({
"machine_id": machine.id,
"machine_uid": machine_uid,
"site_code": site_code,
}),
)
.await;
// TODO(SPEC-016): wire to #dev-alerts — collision requires operator
// confirmation in the dashboard before this endpoint may activate.
tracing::warn!(
"[ENROLL] machine_uid collision -> PENDING: machine_id={} site_code={} \
existing_host={} incoming_host={} from {}",
machine.id, site_code, existing.hostname, hostname, ip
tracing::info!(
"[ENROLL] reuse: machine_id={} re-enrolled at site_code={} from {}",
machine.id,
site_code,
ip
);
return Ok((
StatusCode::ACCEPTED,
StatusCode::OK,
Json(EnrollResponse {
machine_id: machine.id,
key: None,
enrollment_state: "pending".to_string(),
disposition: "collision_pending".to_string(),
key: Some(cak),
enrollment_state: "active".to_string(),
disposition: "reuse".to_string(),
}),
));
}
// Cross-site guard (SPEC-016 Phase A, HIGH): if this machine_uid is
// already bound to a DIFFERENT site, REFUSE — do not move it and do not
// mint a key. `machine_uid` is a raw, spoofable client string in Phase A,
// so a silent move here is a hijack vector: a holder of site B's key could
// claim a site-A machine merely by presenting its uid, repointing the row
// and minting a fresh `cak_` on site B. Deliberate moves are out of scope
// for Phase A and arrive with the Phase-B `--reassign` flow + dashboard
// (SPEC-016 §"Explicitly out of scope"); until then this is the explicit
// accidental-move / hijack guard the spec calls for.
//
// Only `Some(other_site)` is a conflict. `None` (a legacy / connect-path /
// support-code row that never had a relational site binding) is treated as
// a first-time bind to the enrolling site, NOT a cross-site move — there is
// no prior owning site to hijack from.
if let Some(existing_site) = existing.site_id {
if existing_site != site.id {
audit(db, db::events::EventTypes::ENROLL_SITE_CONFLICT, ip, json!({
"machine_id": existing.id,
"machine_uid": machine_uid,
// Record the bound site id for the operator audit trail; the
// response body below leaks nothing about the other site.
"bound_site_id": existing_site,
"attempted_site_code": site_code,
"attempted_site_id": site.id,
}))
.await;
// TODO(SPEC-016): wire to #dev-alerts — cross-site enroll refused
// (possible accidental move or a cross-site claim attempt).
tracing::warn!(
"[ENROLL] cross-site conflict REFUSED: machine_id={} already bound to \
a different site; attempted site_code={} from {}",
existing.id, site_code, ip
);
// Opaque-enough: states the machine is already enrolled elsewhere
// and how to move it deliberately, without naming the other site.
return Err(ApiError::new(
StatusCode::CONFLICT,
"ENROLL_SITE_CONFLICT",
"This machine is already enrolled at another site. A deliberate \
move requires the operator-initiated reassignment flow.",
));
}
}
// Same-site reuse (re-image / re-install) or first relational bind of a
// legacy/connect-path row (existing.site_id was NULL). No site move occurs
// on the unauthenticated path in Phase A.
let params = enroll_params(
&existing.agent_id,
hostname,
machine_uid,
tenant_id,
site.id,
company,
site_label,
&tags,
"active",
);
let machine = db::machines::update_enrolled_machine(db.pool(), existing.id, &params)
.await
.map_err(map_update_err)?;
let cak = mint_cak(db, machine.id, tenant_id).await?;
audit(db, db::events::EventTypes::ENROLL_REUSE, ip, json!({
"machine_id": machine.id,
"machine_uid": machine_uid,
"site_code": site_code,
}))
.await;
tracing::info!(
"[ENROLL] reuse: machine_id={} re-enrolled at site_code={} from {}",
machine.id, site_code, ip
);
return Ok((
StatusCode::OK,
Json(EnrollResponse {
machine_id: machine.id,
key: Some(cak),
enrollment_state: "active".to_string(),
disposition: "reuse".to_string(),
}),
));
}
// -- New enrollment ----------------------------------------------------
None => {
// Fresh opaque agent_id for the new row's `agent_id UNIQUE` column. The
// agent's own config-UUID story is Phase B; the server only needs a
// unique non-null value here, and the authoritative identity is the
// minted cak_ -> machine binding.
let agent_id = format!("enroll-{}", Uuid::new_v4());
let params = enroll_params(
&agent_id,
hostname,
machine_uid,
tenant_id,
site.id,
company,
site_label,
&tags,
"active",
);
let machine = match db::machines::insert_enrolled_machine(db.pool(), &params).await {
Ok(m) => m,
// TOCTOU loser: a concurrent first-enroll of the same machine_uid won
// the race and committed its row between our lookup and this INSERT, so
// the partial unique index on `machine_uid` rejects ours. Loop back: the
// re-lookup now finds the winner's row and we converge to reuse instead
// of 500ing. Capped so a non-uid unique violation (or any persistent
// conflict) surfaces as a 500 rather than spinning.
Err(e) if is_machine_uid_conflict(&e) && attempts < 2 => {
tracing::info!(
"[ENROLL] concurrent first-enroll race on machine_uid; \
// -- New enrollment ----------------------------------------------------
None => {
// Fresh opaque agent_id for the new row's `agent_id UNIQUE` column. The
// agent's own config-UUID story is Phase B; the server only needs a
// unique non-null value here, and the authoritative identity is the
// minted cak_ -> machine binding.
let agent_id = format!("enroll-{}", Uuid::new_v4());
let params = enroll_params(
&agent_id,
hostname,
machine_uid,
tenant_id,
site.id,
company,
site_label,
&tags,
"active",
);
let machine = match db::machines::insert_enrolled_machine(db.pool(), &params).await
{
Ok(m) => m,
// TOCTOU loser: a concurrent first-enroll of the same machine_uid won
// the race and committed its row between our lookup and this INSERT, so
// the partial unique index on `machine_uid` rejects ours. Loop back: the
// re-lookup now finds the winner's row and we converge to reuse instead
// of 500ing. Capped so a non-uid unique violation (or any persistent
// conflict) surfaces as a 500 rather than spinning.
Err(e) if is_machine_uid_conflict(&e) && attempts < 2 => {
tracing::info!(
"[ENROLL] concurrent first-enroll race on machine_uid; \
retrying as reuse (site_code={} from {})",
site_code, ip
);
continue;
}
Err(e) => {
tracing::error!("[ENROLL] DB error inserting enrolled machine: {}", e);
return Err(ApiError::new(
StatusCode::INTERNAL_SERVER_ERROR,
"INTERNAL_ERROR",
"Failed to register machine",
));
}
};
site_code,
ip
);
continue;
}
Err(e) => {
tracing::error!("[ENROLL] DB error inserting enrolled machine: {}", e);
return Err(ApiError::new(
StatusCode::INTERNAL_SERVER_ERROR,
"INTERNAL_ERROR",
"Failed to register machine",
));
}
};
let cak = mint_cak(db, machine.id, tenant_id).await?;
let cak = mint_cak(db, machine.id, tenant_id).await?;
audit(db, db::events::EventTypes::ENROLL_NEW, ip, json!({
"machine_id": machine.id,
"machine_uid": machine_uid,
"site_code": site_code,
"hostname": hostname,
}))
.await;
// TODO(SPEC-016): wire to #dev-alerts — new-enrollment tripwire.
tracing::info!(
"[ENROLL] new: machine_id={} hostname={} site_code={} from {}",
machine.id, hostname, site_code, ip
);
audit(
db,
db::events::EventTypes::ENROLL_NEW,
ip,
json!({
"machine_id": machine.id,
"machine_uid": machine_uid,
"site_code": site_code,
"hostname": hostname,
}),
)
.await;
// TODO(SPEC-016): wire to #dev-alerts — new-enrollment tripwire.
tracing::info!(
"[ENROLL] new: machine_id={} hostname={} site_code={} from {}",
machine.id,
hostname,
site_code,
ip
);
return Ok((
StatusCode::CREATED,
Json(EnrollResponse {
machine_id: machine.id,
key: Some(cak),
enrollment_state: "active".to_string(),
disposition: "new".to_string(),
}),
));
}
return Ok((
StatusCode::CREATED,
Json(EnrollResponse {
machine_id: machine.id,
key: Some(cak),
enrollment_state: "active".to_string(),
disposition: "new".to_string(),
}),
));
}
}
}
}
@@ -636,7 +699,12 @@ fn effective_tags(labels: &EnrollLabels) -> Vec<String> {
.map(|t| t.trim().to_string())
.filter(|t| !t.is_empty())
.collect();
if let Some(d) = labels.department.as_deref().map(str::trim).filter(|s| !s.is_empty()) {
if let Some(d) = labels
.department
.as_deref()
.map(str::trim)
.filter(|s| !s.is_empty())
{
tags.push(format!("department:{}", d));
}
if let Some(d) = labels
@@ -708,7 +776,8 @@ mod tests {
fn timing_equalizer_phc_is_a_valid_parseable_hash() {
// A wrong password must verify to `false` WITHOUT erroring — proving the PHC
// parsed and the KDF actually ran (a parse failure would surface as `Err`).
let res = crate::auth::password::verify_password("cek_anything_at_all", TIMING_EQUALIZER_PHC);
let res =
crate::auth::password::verify_password("cek_anything_at_all", TIMING_EQUALIZER_PHC);
assert!(
res.is_ok(),
"TIMING_EQUALIZER_PHC must be a valid PHC string so the KDF runs; got Err: {:?}",

5
server/src/auth/enrollment_keys.rs
View File

@@ -178,7 +178,10 @@ mod tests {
#[test]
fn fingerprint_differs_per_key() {
assert_ne!(compute_fingerprint("cek_aaa"), compute_fingerprint("cek_bbb"));
assert_ne!(
compute_fingerprint("cek_aaa"),
compute_fingerprint("cek_bbb")
);
}
#[test]

11
server/src/db/enrollment_keys.rs
View File

@@ -102,12 +102,11 @@ pub async fn rotate_key(
let mut tx = pool.begin().await?;
// Highest existing version for this site (NULL -> 0 so the first key is v1).
let current_max: Option<i32> = sqlx::query_scalar(
"SELECT MAX(version) FROM site_enrollment_keys WHERE site_id = $1",
)
.bind(site_id)
.fetch_one(&mut *tx)
.await?;
let current_max: Option<i32> =
sqlx::query_scalar("SELECT MAX(version) FROM site_enrollment_keys WHERE site_id = $1")
.bind(site_id)
.fetch_one(&mut *tx)
.await?;
let next_version = current_max.unwrap_or(0) + 1;
// Deactivate the current active key (if any), stamping rotated_at.

2
server/src/db/mod.rs
View File

@@ -7,9 +7,9 @@ pub mod agent_keys;
pub mod enrollment_keys;
pub mod events;
pub mod machines;
pub mod sites;
pub mod releases;
pub mod sessions;
pub mod sites;
pub mod support_codes;
pub mod tenancy;
pub mod users;

4
server/src/middleware/rate_limit.rs
View File

@@ -708,7 +708,7 @@ mod tests {
let t0 = Instant::now();
assert!(lim.check_at("SITE-A", ip(1), t0));
assert!(!lim.check_at("SITE-A", ip(1), t0)); // same key over cap
// Different site, same IP -> independent bucket.
// Different site, same IP -> independent bucket.
assert!(lim.check_at("SITE-B", ip(1), t0));
// Same site, different IP -> independent bucket.
assert!(lim.check_at("SITE-A", ip(2), t0));
@@ -723,7 +723,7 @@ mod tests {
// Not yet tripped: a check still admits.
assert!(lim.check_at("SITE-A", ip(1), t0));
lim.record_failure_at("SITE-A", ip(1), t0); // 3rd -> trips
// Now locked out: check denies even though under the request cap.
// Now locked out: check denies even though under the request cap.
assert!(!lim.check_at("SITE-A", ip(1), t0));
}