mirror of
https://github.com/rustfs/rustfs.git
synced 2026-09-08 04:58:12 +00:00
fix(ilm): reject invalid retention counts and validate lifecycle filters (#7132)
* fix(ilm): reject invalid retention counts and validate lifecycle filters `NewerNoncurrentVersions` had no lower bound at PUT, and evaluation read a negative count through `usize::try_from(...).unwrap_or(usize::MAX)`. An HTTP-accepted rule therefore retained (almost) everything and silently stopped expiring versions — the one outcome a retention rule must never produce by accident. Reject a negative count during validation, and stop reading one as "retain everything" anywhere it can still arrive from older persistence or an import: evaluation takes no action for such a rule and says so in a diagnostic, the batch limit path yields no event, and `Evaluator::eval` reports a typed corruption error to callers that can surface one. A count-only noncurrent expiration is a MinIO extension, not an AWS form. It used to be rejected as an actionless rule and was never executed. It is now accepted and honoured with the semantics MinIO gives it: the newest N noncurrent versions are kept and every older one is due as soon as it became noncurrent. Zero keeps the meaning the batch limit path has always given it — no count constraint — so a zero-count rule with no age condition still has no action. `LifecycleRuleFilter` is an all-`Option` DTO, so the schema constraints were not checked anywhere: validate at most one top-level predicate, an `And` that combines at least two, no repeated tag key, tag key/value limits, non-negative sizes, and `ObjectSizeGreaterThan < ObjectSizeLessThan`. An empty filter stays valid — AWS documents it as "every object in the bucket". Schema-shape violations are reported with a distinct `ErrorKind` so the S3 boundary answers them with `MalformedXML`; rejected values keep the `InvalidArgument` this path has always returned. backlog#2201 * fix(ilm): satisfy lifecycle clippy checks * fix(ilm): fail closed on invalid lifecycle rules * fix: initialize optional migration source fields --------- Co-authored-by: cxymds <cxymds@gmail.com>
This commit is contained in:
@@ -27,8 +27,8 @@ use super::storage_api::bucket_usecase::bucket::target::BucketTarget;
|
||||
use super::storage_api::bucket_usecase::bucket::{
|
||||
ObjectLockConfigExt as _, VersioningConfigExt as _,
|
||||
lifecycle::bucket_lifecycle_ops::{
|
||||
enqueue_expiry_for_existing_objects, enqueue_transition_for_existing_objects, run_stale_multipart_upload_cleanup_once,
|
||||
validate_lifecycle_config, validate_transition_tier,
|
||||
LIFECYCLE_MALFORMED_XML_ERROR_KIND, enqueue_expiry_for_existing_objects, enqueue_transition_for_existing_objects,
|
||||
run_stale_multipart_upload_cleanup_once, validate_lifecycle_config, validate_transition_tier,
|
||||
},
|
||||
metadata::{
|
||||
BUCKET_CORS_CONFIG, BUCKET_LIFECYCLE_CONFIG, BUCKET_NOTIFICATION_CONFIG, BUCKET_POLICY_CONFIG,
|
||||
@@ -1188,6 +1188,21 @@ fn validate_lifecycle_rule_status(rules: &[LifecycleRule]) -> std::result::Resul
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Map a lifecycle validation failure onto the S3 error the client should see.
|
||||
///
|
||||
/// The validator reports a schema-shape violation (a `Filter` with more than
|
||||
/// one predicate, a one-member `And`) with
|
||||
/// [`LIFECYCLE_MALFORMED_XML_ERROR_KIND`]; AWS answers those with
|
||||
/// `MalformedXML`. Everything else is a value the schema allows but S3 refuses,
|
||||
/// which stays `InvalidArgument` — the code this path has always returned
|
||||
/// (backlog#2201).
|
||||
fn lifecycle_validation_error(err: &std::io::Error) -> S3Error {
|
||||
if err.kind() == LIFECYCLE_MALFORMED_XML_ERROR_KIND {
|
||||
return S3Error::with_message(S3ErrorCode::MalformedXML, format!("Malformed XML: {err}"));
|
||||
}
|
||||
s3_error!(InvalidArgument, "{err}")
|
||||
}
|
||||
|
||||
fn lifecycle_has_transition_rules(config: &BucketLifecycleConfiguration) -> bool {
|
||||
config.rules.iter().any(|rule| {
|
||||
rule.status == ExpirationStatus::from_static(ExpirationStatus::ENABLED)
|
||||
@@ -2296,7 +2311,7 @@ impl DefaultBucketUsecase {
|
||||
};
|
||||
|
||||
if let Err(err) = validate_lifecycle_config(&input_cfg, &rcfg).await {
|
||||
return Err(s3_error!(InvalidArgument, "{err}"));
|
||||
return Err(lifecycle_validation_error(&err));
|
||||
}
|
||||
|
||||
if let Err(err) = validate_transition_tier(&input_cfg).await {
|
||||
@@ -4030,6 +4045,70 @@ mod tests {
|
||||
assert_eq!(rules[2].id.as_deref(), Some("rule-2"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn put_bucket_lifecycle_validation_errors_keep_their_s3_code() {
|
||||
// The PUT path answers a schema-shape violation with MalformedXML and a
|
||||
// rejected value with InvalidArgument. Both categories are produced by
|
||||
// the real validator here, so the mapping cannot drift from it
|
||||
// (backlog#2201).
|
||||
let malformed = validate_lifecycle_config(
|
||||
&BucketLifecycleConfiguration {
|
||||
expiry_updated_at: None,
|
||||
rules: vec![LifecycleRule {
|
||||
status: ExpirationStatus::from_static(ExpirationStatus::ENABLED),
|
||||
expiration: Some(LifecycleExpiration {
|
||||
days: Some(1),
|
||||
..Default::default()
|
||||
}),
|
||||
abort_incomplete_multipart_upload: None,
|
||||
del_marker_expiration: None,
|
||||
filter: Some(s3s::dto::LifecycleRuleFilter {
|
||||
prefix: Some("logs/".to_string()),
|
||||
tag: Some(s3s::dto::Tag {
|
||||
key: Some("env".to_string()),
|
||||
value: Some("prod".to_string()),
|
||||
}),
|
||||
..Default::default()
|
||||
}),
|
||||
id: Some("two-predicates".to_string()),
|
||||
noncurrent_version_expiration: None,
|
||||
noncurrent_version_transitions: None,
|
||||
prefix: None,
|
||||
transitions: None,
|
||||
}],
|
||||
},
|
||||
&ObjectLockConfiguration::default(),
|
||||
)
|
||||
.await
|
||||
.expect_err("a Filter with two predicates is a schema violation");
|
||||
assert_eq!(*lifecycle_validation_error(&malformed).code(), S3ErrorCode::MalformedXML);
|
||||
|
||||
let invalid_value = validate_lifecycle_config(
|
||||
&BucketLifecycleConfiguration {
|
||||
expiry_updated_at: None,
|
||||
rules: vec![LifecycleRule {
|
||||
status: ExpirationStatus::from_static(ExpirationStatus::ENABLED),
|
||||
expiration: None,
|
||||
abort_incomplete_multipart_upload: None,
|
||||
del_marker_expiration: None,
|
||||
filter: None,
|
||||
id: Some("negative-count".to_string()),
|
||||
noncurrent_version_expiration: Some(s3s::dto::NoncurrentVersionExpiration {
|
||||
noncurrent_days: Some(30),
|
||||
newer_noncurrent_versions: Some(-1),
|
||||
}),
|
||||
noncurrent_version_transitions: None,
|
||||
prefix: None,
|
||||
transitions: None,
|
||||
}],
|
||||
},
|
||||
&ObjectLockConfiguration::default(),
|
||||
)
|
||||
.await
|
||||
.expect_err("a negative retention count is rejected");
|
||||
assert_eq!(*lifecycle_validation_error(&invalid_value).code(), S3ErrorCode::InvalidArgument);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_lifecycle_rule_status_rejects_invalid_status() {
|
||||
let rules = vec![LifecycleRule {
|
||||
|
||||
@@ -398,6 +398,12 @@ pub(crate) mod bucket {
|
||||
|
||||
lc.validate(lock_config).await
|
||||
}
|
||||
|
||||
/// The `std::io::ErrorKind` [`validate_lifecycle_config`] uses for a
|
||||
/// lifecycle document that violates the published schema shape, which
|
||||
/// the S3 boundary answers with `MalformedXML` (backlog#2201).
|
||||
pub(crate) const LIFECYCLE_MALFORMED_XML_ERROR_KIND: std::io::ErrorKind =
|
||||
crate::storage::storage_api::ecstore_bucket::lifecycle::lifecycle::LIFECYCLE_MALFORMED_XML_ERROR_KIND;
|
||||
}
|
||||
|
||||
pub(crate) mod lifecycle_contract {
|
||||
|
||||
Reference in New Issue
Block a user