Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use crate::datadog_static_analyzer_server::ide::configuration_file::models::{

use super::error::ConfigFileError;
use super::models::{AddRuleSetsRequest, IgnoreRuleRequest};
use super::static_analysis_config_file::{Base64String, StaticAnalysisConfigFile};
use super::static_analysis_config_file::{
Base64String, ConfigFormat, StaticAnalysisConfigFile, WithHint,
};
use kernel::utils::encode_base64_string;
use rocket::http::Status;
use rocket::response::status::Custom;
Expand Down Expand Up @@ -35,26 +37,35 @@ pub fn post_ignore_rule(
rule,
configuration_base64,
encoded,
..
schema_version,
} = request.into_inner();
tracing::debug!(rule, content = &configuration_base64);
let result = StaticAnalysisConfigFile::with_ignored_rule(
rule.into(),
Base64String(configuration_base64),
ConfigFormat::from(schema_version.as_deref()),
);
to_response_result(result, encoded)
Comment thread
robertohuertasm-datadog marked this conversation as resolved.
}

/// Checks if onboarding is allowed for the configuration file (deprecated).
/// Checks if onboarding is allowed for the configuration file (legacy route).
///
/// # Arguments
/// * `content` - The path to the configuration file.
/// * `content` - The base64-encoded configuration file content, passed as a URL path segment.
#[instrument()]
#[rocket::get("/v1/config/can-onboard/<content..>")]
#[deprecated(note = "IDEs stopped supporting onboarding")]
pub fn get_can_onboard(content: PathBuf) -> Result<Json<bool>, Custom<ConfigFileError>> {
let content_str = content.to_string_lossy().into_owned();
can_onboard(content_str)
// NOTE: this method is still used by Visual Studio.
let mut content_str = content.to_string_lossy().into_owned();
if cfg!(target_os = "windows") {
// NOTE: this is needed due to how Rocket works with multiple segment captures.
content_str = content_str.replace("\\", "/");
}
let config = StaticAnalysisConfigFile::try_from(WithHint(Base64String(content_str), None))
.map_err(|e| Custom(Status::InternalServerError, e))?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like from the description that this 500 is intentional. Would you please add a comment explaining why? I would otherwise wonder why it's not 400 like elsewhere

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! You're right, it should be 400 and I've corrected it. Any error on this route is a parse failure on client-supplied content (bad base64 or invalid YAML), so 500 was semantically wrong. I initially kept the original 500 to avoid inadvertently breaking the Visual Studio extension (the only known caller of this deprecated route), but after checking its source I confirmed it uses a plain try/catch with no status-code inspection, so the change is safe. A comment has been added to the code explaining this. Thanks for flagging it!


let can_onboard = config.is_onboarding_allowed();
Ok(Json(can_onboard))
}

/// Checks if onboarding is allowed for the configuration file (v2).
Expand All @@ -67,15 +78,27 @@ pub fn get_can_onboard(content: PathBuf) -> Result<Json<bool>, Custom<ConfigFile
format = "application/json",
data = "<request>"
)]
#[deprecated(note = "IDEs stopped supporting onboarding")]
pub fn post_can_onboard_v2(
request: Json<CanOnboardRequest>,
) -> Result<Json<bool>, Custom<ConfigFileError>> {
let CanOnboardRequest {
configuration_base64,
..
schema_version,
} = request.into_inner();
can_onboard(configuration_base64)

let format = ConfigFormat::from(schema_version.as_deref());
let config = StaticAnalysisConfigFile::try_from(WithHint(
Base64String(configuration_base64),
Some(format),
))
.map_err(|e| Custom(Status::InternalServerError, e))?;

config
.validate_format(format)
.map_err(|e| Custom(Status::InternalServerError, e))?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe a comment why this isn't 422?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, changed to 422 Unprocessable Entity, consistent with every other SchemaMismatch mapping in the file. I initially kept the original 500 behavior here as well out of caution, to avoid breaking older clients. After verifying that the Visual Studio extension (the only client calling the deprecated v1 route) does not inspect specific HTTP status codes, and that VS Code's canOnboard uses a generic try/catch with no status-code check either, we confirmed the change is safe to make.


let can_onboard = config.is_onboarding_allowed();
Ok(Json(can_onboard))
}

/// Gets the rulesets from the static analysis configuration file (deprecated).
Expand Down Expand Up @@ -117,17 +140,27 @@ pub fn post_get_rulesets_v2(request: Json<GetRulesetsRequest>) -> Json<Vec<Strin

