feat(onboarding): input validation + editable targets
CI / Check (pull_request) Successful in 5m34s
CI / Detect Changes (pull_request) Has been skipped
CI / Deploy Agent (pull_request) Has been skipped
CI / Deploy Dashboard (pull_request) Has been skipped
CI / Deploy Docs (pull_request) Has been skipped
CI / Deploy MCP (pull_request) Has been skipped
CI / Check (pull_request) Successful in 5m34s
CI / Detect Changes (pull_request) Has been skipped
CI / Deploy Agent (pull_request) Has been skipped
CI / Deploy Dashboard (pull_request) Has been skipped
CI / Deploy Docs (pull_request) Has been skipped
CI / Deploy MCP (pull_request) Has been skipped
Two gaps surfaced while testing: bad input (a pasted label in a git URL, a
slash in the name) was only discovered at scan time, and there was no way to
fix a target once created.
Validation (client-side, shared by the wizard and the editor):
- `validate_target_name` — non-empty, no stray spaces, no slashes (the name is
used as the clone directory).
- `validate_artifact_ref` — per-kind checks (git URL shape, http(s) for live
URLs, image-ref/path for the rest). The wizard shows the error inline and
disables Next / + Add until it's clean.
Editing:
- `PATCH /api/v1/targets/{id}` now accepts an `artifacts` replacement.
- New `update_target` server fn + an Edit modal on the Targets page: change
name, type, and add/remove artifacts (same validation), then Save.
Robustness:
- `GitOps::clone_or_fetch` sanitizes the repo name into one filesystem-safe
directory segment, so a slash (or other path-hostile char) in a name can
never nest or break the clone path again (+ unit test).
- Drive-by: `sbom` license summary uses `sort_by_key(Reverse(..))`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
613847d4d3
commit
96763b7fe9
@@ -75,6 +75,9 @@ pub struct UpdateTargetRequest {
|
||||
pub scan_config: Option<TargetScanConfig>,
|
||||
pub compliance_profile: Option<ComplianceProfile>,
|
||||
pub scan_schedule: Option<String>,
|
||||
/// Replace the target's artifacts wholesale (used by the dashboard editor).
|
||||
#[serde(default)]
|
||||
pub artifacts: Option<Vec<ArtifactInput>>,
|
||||
}
|
||||
|
||||
/// One applicable-scan option, serialized for the wizard.
|
||||
@@ -214,6 +217,13 @@ pub async fn update_target(
|
||||
if let Some(ss) = req.scan_schedule {
|
||||
set.insert("scan_schedule", ss);
|
||||
}
|
||||
if let Some(arts) = req.artifacts {
|
||||
let built: Vec<Artifact> = arts.iter().map(ArtifactInput::build).collect();
|
||||
set.insert(
|
||||
"artifacts",
|
||||
to_bson(&built).map_err(|_| StatusCode::BAD_REQUEST)?,
|
||||
);
|
||||
}
|
||||
|
||||
db.onboarded_targets()
|
||||
.update_one(doc! { "_id": oid }, doc! { "$set": set })
|
||||
|
||||
@@ -282,7 +282,7 @@ pub async fn license_summary(
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
summaries.sort_by(|a, b| b.count.cmp(&a.count));
|
||||
summaries.sort_by_key(|s| std::cmp::Reverse(s.count));
|
||||
|
||||
Ok(Json(ApiResponse {
|
||||
data: summaries,
|
||||
|
||||
@@ -80,7 +80,10 @@ impl GitOps {
|
||||
|
||||
#[tracing::instrument(skip_all, fields(repo_name = %repo_name))]
|
||||
pub fn clone_or_fetch(&self, git_url: &str, repo_name: &str) -> Result<PathBuf, AgentError> {
|
||||
let repo_path = self.base_path.join(repo_name);
|
||||
// Names can contain slashes or other path-hostile characters (a target
|
||||
// named after a repo path, say); collapse to one safe directory segment
|
||||
// so the clone path never nests or breaks.
|
||||
let repo_path = self.base_path.join(sanitize_repo_dir(repo_name));
|
||||
|
||||
if repo_path.exists() {
|
||||
tracing::info!("fetching updates for existing repo");
|
||||
@@ -253,3 +256,46 @@ pub struct DiffFile {
|
||||
pub path: String,
|
||||
pub hunks: String,
|
||||
}
|
||||
|
||||
/// Collapse a repository name into a single filesystem-safe directory segment.
|
||||
/// Names may carry slashes or other path-hostile characters (a target named
|
||||
/// after a repo path, for instance); those would otherwise nest or break the
|
||||
/// clone path, so map anything outside `[A-Za-z0-9._-]` to `_`.
|
||||
fn sanitize_repo_dir(name: &str) -> String {
|
||||
let mapped: String = name
|
||||
.chars()
|
||||
.map(|c| {
|
||||
if c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.' {
|
||||
c
|
||||
} else {
|
||||
'_'
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
let trimmed = mapped.trim_matches(|c| c == '.' || c == '_');
|
||||
if trimmed.is_empty() {
|
||||
"repo".to_string()
|
||||
} else {
|
||||
trimmed.to_string()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::sanitize_repo_dir;
|
||||
|
||||
#[test]
|
||||
fn sanitizes_path_hostile_names() {
|
||||
assert_eq!(
|
||||
sanitize_repo_dir("zephyr-example-app"),
|
||||
"zephyr-example-app"
|
||||
);
|
||||
assert_eq!(
|
||||
sanitize_repo_dir("ChristianRinn/bare_metal_stm32f411xe"),
|
||||
"ChristianRinn_bare_metal_stm32f411xe"
|
||||
);
|
||||
assert_eq!(sanitize_repo_dir("../../etc/passwd"), "etc_passwd");
|
||||
assert_eq!(sanitize_repo_dir("a b:c"), "a_b_c");
|
||||
assert_eq!(sanitize_repo_dir("///"), "repo");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user