Hard-fail on invalid ScopeKnownError config, warn on others

[technicalpickles]

Summary

• ScopeKnownError conversion failures (bad regex capture name, broken fix template) now abort config load instead of vanishing silently. A check that isn't actually registered gave false confidence it was protecting you.
• ScopeDoctorGroup and ScopeReportLocation conversion failures warn and continue instead, matching this file's existing warn-and-skip convention for other load errors (unreadable file, bad YAML, bad model shape).
• Multiple bad ScopeKnownError files are all reported in one run instead of one at a time.

Test plan

• New integration tests in tests/scope_root.rs: reserved capture name (the MVCE from Known-error/fix config errors are silently dropped at load time instead of failing loudly #344), a broken fix template, multiple simultaneous known-error failures, and a doctor-group warn-and-continue case.
• cargo test, cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings all clean.

Fixes #344

[dryrunsecurity]

This pull request introduces a low-severity vulnerability in src/shared/config_load.rs where the write_raw_config_to_disk function creates temporary files in a predictable path, making it susceptible to symlink attacks. Although the issue is not blocking, it poses a risk of sensitive file overwriting or unintended data writes.

Insecure temporary file creation in src/shared/config_load.rs (drs_bb2f4f35)

Vulnerability	Insecure temporary file creation
Description	The write_raw_config_to_disk function writes to a predictable path in /tmp/scope/ using File::create. This is vulnerable to symlink attacks where an attacker could create a symlink at the expected path, causing the application to overwrite sensitive files or write data to an unintended location.

scope/src/shared/config_load.rs

Lines 164 to 192 in e5eb4ba

	run_id: config_options.get_run_id(),
	};
	let mut known_error_failures: Vec<String> = Vec::new();
	for raw_config in raw_config {
	let file_path = raw_config.metadata().file_path();
	let kind = raw_config.kind.clone();
	match ParsedConfig::try_from(raw_config) {
	Ok(value) => this.add_model(value),
	Err(e) => {
	let message =
	format!("Unable to convert config from {} because {}", file_path, e);
	if is_hard_fail_kind(&kind) {
	known_error_failures.push(message);
	} else {
	warn!(target: "user", "{}", message);
	}
	}
	}
	}
	if !known_error_failures.is_empty() {
	return Err(anyhow!(known_error_failures.join("\n")));
	}
	Ok(this)
	}
	pub fn write_raw_config_to_disk(&self) -> Result<PathBuf> {

---

Comment to provide feedback on these findings.

Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]

Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing

All finding details can be found in the DryRun Security Dashboard.

[rubberduck203]

LGTM but I have been wondering since we talked if it should just WARN to the user target. That way it wouldn't prevent everyone from running analyze over one bad config file.

I feel like we could maybe do a few different things.

Instead of hard failure we could just emit the warning

warn!(target: "user", "{}", message);

and then set up a monitor to send us an alert if this specific warning is matched.

OR

We could show the user a warning ad send an error to telemetry.

warn!(target: "user", "{}", message);
error!("{}", message);

I'm 99% certain that if we don't specify a target it only ends up in the traces and is not shown to the user, but we'd have to test that to be sure.