/// Parses the static analysis configuration YAML and returns per-product fields.
///
/// Returns `400` when the content cannot be parsed or when the detected schema does not
/// match the caller-declared `schema_version` — mirroring the CLI's filename-strict behavior.
///
/// # Arguments
/// * `request` - The request containing the raw YAML string.
#[instrument()]
#[rocket::post("/v2/config/parse", format = "application/json", data = "<request>")]
pub fn post_parse_config(
request: Json<ParseConfigRequest>,
) -> Result<Json<ParseConfigResponse>, Custom<ConfigFileError>> {
let ParseConfigRequest { configuration } = request.into_inner();
let ParseConfigRequest {
configuration,
schema_version,
} = request.into_inner();
tracing::debug!(%configuration);
let config = StaticAnalysisConfigFile::try_from(configuration)
let format = ConfigFormat::from(schema_version.as_deref());
let config = StaticAnalysisConfigFile::try_from(WithHint(configuration, Some(format)))
.map_err(|e| Custom(Status::BadRequest, e))?;
config
.validate_format(format)
.map_err(|e| Custom(Status::UnprocessableEntity, e))?;
Ok(Json(ParseConfigResponse {
sast: SastParsedConfig {
rulesets: config.sast_rulesets(),
Expand Down Expand Up @@ -158,7 +191,6 @@ pub fn post_add_rulesets(
format = "application/json",
data = "<request>"
)]
#[deprecated(note = "IDEs stopped supporting this flow")]
pub fn post_add_rulesets_v2(
request: Json<AddRuleSetsRequest>,
) -> Result<String, Custom<ConfigFileError>> {
Expand All @@ -169,22 +201,21 @@ pub fn post_add_rulesets_v2(

/// Adds rulesets to the configuration file.
///
/// # Arguments
/// * `request` - The request containing rulesets and configuration file (base64).
/// When no config content is supplied, creates a new file in the format implied by
/// `schema_version` (unified when `"v1"`, legacy otherwise). When content is supplied,
/// validates that the detected format matches the declared one before mutating.
fn add_rulesets(request: Json<AddRuleSetsRequest>) -> Result<String, Custom<ConfigFileError>> {
let AddRuleSetsRequest {
rulesets,
configuration_base64,
encoded,
..
schema_version,
} = request.into_inner();
tracing::debug!(
rulesets=?&rulesets,
content=&configuration_base64
);
tracing::debug!(rulesets=?&rulesets, content=&configuration_base64);
let result = StaticAnalysisConfigFile::with_added_rulesets(
&rulesets,
configuration_base64.map(Base64String),
ConfigFormat::from(schema_version.as_deref()),
);
to_response_result(result, encoded)
}
Expand All @@ -207,7 +238,7 @@ fn get_rulesets(mut content: String) -> Json<Vec<String>> {
content = content.replace("\\", "/");
}
tracing::debug!(%content);
let rulesets = StaticAnalysisConfigFile::try_from(Base64String(content))
let rulesets = StaticAnalysisConfigFile::try_from(WithHint(Base64String(content), None))
.map(|config| config.sast_rulesets())
.unwrap_or_else(|e| {
tracing::error!(error = ?e, "Error trying to parse config file");
Expand All @@ -216,25 +247,11 @@ fn get_rulesets(mut content: String) -> Json<Vec<String>> {
Json(rulesets)
}

/// Checks if onboarding is allowed for the configuration file.
///
/// # Arguments
/// * `content` - The configuration file content (base64).
fn can_onboard(mut content: String) -> Result<Json<bool>, Custom<ConfigFileError>> {
if cfg!(target_os = "windows") {
// NOTE: this is needed due to how Rocket works with multiple segment captures.
// we may get rid of this once v1 endpoints are no longer needed.
content = content.replace("\\", "/");
}
tracing::debug!(%content);
let config = StaticAnalysisConfigFile::try_from(Base64String(content))
.map_err(|e| Custom(Status::InternalServerError, e))?;
let can_onboard = config.is_onboarding_allowed();
Ok(Json(can_onboard))
}

/// Converts the result to a response string, optionally encoding it in base64.
///
/// `SchemaMismatch` maps to `422 Unprocessable Entity` (content is syntactically valid but
/// semantically wrong); all other failures map to `500 Internal Server Error`.
///
/// # Arguments
/// * `result` - The result string or error.
/// * `encode` - Whether to encode the result in base64.
Expand All @@ -244,5 +261,11 @@ fn to_response_result(
) -> Result<String, Custom<ConfigFileError>> {
result
.map(|r| if encode { encode_base64_string(r) } else { r })
.map_err(|e| Custom(Status::InternalServerError, e))
.map_err(|e| {
let status = match e {
ConfigFileError::SchemaMismatch => Status::UnprocessableEntity,
_ => Status::InternalServerError,
};
Custom(status, e)
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ pub enum ConfigFileError {
#[from]
source: ReconcileError,
},

#[error("Config content does not match the declared schema version")]
SchemaMismatch,
}

impl From<anyhow::Error> for ConfigFileError {
Expand All @@ -45,6 +48,7 @@ impl ConfigFileError {
Self::Parser { .. } => 1,
Self::Decoder { .. } => 2,
Self::CommentReconciler { .. } => 3,
Self::SchemaMismatch => 4,
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ pub struct IgnoreRuleRequest {
#[serde(rename = "configuration")]
pub configuration_base64: String,
pub encoded: bool,
#[serde(default)]
pub schema_version: Option<String>,
}

#[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
Expand All @@ -14,6 +16,8 @@ pub struct AddRuleSetsRequest {
#[serde(rename = "configuration")]
pub configuration_base64: Option<String>,
pub encoded: bool,
#[serde(default)]
pub schema_version: Option<String>,
}

#[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
Expand All @@ -26,11 +30,15 @@ pub struct GetRulesetsRequest {
pub struct CanOnboardRequest {
#[serde(rename = "configuration")]
pub configuration_base64: String,
#[serde(default)]
pub schema_version: Option<String>,
}

#[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
pub struct ParseConfigRequest {
pub configuration: String,
#[serde(default)]
pub schema_version: Option<String>,
}

#[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
Expand Down
Loading
Loading