Compare commits

..

7 Commits

Author SHA1 Message Date
houseme 4f29492806 fix(heal): cleanup consumed MRF replay journals
Do not retain Accepted or Merged replay intents as startup anchors after they have been handed to the heal manager. Only refused or still-pending replay records keep the journal on disk until a successor snapshot can persist them.

This keeps successor snapshots limited to the pending queue, which lets successful replay remove both authoritative and legacy journal paths and restores the crash-boundary tests around successor flush.

Co-Authored-By: heihutu <heihutu@gmail.com>

Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
(cherry picked from commit d5b8f49c9d)
2026-09-08 02:10:18 +08:00
houseme c93b9d6531 fix(error): merge equivalent api message branches
Combine the MaxVersionsExceeded and internal IO message branches so Clippy no longer flags identical if blocks while preserving the existing response messages.

Co-Authored-By: heihutu <heihutu@gmail.com>

Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
2026-09-08 00:27:21 +08:00
overtrue 8ca6dfc395 docs(testing): list the full GHSA-g8w9 regression set 2026-09-08 00:18:14 +08:00
overtrue d14cdedaf9 fix(s3): apply presigned signed-header rule to custom routes and harden parsing
Move the GHSA-g8w9-qw9q-fghr check to the first statement of S3Access::check, apply it in S3Router::check_access so admin, console, STS and extension routes that never reach the access hook enforce the same rule, read X-Amz-SignedHeaders with the exact key the verifier uses and treat a duplicate as signing nothing, and log the rejection as a warn event with the repository field shape. Add presigned GET, unsigned x-amz-copy-source and unsigned Content-Type e2e cases plus a router unit test; raise the security smoke floor to 26 and refresh the selection digests.
2026-09-08 00:18:14 +08:00
overtrue a51f8608bd fix(s3): reject unsigned x-amz headers on presigned requests
A SigV4 presigned URL only binds the headers listed in X-Amz-SignedHeaders, but the handlers applied every x-amz-* request header regardless. The holder of a presigned PutObject URL signed with SignedHeaders=host could add x-amz-tagging, x-amz-storage-class, x-amz-website-redirect-location, ACL, metadata, Object Lock or SSE headers and have them applied (GHSA-g8w9-qw9q-fghr). Reject such requests at the S3 access boundary with 403 AccessDenied and the AWS message "There were headers present in the request which were not signed"; x-amz-cf-id stays tolerated for CloudFront. SigV2 and header-signed SigV4 requests are unchanged.

Regression tests are named after the advisory (unit tests in rustfs/src/auth.rs, e2e in crates/e2e_test/src/presigned_negative_test.rs with a signed-tagging positive control); the security smoke floor rises to 20 and the e2e selection digests are refreshed for the two new cases.
2026-09-08 00:18:14 +08:00
Zhengchao An 7c85c72fd1 docs: scope agent guidance and consolidate review workflows (#7419) 2026-09-07 23:58:05 +08:00
Zhengchao An 27d167f7b6 ci(upgrade): run rc.5 multipart layout checks (#7421)
Wire the existing rc.5 multipart upgrade and diagnostic baseline tests into the upgrade matrix.
2026-09-07 23:58:02 +08:00
48 changed files with 1058 additions and 890 deletions
@@ -0,0 +1,33 @@
# Adversarial Review Shape
Use when root `AGENTS.md` triggers adversarial validation or for a substantial PR
review. Paths below are repository-relative. The root finding standard and
completion rule apply; selecting a lens does not require finding a defect.
Risk and review shape:
- **Exempt:** documentation, comments, formatting, or typos with no runtime,
build, test, or agent-execution effect.
- **Mechanical:** renames, moves, test/tooling-only changes, and agent-rule
changes. Run correctness and simplicity lenses.
- **Standard:** localized behavior changes. Run one integrated final-diff pass
covering correctness, simplicity, and test coverage; add only domain lenses
matched by the diff.
- **High risk / substantial PR review:** high risk includes locking,
erasure/quorum/heal, replication, multipart, RPC, lifecycle/tiering,
persistence/fsync, IAM/KMS/auth, cryptography, on-disk/on-wire formats, and
S3-visible semantics. Cover all applicable lenses using exactly two
independent reviewers when delegation is explicitly authorized. Split the
lenses between them. Otherwise perform two fresh sequential passes.
- **Outbound client defaults:** what `TargetClient`, `PutObjectOptions`, or
the remote SDK configuration sends to every replication or migration target
is high risk for every target class even when the change fixes one. Follow
the SOP in `docs/postmortems/2026-09-03-replication-checksum-default-regression.md`:
run the outbound target matrix, document each new env knob in the same PR,
and list verified and unverified target classes in the PR Impact section.
Available domain lenses are security, concurrency/durability, compatibility,
and performance. Select `.agents/skills/adversarial-validation/SKILL.md` for an
explicit adversarial request, a high-risk change, or a substantial PR review;
then read only its matching role references. A routine standard pass does not
load the playbook unless the reviewer needs a RustFS-specific probe.
+65
View File
@@ -0,0 +1,65 @@
# Implementation Rules
Applies when changing code or running artifact-heavy work. Paths below
are repository-relative. Read only the relevant sections during read-only review.
## Worktree and Disk Hygiene
- Start implementation from the latest `origin/main` and confirm the requested
change is not already present.
- An existing clean, isolated task worktree is sufficient. Create another
worktree only when the current checkout is shared, dirty with unrelated work,
or belongs to another task.
- Never commit from a shared checkout.
- Use a task-specific branch named `<type>/<topic>`, such as `fix/...`,
`feat/...`, `test/...`, or `docs/...`, unless the user specifies a name.
- Do not include agent, tool, contributor, account, or organization names in
branch names.
- Push to the user-requested remote or the repository's configured push remote.
Do not hard-code or infer a remote from an account name.
- Check free space before artifact-heavy builds, tests, coverage, or downloads.
Re-check before a broad gate when space is tight.
- Remove only task-owned temporary/build artifacts. Never delete another task's
worktree or uncommitted data.
- At handoff, mention disk or cleanup details only when they affected execution
or artifacts/worktrees remain intentionally.
## Change Style
- Preserve existing control flow unless changing it is required for correctness.
- Prefer a direct local edit over new files, wrappers, managers, or speculative
abstractions.
- Add a helper only when it removes current duplication, names a real domain
boundary, or isolates a non-trivial invariant.
- Remove an in-scope path superseded by the change. If compatibility requires it,
adapt at the boundary to one canonical core and use the repository's
`RUSTFS_COMPAT_TODO` policy.
- Comments explain non-obvious invariants or reasons. Do not narrate code or
record change history.
- Mention unrelated problems when useful; do not fix them in a narrow task.
## Reuse and Boundary Rules
- Before adding helpers, constants, fixtures, or wrappers, search the touched
crate, the domain-owning crate, `crates/utils`, `crates/common`, and relevant
direct dependencies.
- Reuse requires matching semantics: normalization, error types, deadlines,
durability, and compatibility must fit the call site. A narrowly named local
helper is better than forced reuse with different semantics.
- Validate untrusted input at its trust boundary, then trust the validated type.
Values crossing disk, RPC, persistence, or version boundaries remain
untrusted at every consumer.
- Re-check boundary values immediately before destructive actions such as
delete, overwrite, or quorum decisions.
- Every new branch needs a concrete triggering input/state. For decoded or peer
data, corruption and mixed-version input are valid triggers.
- Required values must return a typed error when absent or corrupt; do not use a
default that converts corruption into a plausible result.
- Attach error context once where it is actionable. Do not erase typed errors
below aggregation or quorum layers.
## Naming
Use Rust API naming: `SCREAMING_SNAKE_CASE` constants/statics, `snake_case`
functions/variables, and `PascalCase` types. Do not rename unrelated existing
violations.
+51
View File
@@ -0,0 +1,51 @@
# Git and Pull Request Rules
Applies to commits, pushes, PRs, and issue/discussion actions. Paths below are
repository-relative. User authorization and root `AGENTS.md` still govern scope.
## Final PR Preflight
Before creating or updating a PR, reuse completed review and verification:
- Verify the actual base (normally `origin/main`) and the complete task diff,
including file names and whitespace. Exclude secrets, logs, generated
artifacts, and unrelated edits. Retain an existing PR's base unless requested.
- Confirm the final diff passed the root verification tier; fix task-owned
failures and run missing scoped checks. Report unresolved required checks or
authority without expanding the task. Do not start another general review.
- Keep the English Conventional Commit title at most 72 characters. Use the
template headings, actual checks, material risks, and rollback notes.
- Immediately before writing to GitHub, confirm the head and task diff are
unchanged. Rerun only checks invalidated by edits or relevant state changes.
## Pull Request Lifecycle
- Creating or updating a PR includes one immediate snapshot of checks,
mergeability, reviews, and unresolved threads.
- Unless the user explicitly requests monitoring, a release workflow requires
it, or an automation already owns it, hand off after the PR is open with the
current state and next event to watch. Do not delay ordinary handoff with
fixed quiet-period sleeps.
- For requested monitoring, use event-driven or bounded waits. Report only state
changes, actionable failures, or a meaningful prolonged delay.
- Investigate failures/comments before changing code. Fix task-attributable
issues, rerun affected verification, push, reply or resolve the thread, then
resume the requested monitor.
- Never merge without required reviewer approval or explicit authority.
- After an observed merge, verify the commit reached the base, then clean the
task worktree/branch when safe. Preserve unmerged work for closed PRs unless
deletion was explicitly authorized.
## Git and PR Baseline
- Follow Conventional Commits; keep the subject at most 72 characters.
- Source comments, commits, PR titles, and PR bodies are in English.
- Keep every heading from `.github/pull_request_template.md`; use `N/A` where
needed and include commands actually run.
- Use `--body-file` for multiline `gh pr create`/`gh pr edit` content.
- PR/issue/discussion content must not contain the literal sequence `\n` or
hard-wrapped prose paragraphs.
- Do not include local absolute paths or tool-specific labels/prefixes in GitHub
content.
- Resolve review threads after the underlying issue is fixed. If declining a
suggestion, reply with a short evidence-based reason.
+11 -8
View File
@@ -1,11 +1,11 @@
---
name: adversarial-validation
description: Review a final RustFS diff adversarially when the user requests adversarial review, the root AGENTS.md classifies the change as high risk, or a substantial PR is being reviewed. Do not use for ordinary questions, diagnosis, planning, status, documentation-only work, or routine low-risk implementation.
description: Review RustFS diffs or designs for explicit adversarial requests, high-risk changes under the repository review policy, or substantial PR reviews. Skip ordinary questions, diagnosis, planning, status, routine low-risk implementation, and prose with no execution effect.
---
# RustFS Adversarial Validation
Use the risk tier and review shape defined in the root `AGENTS.md`. This skill
Use the [repository risk tiers and review shape](../../references/adversarial-validation.md). This skill
routes a review to RustFS-specific probes without loading unrelated domains.
## Select Lenses
@@ -31,15 +31,18 @@ adversarial review.
## Review Protocol
1. Freeze the exact final diff/head and list the selected lenses.
2. Run the review shape required by root `AGENTS.md`.
1. Freeze the exact final diff/head (or the design under review) and list the
selected lenses.
2. Run the review shape required by the repository risk tier.
3. For each selected lens, either report a concrete finding or a null verdict
naming the attacks performed.
4. A finding needs `file:line`, a triggering input/state/interleaving, the wrong
outcome, and a focused fix or missing regression check.
5. Fix or rebut every finding with code-path, test, or invariant evidence.
4. Apply root `AGENTS.md`'s finding standard. Test each candidate against callers,
existing coverage, and invariants before accepting it; an adversarial role
does not have to produce a defect.
5. Fix or rebut supported findings with code-path, test, or invariant evidence.
6. After a non-trivial edit, rerun only lenses affected by that edit against the
new exact diff.
Do not turn a null verdict into a long checklist. Record concise evidence that
the relevant failure classes were attacked.
the relevant failure classes were attacked, then stop under the root completion
rule. Keep the required per-lens verdicts for high-risk PRs.
@@ -3,9 +3,10 @@
- For every behavior claim, name the focused test/check that fails if the
changed hunk is reverted. If none is practical, require the reason and
residual risk.
- Confirm tests exercise the real production path and assert returned values,
exact bytes, stored state, or the specific error variant—not only success,
`is_err()`, or no panic.
- Confirm tests exercise the real production path and distinguish the intended
behavior from the named regression. A success, `is_err()`, or no-panic check
can be sufficient when that is the actual contract; require exact values,
bytes, state, or error variants when those distinctions matter to the change.
- For new flags/modes, verify each branch and ask which test fails if the branch
is inverted.
- For new error propagation, inject the failure and assert the caller observes
+6 -4
View File
@@ -1,12 +1,13 @@
---
name: arch-checks
description: Resolve failures from the repository's architecture guard scripts — check_layer_dependencies.sh, check_architecture_migration_rules.sh, check_unsafe_code_allowances.sh, check_logging_guardrails.sh, check_doc_paths.sh. Use when make pre-commit / pre-pr or CI fails on one of these checks.
description: Diagnose failures from check_layer_dependencies.sh, check_architecture_migration_rules.sh, check_unsafe_code_allowances.sh, check_logging_guardrails.sh, check_doc_paths.sh, or check_no_planning_docs.sh. Use when one of these guards fails, not for every architecture question or documentation edit.
---
# Architecture Guard Checks
All five run in `make pre-commit` / `make pre-pr` and in CI. Fix the cause;
never weaken a check to get green.
Read only the section for the failing guard. Use `.config/make/` and the current
workflow to verify its wiring; not every guard is part of every gate. Fix the
cause and rerun the failed guard; never weaken a check to get green.
## `check_layer_dependencies.sh` — layer DAG in `rustfs/src`
@@ -54,7 +55,8 @@ Instruction docs (`AGENTS.md`, `CLAUDE.md`, `ARCHITECTURE.md`) and every
Markdown file under `docs/` (architecture, operations, testing, index) must not
reference repo file paths that no longer exist. If your refactor moved code,
update the docs that point at it — the error message lists `doc -> stale-path`
pairs. Cite paths plus symbol names, never line numbers (see `docs/README.md`).
pairs. In durable docs, cite paths plus symbol names rather than line numbers
(see `docs/architecture/README.md`). Review findings still need `file:line`.
## `check_no_planning_docs.sh`
@@ -8,19 +8,11 @@ description: Review a commit, PR, or merged patch when the user requests ordinar
Use this skill for an ordinary requested review. If the root policy or user calls
for adversarial validation, use `adversarial-validation` instead of running both.
## Quick Start
1. Read the scope: commit, PR, patch, or file list.
2. Map each changed area by risk and user impact.
3. Inspect each risky change in context.
4. Report findings first, ordered by severity.
5. Close with residual risks and verification recommendations.
## Core Workflow
### 1) Scope and assumptions
- Confirm change source (diff, commit, PR, files), target branch, language/runtime, and version.
- If context is missing, state assumptions before deeper analysis.
- Derive the change source, target branch, and relevant runtime/version from the
supplied diff and metadata. Ask only when missing context could change the verdict.
- Focus only on requested scope; avoid reviewing unrelated files.
### 2) Risk map
@@ -40,43 +32,20 @@ for adversarial validation, use `adversarial-validation` instead of running both
- unchecked assumptions and null/empty/error-path handling
- stale tests, fixtures, and configs
- hidden coupling to shared helpers/constants/features
- If a point is uncertain, mark it as an open question instead of guessing.
- Apply root `AGENTS.md`'s finding standard: try to disprove a candidate before
reporting it. Mention an unresolved question only when it could materially
change the verdict; do not fill the report with speculative possibilities.
#### Rust-specific checks (apply to all Rust changes)
#### Rust-specific checks
Run the full checklist in [rust-code-quality](../rust-code-quality/SKILL.md) — the canonical Rust review checklist for the unwrap/casting/cloning/locking/recursion/error-type/serde/test rules and the reuse-and-necessity checks (duplicated helpers, defensive branches without a nameable trigger, redundant error wrapping). Do not restate those rules here; carry its P0P3 ratings over unchanged and use this skill's output format.
For changed Rust behavior, use the matching sections of [rust-code-quality](../rust-code-quality/SKILL.md). Reuse checks already performed by the selected review workflow. Comment-only or formatting-only Rust diffs do not require the full Rust checklist. Carry its P0P3 ratings over unchanged and use this skill's output format.
### 4) Findings-first output
- Order findings by severity:
- P0: critical failure, security breach, or data loss risk
- P1: high-impact regression
- P2: medium risk correctness gap
- P3: low risk/quality debt
- For each finding include:
- Severity
- `path:line` reference
- concise issue statement
- impact and likely failure mode
- specific fix or mitigation
- validation step to confirm
- If no issues exist, explicitly state `No findings` and why.
- Order supported findings by P0P3 severity; preserve the Rust ratings above.
Include `path:line`, the failure and impact, a focused fix, and its validation.
- If no supported issues remain, state `No findings` with the reviewed scope and
any material verification limitation. Do not append optional improvements to
make a clean review look productive.
### 5) Close
- Report assumptions and unknowns.
- Suggest targeted checks (tests, canary checks, logs/metrics, migration validation).
## Output Template
1. Findings
2. No findings (if applicable)
3. Assumptions / Unknowns
4. Recommended verification steps
## Finding Template
- `[P1] Missing timeout for downstream call`
- Location: `path/to/file.rs:123`
- Issue: ...
- Impact: ...
- Fix suggestion: ...
- Validation: ...
Close after the required review. Recommend additional verification only for an
identified unresolved risk or required gate; reuse evidence for unchanged code.
+28 -32
View File
@@ -1,6 +1,6 @@
---
name: issue-triage
description: Triage a GitHub issue — determine if it is already fixed, needs implementation, or should be closed. Searches related commits and PRs, verifies implementation status, and posts a triage comment or closes the issue. Use when the user provides an issue URL and asks whether it can be closed or needs work.
description: Assess whether a GitHub issue is fixed, needs implementation, or can be closed by checking related work and current code. Use for issue completion/triage requests. Status questions are read-only; comment, close, or change labels only when the conversation authorizes that action.
---
# Issue Triage
@@ -20,6 +20,8 @@ Read the issue body to understand what was requested. Extract:
- Any linked PRs or commits mentioned in the body or comments.
- Any checklist items or sub-issues.
Resolve the issue repository and implementation repository separately (for example, `rustfs/backlog` tracks work in `rustfs/rustfs`). Pass the implementation repository explicitly to PR queries; the current checkout may belong to another repository.
### 2. Search for related work
Search git history for commits referencing the issue:
@@ -29,26 +31,29 @@ git log --oneline --all --grep="<N>" | head -30
Search for related PRs:
```bash
gh pr list --search "fixes #<N> OR closes #<N> OR #<N>" --state all --json number,title,state,mergedAt
gh pr list --repo <implementation-repo> --search "<issue-url>" --state all --json number,title,state,mergedAt
```
Also search qualified issue references and subject keywords; for same-repository
issues, include `#<N>`. Follow explicit links even without a text match. A search
page with no match does not prove the work is absent.
If the issue mentions specific PRs, check their status:
```bash
gh pr view <PR_N> --json state,mergedAt,title
gh pr view <PR_N> --repo <implementation-repo> --json state,mergedAt,title,mergeCommit,baseRefName
```
### 3. Verify implementation
For each linked or related PR that is merged, verify the fix is actually present on the current main branch:
Fetch the implementation repository's current base branch. For each merged candidate, verify its merge commit is present and inspect the current code for the claimed behavior; a commit message match alone is not proof:
```bash
git log --oneline main | grep -i "<keyword>"
# or
git log --oneline main --grep="<PR_N>"
git fetch <implementation-remote> <base-branch>
git merge-base --is-ancestor <merge-commit> <implementation-remote>/<base-branch>
```
If the issue describes a specific defect, check the relevant code to confirm the fix is in place:
If the issue describes a specific defect, inspect the fetched base's code rather than assuming the current checkout contains it:
```bash
grep -n "<pattern>" crates/<relevant>/src/<file>.rs
git show <implementation-remote>/<base-branch>:crates/<relevant>/src/<file>.rs
```
For issues with checklists, verify each item individually. If sub-items are tracked as separate issues, check those too:
@@ -58,13 +63,15 @@ gh issue view <SUB_N> --repo <owner/repo> --json state
### 4. Determine verdict
- **All items fixed and merged**: Close with a summary comment listing what was fixed and which PRs.
- **Some items fixed, some remaining**: Comment with status of each item. Do not close.
- **Not yet implemented**: Comment with a summary of what remains. Do not close.
- **Superseded or no longer relevant**: Close with explanation.
- **All items fixed and merged**: Recommend closing; name the verified PRs and behavior.
- **Some items fixed, some remaining**: Keep open; report each remaining item.
- **Not yet implemented**: Keep open; report what remains.
- **Superseded or no longer relevant**: Recommend closing with evidence.
### 5. Take action
For a status-only request, return the assessment without GitHub writes. If commenting, closing, or label edits are authorized, perform only those actions; do not ask again for authority already given. Prepare the final assessment before asking for any missing authority. Write `rustfs/backlog` issue content in Chinese.
Close with comment:
```bash
gh issue close <N> --repo <owner/repo> --comment "<body>"
@@ -75,9 +82,9 @@ Comment without closing:
gh issue comment <N> --repo <owner/repo> --body-file /tmp/triage.md
```
Update issue labels if needed:
Update labels only when label changes are authorized, using existing repository labels; never add tool-specific labels:
```bash
gh issue edit <N> --repo <owner/repo> --add-label "completed" --remove-label "needs-triage"
gh issue edit <N> --repo <owner/repo> --add-label "<existing-label>"
```
Always use `--body-file` for multiline content, never inline `--body`.
@@ -85,27 +92,16 @@ Always use `--body-file` for multiline content, never inline `--body`.
### 6. Handle multi-issue batches
When the user asks to check multiple issues (e.g., "check all issues by user X" or "scan backlog for closable issues"):
1. List the issues: `gh issue list --repo <repo> --author <user> --state open --json number,title,updatedAt`
1. List the full requested scope with pagination (for example `gh api --paginate 'repos/<repo>/issues?state=open&per_page=100'`, excluding entries with `pull_request`). Add an author filter only when the user requested one; the default page/limit is not evidence that all issues were checked.
2. For each issue, run steps 1-5 above.
3. Report a summary table of all triaged issues with verdicts.
## Output format
## Report
### Issue Triage: #<N> — <title>
**State**: OPEN / CLOSED
**Linked PRs**: <list with merge status>
#### Assessment
<what was requested vs what is implemented>
#### Verdict
- Close — all items resolved by <PR list>
- Keep open — <remaining items>
- Not started — <what needs to be done>
#### Action taken
- Closed with comment / Commented / No action
Identify the issue and current state, verified implementation/PR evidence,
remaining items, verdict, and action actually taken. Use a table for batches;
a single issue does not require a heading for each field. Follow step 4's
verdicts without repeating the assessment in another template.
## Notes
@@ -1,6 +1,6 @@
---
name: plugin-contract-guard
description: Invariants and change procedure for the target-plugin / extension system — plugin manifests, admin plugin/extension catalog and instance APIs, secret redaction, external-plugin install policy. Use when editing crates/targets (manifest, plugin, control_plane, catalog, runtime), crates/extension-schema, or rustfs/src/admin plugin_contract.rs / plugins_*.rs / extensions.rs / target_descriptor.rs.
description: Guard changes to target-plugin manifests, extension schemas, admin catalog/instance contracts, secret redaction, and external-plugin install policy. Use when a diff changes those contracts in crates/targets, crates/extension-schema, or admin plugin/extension handlers; path membership alone, comments, and unrelated runtime internals do not trigger it.
---
# Plugin & Extension Contract Guard
@@ -1,46 +0,0 @@
---
name: pr-creation-checker
description: Perform the final RustFS PR preflight and draft compliant English title/body metadata immediately before creating or updating a PR. Do not use during implementation or as a second general code review.
---
# PR Creation Checker
Use this skill only at the PR boundary. Reuse completed diff review and
verification evidence; do not reread the repository or rerun equivalent checks.
## Preflight
1. Confirm the branch is based on current `origin/main` and contains only the
intended task diff.
2. Inspect `git diff --stat`, `git diff --check`, and changed file names for
secrets, logs, generated artifacts, or unrelated edits.
3. Confirm the checks selected by root `AGENTS.md` passed on the final diff.
Do not replace focused behavioral tests with a generic gate or rerun checks
already covered by an unchanged umbrella run.
4. Read `.github/pull_request_template.md`. Consult `Makefile`, `.config/make/`,
or CI only when the required command/current gate is uncertain.
5. Return `BLOCKED` for an unclean scope, missing required evidence, failed
required checks, or non-compliant metadata.
## Metadata
- Title: English Conventional Commit, at most 72 characters, with no tool
prefix.
- Body: English, exact template headings, `N/A` where needed, concise rationale,
actual verification commands, and material risks/rollback notes.
- Use repository-relative paths; never include local absolute paths.
- Keep prose paragraphs on one logical line and never include the literal
sequence `\n`.
- Use a temporary body file with `gh pr create --body-file` or
`gh pr edit --body-file`; never pass multiline Markdown inline.
## Output
- Status: `READY` or `BLOCKED`.
- Title.
- Complete PR body.
- Verification commands and results.
- Risks or `N/A`.
Immediately before the GitHub write, repeat only the five preflight checks above
against the final head.
@@ -1,4 +0,0 @@
interface:
display_name: "PR Creation Checker"
short_description: "Draft RustFS-ready PRs with checks, template, and blockers."
default_prompt: "Use $pr-creation-checker for final PR preflight and compliant English title/body metadata."
+30 -84
View File
@@ -1,147 +1,93 @@
---
name: pr-review
description: Review a GitHub PR end-to-end from a URL or number — fetch metadata, inspect the diff, run multi-role adversarial review, check CI status, and post the review comment. Use when the user provides a PR link and asks to review it.
description: Review a GitHub PR from a URL or number using its actual base/head and risk-appropriate code review. Use when the user asks for a PR review, not a status lookup or PR wording edit. Publish a review only when authorized; delegation and monitoring follow the requested scope and root AGENTS.md.
---
# PR Review
Use this skill when the user provides a GitHub PR URL or number and asks to review it. This covers the full review lifecycle: data gathering, code review, CI verification, and posting the result.
Use this skill for PR context and review delivery. An ordinary review request is read-only unless the conversation also authorizes posting or fixes. Reuse that authorization without asking again; prepare the review before requesting any missing publication approval.
## Prerequisites
- Read `AGENTS.md` for the repository's adversarial validation policy and change-style rules.
- The `adversarial-validation` skill handles the review role playbooks; this skill orchestrates the workflow around it.
- Follow root `AGENTS.md`; classify risk with the [review policy](../../references/adversarial-validation.md) and consult relevant [change-style and boundary rules](../../references/implementation.md).
- Select `code-change-verification` for ordinary review or `adversarial-validation` for explicitly adversarial, substantial, or high-risk review; do not run both on the same diff.
## Workflow
### 1. Gather PR context
```bash
gh pr view <N> --json title,author,state,body,additions,deletions,changedFiles,commits,baseRefName,headRefName
gh pr diff <N> --name-only
gh pr view <N> --repo <owner/repo> --json title,author,state,body,additions,deletions,changedFiles,commits,baseRefName,headRefName,baseRefOid,headRefOid
gh pr diff <N> --repo <owner/repo> --name-only
```
Read the PR body and linked issues to understand the change's purpose. If the PR references an issue, fetch that too:
```bash
gh issue view <ISSUE> --json title,body,state
gh issue view <ISSUE> --repo <issue-owner/repo> --json title,body,state
```
### 2. Fetch the diff and classify the change
```bash
git fetch origin pull/<N>/head:pr-<N>
git diff main...pr-<N> --stat
git fetch <repo-remote> <baseRefName> refs/pull/<N>/head
git diff <baseRefOid>...<headRefOid> --stat
```
Classify the change by risk tier (per AGENTS.md):
- **Exempt**: docs/comments/instruction-only, formatting, typos.
- **Mechanical**: renames, file moves, test-only or tooling changes.
- **Standard** (default): any behavior change.
- **High risk**: locking, erasure coding, quorum/heal, replication, multipart, RPC, lifecycle/tiering, metadata formats, persistence/fsync, IAM/KMS/auth, on-disk/on-wire formats, S3 API-visible behavior.
Resolve `<repo-remote>` to the PR repository; do not assume the current checkout's `origin` or `main` matches. Record the exact base/head used. If either moved during fetching, refresh the snapshot before reviewing. Classify using the repository review policy; instruction changes that affect agent execution are mechanical, not exempt.
### 3. Cluster changed files and delegate review
### 3. Review the changed behavior
Group the changed files into logical clusters (by crate or functional area). For each cluster, spawn a subagent with a focused review prompt that includes:
- The cluster's changed files and their diffs.
- The applicable adversarial role probes (from the `adversarial-validation` skill).
- The repository's AGENTS.md rules relevant to that domain.
Group files by functional area to trace callers and invariants. Use the root risk tier's review shape and only matching lenses. File count does not authorize delegation. When delegation is explicitly authorized, high-risk/substantial reviews use exactly two independent reviewers with the applicable lenses split between them; otherwise use two fresh sequential passes. Reviewers do not spawn further agents.
For standard-tier changes: correctness adversary + simplicity adversary + test-coverage skeptic, plus every role whose domain the diff touches.
For high-risk changes: run all seven roles.
Each subagent must produce findings (concrete failure scenario with file:line) or a null report ("attacked X, Y, Z — no break found").
Findings need a concrete failure scenario with `file:line`; a null verdict briefly names the relevant probes. Reuse existing evidence and choose local checks from the final diff under the root verification policy.
### 4. Check CI status
```bash
gh pr checks <N>
gh pr checks <N> --repo <owner/repo>
```
If any checks fail, investigate:
Investigate a failed check when it bears on a finding or the user requested CI diagnosis/merge readiness:
```bash
gh run view --log-failed --job=<JOB_ID>
gh run view --repo <owner/repo> --log-failed --job=<JOB_ID>
```
Determine whether failures are pre-existing (on main), flaky, or caused by the PR.
Use current evidence to distinguish pre-existing, flaky, and PR-caused failures. Do not classify them by guesswork or turn a code-only review into unrelated CI repair.
### 5. Synthesize findings
Combine all subagent findings into a structured review:
- **Summary**: one-paragraph overview of the change and overall assessment.
- **Findings**: each finding with severity (critical/major/minor/nit), file:line, concrete failure scenario, and suggested fix.
- **CI status**: pass/fail with notes on any failures.
- **Verdict**: APPROVE, REQUEST_CHANGES, or COMMENT.
Report the PR, reviewed base/head, and risk tier, then summarize the assessment.
Use the selected review's P0P3 ratings and root finding standard: supported
findings with `file:line`, failure scenario, and fix, or `No findings`.
State the observed check status, including pending or unavailable checks, and
the verdict (`APPROVE`, `REQUEST_CHANGES`, or `COMMENT`). Do not infer a pass
from missing checks or add style nits to populate a clean review.
### 6. Post the review
Write the review body to a temp file and post via CLI:
Only when posting is authorized, write the review body to a temp file and post via CLI. Refresh the PR head first; if it changed, review the delta and update the verdict before posting:
```bash
# Request changes
gh pr review <N> --request-changes --body-file /tmp/pr_review.md
gh pr review <N> --repo <owner/repo> --request-changes --body-file /tmp/pr_review.md
# Approve
gh pr review <N> --approve --body-file /tmp/pr_review.md
gh pr review <N> --repo <owner/repo> --approve --body-file /tmp/pr_review.md
# Comment only (no verdict)
gh pr review <N> --comment --body-file /tmp/pr_review.md
gh pr review <N> --repo <owner/repo> --comment --body-file /tmp/pr_review.md
```
For inline comments on specific lines, use the GitHub API:
```bash
cat > /tmp/pr_review.json <<'EOF'
{
"body": "review body",
"event": "REQUEST_CHANGES",
"comments": [
{
"path": "crates/foo/src/bar.rs",
"line": 42,
"body": "finding description"
}
]
}
EOF
gh api --method POST /repos/{owner}/{repo}/pulls/<N>/reviews --input /tmp/pr_review.json
```
For authorized inline comments, use [the submission example](references/posting.md).
Always use `--body-file` or `--input`, never inline multiline `--body`.
### 7. Handle follow-up
If the review requests changes:
- Monitor for new commits: `gh pr view <N> --json commits`
- Re-review changed files only: `git diff pr-<N>..origin/pull/<N>/head`
- Update the review when findings are addressed.
If CI was failing due to pre-existing main breakage:
- Comment on the PR noting the failure is pre-existing.
- Suggest updating the branch: `gh pr update-branch <N>`
## Output format
### PR Review: #<N> — <title>
**Author**: <author>
**Risk tier**: exempt | mechanical | standard | high-risk
**Changed files**: <count> across <cluster count> clusters
#### Summary
<one-paragraph overview>
#### Findings
| Severity | Location | Finding |
|----------|----------|---------|
| critical | file:line | concrete failure scenario |
#### CI Status
- All checks pass / Failing: <details>
#### Verdict
APPROVE / REQUEST_CHANGES / COMMENT
Follow the [PR lifecycle](../../references/pull-requests.md) and any explicit monitoring request. For follow-up, fetch the new head and compare the recorded reviewed SHA with the new SHA; revisit affected callers and findings. Never use an unfetched `origin/pull/<N>/head` ref as evidence. Update the posted review or resolve addressed threads only within existing authorization.
## Notes
- The user may ask for review in Chinese; respond in the same language but keep the review body in English per AGENTS.md rules.
- When the user asks for "多角色对抗 review", run the full adversarial validation protocol — this skill's step 3 covers that.
- If the PR is from a fork, check `maintainerCanModify` before attempting to push fixes.
- For very large PRs (>50 files), cluster aggressively and delegate in parallel to keep review time reasonable.
- For very large PRs, batch the review by functional area while keeping the same bounded review shape.
@@ -0,0 +1,22 @@
# Inline PR Review Submission
Read only when an inline review is authorized. Recheck the PR head before posting and bind the review to the reviewed commit.
For inline comments on specific lines, use the GitHub API:
```bash
cat > /tmp/pr_review.json <<'EOF'
{
"commit_id": "<reviewed-head-sha>",
"body": "review body",
"event": "REQUEST_CHANGES",
"comments": [
{
"path": "crates/foo/src/bar.rs",
"line": 42,
"body": "finding description"
}
]
}
EOF
gh api --method POST /repos/{owner}/{repo}/pulls/<N>/reviews --input /tmp/pr_review.json
```
+16 -23
View File
@@ -1,6 +1,6 @@
---
name: rust-code-quality
description: Run a focused Rust quality review when the user requests one, when reviewing a Rust PR/commit, or when another selected review workflow delegates Rust-specific checks. Do not auto-load for every implementation edit.
description: Run a focused Rust quality review when the user requests one or a selected review workflow needs Rust-specific checks for changed behavior. Do not auto-load for every implementation edit, comment-only or formatting-only Rust diff, or repeat an already completed review.
---
# Rust Code Quality Gate
@@ -8,12 +8,18 @@ description: Run a focused Rust quality review when the user requests one, when
Use this skill for a dedicated Rust review to cover rules that `cargo clippy`
does not catch.
Search matches and checklist items are candidates, not findings. Apply the root
finding standard; distinguish a demonstrated bug, an explicit rule violation,
and an optional preference. P2/P3 suggestions do not need to be invented or
included in an otherwise clean correctness review.
## Quick Start
1. Identify changed `.rs` files.
2. Run automated checks on changed files.
3. Run manual review checklist on the diff.
4. Resolve or rebut every finding with evidence; P0/P1 findings cannot be deferred.
2. Run the matching candidate searches on changed files.
3. Apply the manual checklist sections whose behavior the diff touches.
4. Report or rebut every finding with evidence; P0/P1 findings block approval.
Fix them when implementation is authorized; a read-only review reports them.
## Automated Checks
@@ -35,7 +41,7 @@ rg -n 'Result<.*String>' <changed-files>
rg -n 'Box<dyn.*Error' <changed-files>
# 5. println/eprintln in production
rg -n 'println!\|eprintln!' <changed-files>
rg -n 'println!|eprintln!' <changed-files>
# 6. Ordering::Relaxed usage (verify each is intentional)
rg -n 'Ordering::Relaxed' <changed-files>
@@ -80,7 +86,7 @@ For the Rust diff under review, verify:
- [ ] Test volume and line count are never treated as production-code growth
### Serde
- [ ] Structs from untrusted input have `#[serde(deny_unknown_fields)]`
- [ ] Structs from untrusted input reject unknown fields where the compatibility contract permits; otherwise validate security-critical fields explicitly and test the supported input shape
- [ ] `#[serde(default)]` not used on security-critical fields without validation
### Code Hygiene
@@ -104,20 +110,7 @@ For the Rust diff under review, verify:
## Output Template
```
## Rust Code Quality Report
### Automated Scan
- unwrap/expect candidates inspected: N
- numeric-cast candidates inspected: N
- error-type candidates inspected: N
- output-macro candidates inspected: N
### Findings
- [P1] `path:line` — description
- Fix: ...
- Validation: ...
### Verdict
PASS / BLOCKED (list blocking findings)
```
Use the calling review's output format. For a standalone review, report supported
findings with severity, location, impact, fix, and validation, or `No findings`.
Include only material unverified checks. Candidate counts are not a quality
metric and do not need a separate scan report.
+13 -104
View File
@@ -4,9 +4,14 @@ description: "Run the end-to-end RustFS console gate, version bump, preview vali
---
# RustFS Release Publish (preview-validated pipeline)
This skill orchestrates a full release. It wraps `rustfs-release-version-bump` (which only edits version files and opens the PR) with a mandatory preview-tag validation loop before the final tag is published.
This skill orchestrates a full release. It wraps `rustfs-release-version-bump` (invoked here with the authorized commit/push/PR scope) with a mandatory preview-tag validation loop before the final tag is published.
Core design: **version files never carry a `-preview.N` suffix**. The preview suffix exists only in tag names. A preview tag creates a visible GitHub Release marked Prerelease and uploads versioned assets, but it never becomes GitHub Latest and never updates `*-latest`, `latest.json`, R2, Docker, or Helm channels. That Release is temporary: `build.yml` deletes it automatically once the final tag's Release is published, so the Releases page ends up carrying deliverables only while the `-preview.N` tags stay behind as the traceability record. This works because the binary self-reports the git tag it was built from (`build::TAG` via shadow_rs, see `rustfs/src/config/cli.rs` `SHORT_VERSION`), and `build.yml` derives artifact names and preview classification from the tag name — Cargo.toml's version is only a no-tag fallback. Therefore the preview tag and the final tag can (and MUST) point at the exact same commit: what you validated is byte-for-byte the source that ships.
The binary reports its build tag (`build::TAG` via shadow_rs; `SHORT_VERSION` in
`rustfs/src/config/cli.rs`), and `build.yml` derives asset names and preview
classification from that tag. Cargo.toml supplies only the no-tag fallback.
Preview and final tags must therefore share the validated source commit;
their tag-dependent version and asset names differ. The channel and cleanup
constraints are defined once under Preview tag naming and Hard rules below.
Pipeline shape:
@@ -29,9 +34,9 @@ On validation failure: fix lands on main via normal PR (version files are alread
- Final target version, for example `1.0.0-beta.10`.
- Preview iteration `N` (default: next unused preview tag for that target; check with `git tag -l '<target>-preview.*'` after `git fetch --tags`).
If the target version is missing or ambiguous, stop and ask before doing anything (see the semver gate below).
If the target version is missing or ambiguous, collect the current release/tag baseline and ask before version edits or publication. Continue independent read-only preflight while the answer is pending (see the semver gate below).
## Semver gate — confirm the target version before touching anything
## Semver gate — resolve the target before version edits or publication
Versions follow [SemVer 2.0.0](https://semver.org/). Precedence reminder:
@@ -43,7 +48,7 @@ Numeric prerelease identifiers compare numerically (`beta.9 < beta.10`), not lex
Rules:
- A request like "发个版" / "release the next version" without an exact version string is ALWAYS ambiguous. Derive the current latest tag (`git tag --sort=-v:refname | head`), then ask the user to choose via AskUserQuestion with concrete candidates, e.g. from `1.0.0-beta.10`: next prerelease `1.0.0-beta.11`, promote to `1.0.0-rc.1`, promote to stable `1.0.0`. Never guess between these — they have very different meanings (channel promotion vs. iteration) and different CI classification consequences.
- A request like "发个版" / "release the next version" without an exact version string is ALWAYS ambiguous. Derive the current latest tag (`git tag --sort=-v:refname | head`), then ask the user to choose with concrete candidates, e.g. from `1.0.0-beta.10`: next prerelease `1.0.0-beta.11`, promote to `1.0.0-rc.1`, promote to stable `1.0.0`. Never guess between these — they have very different meanings (channel promotion vs. iteration) and different CI classification consequences.
- After a stable `X.Y.Z` exists, the next version must state which component bumps: patch `X.Y.(Z+1)` for fixes only, minor `X.(Y+1).0` for backward-compatible features, major `(X+1).0.0` for breaking changes. If the user names a bump type but not a number, compute it from the latest stable tag and echo the exact resulting version back for confirmation.
- Echo the final confirmed version string verbatim in your first status report; every later phase must use exactly that string. If at any point the user's wording and the confirmed version diverge, stop and re-confirm.
@@ -77,58 +82,7 @@ Rules:
### Console release gate
Complete this gate before changing any RustFS version file or creating any RustFS tag. RustFS `build.yml` downloads the asset returned by `repos/rustfs/console/releases/latest`, so a successful Console build alone is insufficient.
1. Read the latest published Console tag and compare it with Console `main`:
```bash
CONSOLE_REPO="rustfs/console"
CONSOLE_LATEST=$(gh api "repos/${CONSOLE_REPO}/releases/latest" --jq .tag_name)
gh api "repos/${CONSOLE_REPO}/compare/${CONSOLE_LATEST}...main" \
--jq '{status, ahead_by, behind_by, commits: [.commits[] | {sha, message: .commit.message}]}'
```
- `ahead_by == 0`: no merged Console change is waiting for release. Still verify the current latest asset using step 4, then continue to Phase 1.
- `ahead_by > 0` and `behind_by == 0`: publish Console before continuing. Report the merged commits and select the next unused `vX.Y.Z` tag. Default to the next patch version when the changes are fixes or backward-compatible UI work; stop for confirmation if a minor/major bump is plausible.
- Any diverged history or `behind_by > 0`: stop and resolve the Console release baseline explicitly. Do not guess a range or publish RustFS.
2. Clone/fetch `rustfs/console` into a scratch directory and record its exact `main` commit. Before creating a tag, check for a `v*` tag or Release workflow already associated with that hash. If one is in progress, wait for it instead of creating another version:
```bash
CONSOLE_SCRATCH=$(mktemp -d)
gh repo clone "$CONSOLE_REPO" "$CONSOLE_SCRATCH/console"
git -C "$CONSOLE_SCRATCH/console" fetch origin main --tags
CONSOLE_HASH=$(git -C "$CONSOLE_SCRATCH/console" rev-parse origin/main)
git -C "$CONSOLE_SCRATCH/console" tag --points-at "$CONSOLE_HASH" 'v*'
gh run list -R "$CONSOLE_REPO" --workflow release.yml --commit "$CONSOLE_HASH" --limit 5
```
If no release exists or is running for `CONSOLE_HASH`, create the selected annotated tag at that exact hash and push it:
```bash
git -C "$CONSOLE_SCRATCH/console" tag -a "<console-tag>" -m "Release <console-tag>" "$CONSOLE_HASH"
git -C "$CONSOLE_SCRATCH/console" push origin "<console-tag>"
```
Console tags include the `v` prefix. Pushing the tag triggers `.github/workflows/release.yml` (`🚀 Release`). Remove `CONSOLE_SCRATCH` after the gate completes.
3. Find the exact tag run and wait for completion:
```bash
gh run list -R "$CONSOLE_REPO" --workflow release.yml --branch "<console-tag>" --limit 1
gh run watch -R "$CONSOLE_REPO" "<console-run-id>" --exit-status
```
4. Block until the published Release is non-draft, the latest endpoint returns the expected tag, and `rustfs-console-<console-tag>.zip` is uploaded, non-empty, and carries a `sha256:` digest:
```bash
gh release view -R "$CONSOLE_REPO" "<console-tag>" --json isDraft,isPrerelease,assets,url
test "$(gh api "repos/${CONSOLE_REPO}/releases/latest" --jq .tag_name)" = "<console-tag>"
test "$(gh api "repos/${CONSOLE_REPO}/releases/tags/<console-tag>" \
--jq '[.assets[] | select(.name == "rustfs-console-<console-tag>.zip" and .state == "uploaded" and .size > 0 and (.digest | startswith("sha256:")))] | length')" -eq 1
```
Treat a missing/mismatched asset, digest, latest tag, or failed/cancelled workflow as BLOCKED. Do not start Phase 1 until the Console gate passes. Record `CONSOLE_TAG`, `CONSOLE_HASH`, Console run URL, and Release URL for the final report.
Read and complete [the Console gate](references/console-gate.md) before Phase 1. Verify the latest published Console asset and exact commit; if Console main is ahead, complete its release and asset verification first. A successful build alone does not satisfy this gate.
## Phase 1 — Version bump to the final target (once)
@@ -164,54 +118,9 @@ On a restart (N+1), refresh `PREVIEW_HASH=$(git rev-parse origin/main)` first
- Record `PREVIOUS_DELIVERABLE`, selected from published Releases by `publishedAt` after excluding the current tag and every `-preview.N` tag. Verify `gh release view "<preview-tag>" --json body --jq .body` contains `## What's Changed` and, when `PREVIOUS_DELIVERABLE` exists, `**Full Changelog**: https://github.com/rustfs/rustfs/compare/<PREVIOUS_DELIVERABLE>...<preview-tag>`. For a repository with no previous deliverable, verify a Full Changelog link exists and record the GitHub baseline fallback.
- Confirm preview-triggered Docker and Helm jobs are skipped. Preview validation covers the built RustFS binaries, embedded console, and rc compatibility; Docker image construction and Helm publication are deferred to the final tag because the Dockerfiles consume GitHub Release assets.
## Phase 4 — Run the artifact locally, verify the console
## Phases 45Local artifact, Console, and rc acceptance
Work inside the session scratchpad directory; never leave stray data dirs.
```bash
gh release download "<preview-tag>" -p "rustfs-macos-aarch64-v<preview-tag>.zip" -D "$SCRATCH"
cd "$SCRATCH" && unzip -o rustfs-*.zip
./rustfs --version # must report the PREVIEW TAG (build::TAG), not the Cargo.toml version, plus expected short SHA
mkdir -p data
RUSTFS_ACCESS_KEY=rustfsadmin RUSTFS_SECRET_KEY=rustfsadmin ./rustfs ./data
```
Defaults: S3 endpoint `:9000`, embedded console `:9001`.
Checks (all must pass):
- `./rustfs --version` reports the preview tag name and the short SHA of `PREVIEW_HASH`. Reporting `<target>` without the `-preview.N` suffix means the build did not embed the tag — treat as FAIL and investigate before proceeding.
- `curl -fsS http://localhost:9000/health/ready` returns ready.
- Startup log shows the embedded console being served (this was the regression that `fix(release): require embedded console assets` guards).
- Open `http://localhost:9001` in the browser: login with `rustfsadmin`/`rustfsadmin`; dashboard renders without JS console errors; create a bucket, upload a file, download it back (byte-identical), delete the object and bucket. Keep the server running for Phase 5.
## Phase 5 — Validate with the latest rc client
`rc` is the RustFS CLI client from <https://github.com/rustfs/cli>.
- Ensure the latest release is installed: compare `rc --version` against `gh api repos/rustfs/cli/releases/latest --jq .tag_name`; update via `brew upgrade rustfs/tap/rc` (or download the release binary).
- Point it at the preview server and run the command matrix, recording PASS/FAIL per command:
```bash
rc alias set preview http://localhost:9000 rustfsadmin rustfsadmin
rc ls preview/
rc mb preview/rel-check
rc cp <local-file> preview/rel-check/
rc stat preview/rel-check/<file>
rc cat preview/rel-check/<file> # matches source
rc cp preview/rel-check/<file> ./out && cmp <local-file> ./out
rc cp -r <local-dir>/ preview/rel-check/dir/
rc find preview/rel-check --name "*"
rc share download preview/rel-check/<file> --expire 1h # presigned URL fetchable via curl
rc rm preview/rel-check/<file> && rc rm -r --force preview/rel-check/dir
rc rb preview/rel-check
rc admin user list preview/
rc admin user add preview/ relcheckuser relchecksecret12
rc admin user remove preview/ relcheckuser
rc alias remove preview
```
- Any FAIL blocks the release. Afterwards stop the server and delete the scratch data directory.
Read and complete [preview acceptance](references/preview-acceptance.md): verify the downloaded binary's tag/SHA and readiness, exercise Console CRUD with byte-identical download, and pass the full latest-rc command matrix. Any failure blocks final publication. Retain the results for the confirmation gate below.
### Manual confirmation gate
@@ -0,0 +1,58 @@
# Console Release Gate
Read during Phase 0, before changing RustFS version files or tags. Follow the parent skill's release scope and authorization rules.
### Console release gate
Complete this gate before changing any RustFS version file or creating any RustFS tag. RustFS `build.yml` downloads the asset returned by `repos/rustfs/console/releases/latest`, so a successful Console build alone is insufficient.
1. Read the latest published Console tag and compare it with Console `main`:
```bash
CONSOLE_REPO="rustfs/console"
CONSOLE_LATEST=$(gh api "repos/${CONSOLE_REPO}/releases/latest" --jq .tag_name)
gh api "repos/${CONSOLE_REPO}/compare/${CONSOLE_LATEST}...main" \
--jq '{status, ahead_by, behind_by, commits: [.commits[] | {sha, message: .commit.message}]}'
```
- `ahead_by == 0`: no merged Console change is waiting for release. Still verify the current latest asset using step 4, then continue to Phase 1.
- `ahead_by > 0` and `behind_by == 0`: publish Console before continuing. Report the merged commits and select the next unused `vX.Y.Z` tag. Default to the next patch version when the changes are fixes or backward-compatible UI work; stop for confirmation if a minor/major bump is plausible.
- Any diverged history or `behind_by > 0`: stop and resolve the Console release baseline explicitly. Do not guess a range or publish RustFS.
2. Clone/fetch `rustfs/console` into a scratch directory and record its exact `main` commit. Before creating a tag, check for a `v*` tag or Release workflow already associated with that hash. If one is in progress, wait for it instead of creating another version:
```bash
CONSOLE_SCRATCH=$(mktemp -d)
gh repo clone "$CONSOLE_REPO" "$CONSOLE_SCRATCH/console"
git -C "$CONSOLE_SCRATCH/console" fetch origin main --tags
CONSOLE_HASH=$(git -C "$CONSOLE_SCRATCH/console" rev-parse origin/main)
git -C "$CONSOLE_SCRATCH/console" tag --points-at "$CONSOLE_HASH" 'v*'
gh run list -R "$CONSOLE_REPO" --workflow release.yml --commit "$CONSOLE_HASH" --limit 5
```
If no release exists or is running for `CONSOLE_HASH`, create the selected annotated tag at that exact hash and push it:
```bash
git -C "$CONSOLE_SCRATCH/console" tag -a "<console-tag>" -m "Release <console-tag>" "$CONSOLE_HASH"
git -C "$CONSOLE_SCRATCH/console" push origin "<console-tag>"
```
Console tags include the `v` prefix. Pushing the tag triggers `.github/workflows/release.yml` (`🚀 Release`). Remove `CONSOLE_SCRATCH` after the gate completes.
3. Find the exact tag run and wait for completion:
```bash
gh run list -R "$CONSOLE_REPO" --workflow release.yml --branch "<console-tag>" --limit 1
gh run watch -R "$CONSOLE_REPO" "<console-run-id>" --exit-status
```
4. Block until the published Release is non-draft, the latest endpoint returns the expected tag, and `rustfs-console-<console-tag>.zip` is uploaded, non-empty, and carries a `sha256:` digest:
```bash
gh release view -R "$CONSOLE_REPO" "<console-tag>" --json isDraft,isPrerelease,assets,url
test "$(gh api "repos/${CONSOLE_REPO}/releases/latest" --jq .tag_name)" = "<console-tag>"
test "$(gh api "repos/${CONSOLE_REPO}/releases/tags/<console-tag>" \
--jq '[.assets[] | select(.name == "rustfs-console-<console-tag>.zip" and .state == "uploaded" and .size > 0 and (.digest | startswith("sha256:")))] | length')" -eq 1
```
Treat a missing/mismatched asset, digest, latest tag, or failed/cancelled workflow as BLOCKED. Do not start Phase 1 until the Console gate passes. Record `CONSOLE_TAG`, `CONSOLE_HASH`, Console run URL, and Release URL for the final report.
@@ -0,0 +1,52 @@
# Preview Artifact Acceptance
Read after Phase 3 succeeds. Complete every check below before the parent skill's manual confirmation gate. These checks cover the downloaded artifact, embedded Console, and latest rc client.
## Phase 4 — Run the artifact locally, verify the console
Work inside the session scratchpad directory; never leave stray data dirs.
```bash
gh release download "<preview-tag>" -p "rustfs-macos-aarch64-v<preview-tag>.zip" -D "$SCRATCH"
cd "$SCRATCH" && unzip -o rustfs-*.zip
./rustfs --version # must report the PREVIEW TAG (build::TAG), not the Cargo.toml version, plus expected short SHA
mkdir -p data
RUSTFS_ACCESS_KEY=rustfsadmin RUSTFS_SECRET_KEY=rustfsadmin ./rustfs ./data
```
Defaults: S3 endpoint `:9000`, embedded console `:9001`.
Checks (all must pass):
- `./rustfs --version` reports the preview tag name and the short SHA of `PREVIEW_HASH`. Reporting `<target>` without the `-preview.N` suffix means the build did not embed the tag — treat as FAIL and investigate before proceeding.
- `curl -fsS http://localhost:9000/health/ready` returns ready.
- Startup log shows the embedded console being served (this was the regression that `fix(release): require embedded console assets` guards).
- Open `http://localhost:9001` in the browser: login with `rustfsadmin`/`rustfsadmin`; dashboard renders without JS console errors; create a bucket, upload a file, download it back (byte-identical), delete the object and bucket. Keep the server running for Phase 5.
## Phase 5 — Validate with the latest rc client
`rc` is the RustFS CLI client from <https://github.com/rustfs/cli>.
- Ensure the latest release is installed: compare `rc --version` against `gh api repos/rustfs/cli/releases/latest --jq .tag_name`; update via `brew upgrade rustfs/tap/rc` (or download the release binary).
- Point it at the preview server and run the command matrix, recording PASS/FAIL per command:
```bash
rc alias set preview http://localhost:9000 rustfsadmin rustfsadmin
rc ls preview/
rc mb preview/rel-check
rc cp <local-file> preview/rel-check/
rc stat preview/rel-check/<file>
rc cat preview/rel-check/<file> # matches source
rc cp preview/rel-check/<file> ./out && cmp <local-file> ./out
rc cp -r <local-dir>/ preview/rel-check/dir/
rc find preview/rel-check --name "*"
rc share download preview/rel-check/<file> --expire 1h # presigned URL fetchable via curl
rc rm preview/rel-check/<file> && rc rm -r --force preview/rel-check/dir
rc rb preview/rel-check
rc admin user list preview/
rc admin user add preview/ relcheckuser relchecksecret12
rc admin user remove preview/ relcheckuser
rc alias remove preview
```
- Any FAIL blocks the release. Afterwards stop the server and delete the scratch data directory.
@@ -4,17 +4,16 @@ description: "Prepare the version-file and release-asset bump for an exact RustF
---
# RustFS Release Version Bump
Use this skill to publish a RustFS release (alpha, beta, or stable) with a minimal, auditable diff and a complete ship flow (`edit -> verify -> commit -> push -> PR`).
Use this skill to prepare and verify release version files. Commit, push, and PR steps apply only when included in the user's delivery scope; publishing release tags belongs to `rustfs-release-publish`.
Validated baseline: release pattern used in PR `#2957`.
## Required inputs
- Exact target version, for example `1.0.0-beta.4`.
- Delivery scope:
- Local only (`edit/verify`).
- Local + git (`commit/push`).
- Full GitHub flow (`commit/push/PR`).
- Delivery scope: local (`edit/verify`), git (`commit/push`), or GitHub
(`commit/push/PR`). Derive it from the conversation; when unspecified, prepare
and verify locally without blocking on a delivery question.
If target version is missing or ambiguous, stop and ask before editing.
@@ -23,7 +22,7 @@ Reject any target version containing `-preview`: preview identifiers are tag-onl
## Read before editing
- `AGENTS.md` (root and nearest path-specific files).
- `.github/pull_request_template.md`.
- `.github/pull_request_template.md` only when preparing a PR.
- Current branch status and diff against `origin/main`.
## Default release file scope
@@ -50,8 +49,7 @@ Only drop a file when the current repository release process clearly no longer r
## Step-by-step workflow
1. Confirm intent and isolate scope
- Confirm target version string exactly.
- Confirm whether user requested local-only or full GitHub flow.
- Use the exact target and delivery scope already supplied; ask only for a missing or ambiguous target or a material release-policy choice.
- Inspect current branch and ensure only release-related files are touched for this task.
2. Update workspace versions
@@ -82,18 +80,18 @@ Only drop a file when the current repository release process clearly no longer r
4. Verify before shipping
- Run:
- `make pre-commit`
- If `make pre-commit` fails, return `BLOCKED` with root cause and do not silently widen scope to fix unrelated issues unless user asks.
- If `make pre-commit` fails, fix task-attributable failures and rerun affected checks. Report unresolved required checks as `BLOCKED`; do not silently widen scope to fix unrelated issues.
5. Commit strategy
5. Commit strategy (only when committing is authorized)
- Preferred split when both parts changed:
- `chore(release): prepare <version>` for `Cargo.toml` and `Cargo.lock`.
- `chore(release): align release assets for <version>` for docs and packaging files.
- If user asks for one commit, use one commit.
- Stage only intended release files; do not include unrelated working tree changes.
6. Push and PR
6. Push and PR (only for the authorized delivery scope)
- Push branch:
- `git push -u origin <branch>` (first push), or `git push` (tracking already exists).
- Use the user-requested or configured push remote: `git push -u <push-remote> <branch>` (first push), or `git push` when tracking is already configured.
- Create PR with template headings unchanged:
- `gh pr create --base main --head <branch> --title ... --body-file ...`
- PR title/body must be English.
@@ -12,8 +12,9 @@ matched security surface, the concise security reference under
## Workflow
1. Freeze the exact diff/head and identify the changed trust boundaries.
2. Read [advisory-patterns.md](references/advisory-patterns.md), then apply only
the matching sections. Useful headings are
2. Inspect the headings in [advisory-patterns.md](references/advisory-patterns.md),
then read the matching sections. Read the full map only for a broad security
audit. Useful headings are
auth/admin, IAM/STS/OIDC, policy/plugins, S3/copy/multipart, protocols, paths,
secrets/logging/RPC, browser/CORS/proxy, SSE, Object Lock, and serde.
3. Trace unauthenticated, low-privilege, wrong-action/owner/bucket, malformed,
@@ -108,9 +108,9 @@ Update this file only when an advisory adds or changes a reusable lesson, affect
### Serde deserialization and input validation
- No `#[serde(deny_unknown_fields)]` found across the entire codebase. Lesson: all structs deserialized from untrusted input (S3 API XML/JSON, lifecycle rules, bucket policies, replication configs) should have `#[serde(deny_unknown_fields)]` to reject malformed or adversarial payloads.
- Reject unknown fields in untrusted S3 API XML/JSON, lifecycle, policy, and replication input where compatibility permits. Check the current type and supported payload fixtures; do not infer the repository's current coverage from an older audit. Where extra fields are part of the compatibility contract, validate security-critical values explicitly.
- `#[serde(default)]` on security-critical fields silently accepts missing values as zero/empty. Lesson: when a field has security implications (retention days, permissions, limits), validate the deserialized value explicitly rather than relying on defaults.
- Integer fields deserialized from user input and cast with `as` (e.g., `i32 as u32`) can wrap negative values to large positives. Lesson: validate ranges before casting; use `try_into()` or clamp.
- Integer fields deserialized from user input and cast with `as` (e.g., `i32 as u32`) can wrap negative values to large positives. Lesson: validate ranges before casting; use `try_into()` with a typed error, or clamp only when the domain explicitly requires saturation.
- XML config typos (e.g., `"NoncurentDays"` instead of `"NoncurrentDays"`) are silently accepted when `deny_unknown_fields` is absent. Lesson: strict deserialization prevents silent misconfiguration that could cause data loss or unexpected retention behavior.
## Useful Search Seeds
+28 -37
View File
@@ -1,6 +1,6 @@
---
name: test-coverage-improver
description: Run project coverage checks, rank high-risk gaps, and propose high-impact tests to improve regression confidence for changed and critical code paths before release.
description: Analyze a supplied coverage report or perform an explicitly requested RustFS coverage assessment, rank uncovered risks, and propose focused tests. Do not trigger for ordinary implementation verification, a single regression test, documentation wording, or release preparation without a coverage request.
---
# Test Coverage Improver
@@ -9,58 +9,49 @@ Use this skill when you need a prioritized, risk-aware plan to improve tests fro
## Usage assumptions
- Focus scope is either changed lines/files, a module, or the whole repository.
- Coverage artifact must be generated or provided in a supported format.
- Reuse a supplied coverage artifact when its revision, scope, and format match.
- If required context is missing, call out assumptions explicitly before proposing work.
## Workflow
1. Define scope and baseline
- Confirm target language, framework, and branch.
- Confirm whether the scope is changed files only or full-repo.
- Derive the revision and scope from the request, diff, or supplied report.
- Default to the affected files/module; whole-workspace coverage requires that
scope in the request. Ask only if a wrong scope would change the result.
2. Produce coverage snapshot
- Rust: `cargo llvm-cov` (or `cargo tarpaulin`) with existing repo config.
- JavaScript/TypeScript: `npm test -- --coverage` and read `coverage/coverage-final.json`.
- Python: `pytest --cov=<pkg> --cov-report=json` and read `coverage.json`.
- Collect total, per-file, and changed-line coverage.
2. Obtain coverage evidence
- First inspect a matching existing artifact; do not regenerate it merely
because this skill was selected.
- If measurement is needed, read the Coverage section of
[the testing guide](../../../docs/testing/README.md#coverage), check disk
space/tool availability, and select package/test-scoped `cargo llvm-cov`
using the repository's nextest configuration. `make coverage` measures the
whole workspace (excluding E2E) and is only for that requested scope.
- Collect only metrics the report supports. Missing branch/changed-line
coverage is unknown, not zero.
- If measurement cannot run, continue with code-based test proposals and
mark measured coverage unverified; do not invent a coverage percentage.
3. Rank highest-risk gaps
- Prioritize changed code, branch coverage gaps, and low-confidence boundaries.
- Apply the risk rubric in [coverage-prioritization.md](references/coverage-prioritization.md).
- Keep shortlist to 58 gaps.
- Report up to 58 evidenced gaps; do not pad a small scope.
- For each gap, capture: file, lines, uncovered branches, and estimated risk score.
4. Propose high-impact tests
- For each shortlisted gap, output:
- Intent and expected behavior.
- Normal, edge, and failure scenarios.
- Assertions and side effects to verify.
- Setup needs (fixtures, mocks, integration dependencies).
- Estimated effort (`S/M/L`).
- For each gap, name the behavior and regression, distinguishing assertions,
relevant normal/edge/failure cases, necessary setup, and estimated effort.
- Include only scenarios and setup that apply; reuse shared fixture details.
5. Close with validation plan
- State which gaps remain after proposals.
- Provide concrete verification command and acceptance threshold.
- Give a scoped verification command and behavior-based acceptance criterion;
use a coverage threshold only when the task or repository requires one.
- List assumptions or blockers (environment, fixtures, flaky dependencies).
## Output template
## Report
### Coverage Snapshot
- total / branch coverage
- changed-file coverage
- top missing regions by size
### Top Gaps (ranked)
- `path:line-range` | risk score | why critical
### Test Proposals
- `path:line-range`
- Test name
- scenarios
- assertions
- effort
### Validation Plan
- command
- pass criteria
- remaining risk
Summarize the supported metrics, then combine each ranked gap with its proposed
test and validation. Include source lines only when supplied or inspected;
mark missing metrics or locations as unknown. Do not duplicate gaps and tests
in separate templates or fill empty categories for an otherwise small report.
@@ -1,4 +1,4 @@
interface:
display_name: "Test Coverage Improver"
short_description: "Find top uncovered risk areas and propose high-impact tests."
default_prompt: "Run coverage checks, identify largest gaps, and recommend highest-impact test cases to improve risk coverage."
default_prompt: "Use $test-coverage-improver to analyze coverage for the requested scope, reuse matching reports, and propose tests for evidenced risks."
+5 -2
View File
@@ -5,8 +5,11 @@ description: Debug ILM tiering / lifecycle transition issues — NoSuchVersion o
# Tier / ILM Debugging
Full playbook: [docs/operations/tier-ilm-debugging.md](../../../docs/operations/tier-ilm-debugging.md)
— read it before changing tier code.
Playbook: [docs/operations/tier-ilm-debugging.md](../../../docs/operations/tier-ilm-debugging.md).
Read the section matching the symptom: metadata/`xl.meta`, runtime versionId,
manual jobs, or retained-record recovery. Read the local-first expiry invariant
before changing cleanup ordering. Before a reconcile/disposition action, read
its entire procedure and retain its exact-evidence and confirmation gates.
Quick moves:
+5
View File
@@ -0,0 +1,5 @@
# Bound individual tool outputs retained in context; retrieve relevant ranges
# from task-owned log files when more evidence is needed.
# https://learn.chatgpt.com/docs/config-file/config-reference
tool_output_token_limit = 4000
+1 -1
View File
@@ -1,2 +1,2 @@
sha256-darwin=53b05ac745905809d3828c6994bdd8ecf9d20b2b61a8a9d80fe15eb62f932193
sha256-darwin=dd14f49a7b0e2c156b4457fdd836499d837890d3e439689a4eff9e8875ee2f5b
sha256-linux=7c892afa4b9d1591b46bd79c976b647109a277284fddb3b98edced4b0297eda2
+1 -1
View File
@@ -1 +1 @@
sha256=5db88c6fec94d4f269c7d9cfc128bd2adc27b3d7021127e2fa0b1daccc5f900f
sha256=6d18f9cce820c51d5589de944e8cc185f73eeca0ea9a9916651943e3759169d0
+1 -1
View File
@@ -9,4 +9,4 @@
# if the selected count drops below this number, so a rename or removal that
# thins the security smoke gate must update this file in the same PR.
# Adding tests does not require a bump, but bumping keeps the guard tight.
18
26
+8
View File
@@ -82,6 +82,14 @@ jobs:
cache_key: e2e-odm-config-rollback
test: rc5_rollback_requires_restoring_odm_configuration
artifact: odm-config-rollback
- name: Multipart layouts survive the rc.5 upgrade
cache_key: e2e-multipart-layout-upgrade
test: direct_upgrade_from_rc5_preserves_multipart_layouts
artifact: multipart-layout-upgrade
- name: rc.5 multipart replication baseline
cache_key: e2e-multipart-layout-baseline
test: rc5_baseline_replicates_multipart_layouts
artifact: multipart-layout-baseline
runs-on: ubuntu-latest
timeout-minutes: 60
env:
+38 -123
View File
@@ -7,8 +7,11 @@ This file contains repository-wide rules. Use the nearest subdirectory
1. System/developer instructions.
2. The current user request.
3. The nearest `AGENTS.md`.
4. This file.
3. Applicable `AGENTS.md` files, with the nearest file winning conflicts.
4. Selected skills and reference documents.
Nested instructions add to ancestor rules; they do not discard non-conflicting
rules. A skill cannot expand the user's requested scope or grant authorization.
## Operating Model
@@ -21,63 +24,29 @@ This file contains repository-wide rules. Use the nearest subdirectory
- Do not load every skill or inspect unrelated modules preemptively. Select a
skill only when its description directly matches the request or changed
surface.
- Resolve repository workflow skills under `.agents/skills/` when a global
skill has the same name, unless the user explicitly selects another path.
- Avoid repeated reads and equivalent verification commands once enough
evidence exists.
- Search for relevant symbols/headings before reading long files; return only
matching ranges. If output is truncated, narrow the query instead of repeating
a full read. Keep reusable raw logs in task artifacts and report the evidence.
- Reuse authorization already given in the conversation. Resolve routine choices
within that scope and continue independent work while a material question is
pending. Before requesting missing approval, prepare the concrete result that
is already authorized; retain explicit merge and release gates.
## Worktree and Disk Hygiene
## Task-Specific Guidance
- Start implementation from the latest `origin/main` and confirm the requested
change is not already present.
- An existing clean, isolated task worktree is sufficient. Create another
worktree only when the current checkout is shared, dirty with unrelated work,
or belongs to another task.
- Never commit from a shared checkout.
- Use a task-specific branch named `<type>/<topic>`, such as `fix/...`,
`feat/...`, `test/...`, or `docs/...`, unless the user specifies a name.
- Do not include agent, tool, contributor, account, or organization names in
branch names.
- Push to the user-requested remote or the repository's configured push remote.
Do not hard-code or infer a remote from an account name.
- Check free space before artifact-heavy builds, tests, coverage, or downloads.
Re-check before a broad gate when space is tight.
- Remove only task-owned temporary/build artifacts. Never delete another task's
worktree or uncommitted data.
- At handoff, mention disk or cleanup details only when they affected execution
or artifacts/worktrees remain intentionally.
Read only the reference needed for the current task, once per unchanged context:
## Change Style
- Preserve existing control flow unless changing it is required for correctness.
- Prefer a direct local edit over new files, wrappers, managers, or speculative
abstractions.
- Add a helper only when it removes current duplication, names a real domain
boundary, or isolates a non-trivial invariant.
- Remove an in-scope path superseded by the change. If compatibility requires it,
adapt at the boundary to one canonical core and use the repository's
`RUSTFS_COMPAT_TODO` policy.
- Comments explain non-obvious invariants or reasons. Do not narrate code or
record change history.
- Mention unrelated problems when useful; do not fix them in a narrow task.
## Reuse and Boundary Rules
- Before adding helpers, constants, fixtures, or wrappers, search the touched
crate, the domain-owning crate, `crates/utils`, `crates/common`, and relevant
direct dependencies.
- Reuse requires matching semantics: normalization, error types, deadlines,
durability, and compatibility must fit the call site. A narrowly named local
helper is better than forced reuse with different semantics.
- Validate untrusted input at its trust boundary, then trust the validated type.
Values crossing disk, RPC, persistence, or version boundaries remain
untrusted at every consumer.
- Re-check boundary values immediately before destructive actions such as
delete, overwrite, or quorum decisions.
- Every new branch needs a concrete triggering input/state. For decoded or peer
data, corruption and mixed-version input are valid triggers.
- Required values must return a typed error when absent or corrupt; do not use a
default that converts corruption into a plausible result.
- Attach error context once where it is actionable. Do not erase typed errors
below aggregation or quorum layers.
- Before code changes or artifact-heavy work, read [implementation rules](.agents/references/implementation.md).
For a read-only code review, use its change-style and boundary sections as needed.
- Before commits, pushes, PR creation/updates, or posting to PRs/issues/discussions,
read [Git and PR rules](.agents/references/pull-requests.md).
Reuse existing authorization; a reference does not authorize posting, merging, or publishing.
- Preserve unrelated work. Never commit from a shared checkout or delete another task's artifacts.
- Source comments, commits, PR titles, and PR bodies are in English.
## Sources of Truth
@@ -147,73 +116,25 @@ requested adversarial/design reviews, and agent-instruction changes that alter
execution. Ordinary questions, diagnoses, status reports, non-adversarial code
reviews, and low-risk planning do not trigger it.
Risk and review shape:
For applicable work and substantial PR reviews, read the [risk tiers and review shape](.agents/references/adversarial-validation.md).
Load only the matching domain probes; ordinary reviews do not become adversarial
merely because this reference exists.
- **Exempt:** documentation, comments, formatting, or typos with no runtime,
build, test, or agent-execution effect.
- **Mechanical:** renames, moves, test/tooling-only changes, and agent-rule
changes. Run correctness and simplicity lenses.
- **Standard:** localized behavior changes. Run one integrated final-diff pass
covering correctness, simplicity, and test coverage; add only domain lenses
matched by the diff.
- **High risk / substantial PR review:** high risk includes locking,
erasure/quorum/heal, replication, multipart, RPC, lifecycle/tiering,
persistence/fsync, IAM/KMS/auth, cryptography, on-disk/on-wire formats, and
S3-visible semantics. Cover all applicable lenses using exactly two
independent reviewers when delegation is explicitly authorized. Split the
lenses between them. Otherwise perform two fresh sequential passes.
- **Outbound client defaults:** what `TargetClient`, `PutObjectOptions`, or
the remote SDK configuration sends to every replication or migration target
is high risk for every target class even when the change fixes one. Follow
the SOP in `docs/postmortems/2026-09-03-replication-checksum-default-regression.md`:
run the outbound target matrix, document each new env knob in the same PR,
and list verified and unverified target classes in the PR Impact section.
A review has no finding quota; `No findings` is a complete outcome. A request to
find problems is not evidence that a defect exists. Before reporting a candidate,
check callers, invariants, and existing tests for evidence that disproves it.
Findings need `file:line` and a concrete failure or violation of an explicit
requirement. Missing required tests/checks are verification gaps, not proof of a
runtime bug; name the unprotected behavior or unmet gate. Keep optional style or
refactoring preferences out of defect findings unless that review was requested.
Available domain lenses are security, concurrency/durability, compatibility,
and performance. Select `.agents/skills/adversarial-validation/SKILL.md` for an
explicit adversarial request, a high-risk change, or a substantial PR review;
then read only its matching role references. A routine standard pass does not
load the playbook unless the reviewer needs a RustFS-specific probe.
A finding must name a concrete input/state/interleaving and wrong outcome, or a
specific missing regression check, with `file:line`. Resolve it by fixing the
diff or rebutting it with code-path/test/invariant evidence. After a non-trivial
fix, rerun only affected lenses.
Fix or rebut supported findings within the authorized scope. Once the required
passes are complete, stop. Reopen only for changed code, new evidence, an
unresolved finding, or an explicit re-review request; an unchanged diff does not
need another pass at every conversation turn or workflow handoff.
For high-risk PRs, record one concise verdict per covered lens in the PR body.
## Pull Request Lifecycle
- Creating or updating a PR includes one immediate snapshot of checks,
mergeability, reviews, and unresolved threads.
- Unless the user explicitly requests monitoring, a release workflow requires
it, or an automation already owns it, hand off after the PR is open with the
current state and next event to watch. Do not delay ordinary handoff with
fixed quiet-period sleeps.
- For requested monitoring, use event-driven or bounded waits. Report only state
changes, actionable failures, or a meaningful prolonged delay.
- Investigate failures/comments before changing code. Fix task-attributable
issues, rerun affected verification, push, reply or resolve the thread, then
resume the requested monitor.
- Never merge without required reviewer approval or explicit authority.
- After an observed merge, verify the commit reached the base, then clean the
task worktree/branch when safe. Preserve unmerged work for closed PRs unless
deletion was explicitly authorized.
## Git and PR Baseline
- Follow Conventional Commits; keep the subject at most 72 characters.
- Source comments, commits, PR titles, and PR bodies are in English.
- Keep every heading from `.github/pull_request_template.md`; use `N/A` where
needed and include commands actually run.
- Use `--body-file` for multiline `gh pr create`/`gh pr edit` content.
- PR/issue/discussion content must not contain the literal sequence `\n` or
hard-wrapped prose paragraphs.
- Do not include local absolute paths or tool-specific labels/prefixes in GitHub
content.
- Resolve review threads after the underlying issue is fixed. If declining a
suggestion, reply with a short evidence-based reason.
## Security Baseline
- Never commit secrets, credentials, or key material.
@@ -250,12 +171,6 @@ Use `.agents/skills/rustfs-logging-governance/SKILL.md` for logging changes.
- `DataUsageCacheInfo` and `DataUsageEntry` keep their hand-written map
serialization and new fields remain `#[serde(default)]` for older readers.
## Naming
Use Rust API naming: `SCREAMING_SNAKE_CASE` constants/statics, `snake_case`
functions/variables, and `PascalCase` types. Do not rename unrelated existing
violations.
## Scoped Guidance
Before editing, locate the nearest instructions with:
@@ -265,4 +180,4 @@ git ls-files '*AGENTS.md'
```
The nearest file wins for domain invariants. Keep generic workflow and
validation policy in this root file.
validation policy in this root file and its task-specific references.
+3
View File
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Security
- **Presigned URLs honour only signed headers** (GHSA-g8w9-qw9q-fghr): a SigV4 presigned request that carries an `x-amz-*` request header not listed in `X-Amz-SignedHeaders` is now rejected with `403 AccessDenied` ("There were headers present in the request which were not signed"), matching AWS S3. Previously the holder of a presigned `PutObject` URL could add unsigned `x-amz-tagging`, `x-amz-storage-class`, `x-amz-website-redirect-location`, ACL, metadata, Object Lock or SSE headers and have them applied. Presigners that intend a property must set it before signing so the SDK lists the header in `SignedHeaders`; `x-amz-cf-id` (CloudFront) remains tolerated unsigned. Header-signed SigV4 and SigV2 requests are unchanged.
### Fixed
- **Multipart admission queue**: an `UploadPart` waiting for a foreground write permit now waits at most 10 s by default (`RUSTFS_PUT_MULTIPART_FOREGROUND_ADMISSION_WAIT_TIMEOUT_MS`, previously 30 s), so a queued part returns S3 `SlowDown` before the client's socket write timeout drops the connection. Separately, the API listener no longer forces a 4 MiB `SO_RCVBUF` on every accepted socket (kernel autotuning applies; `RUSTFS_HTTP_SOCKET_RECV_BUFFER_BYTES` restores a fixed size), so a queued part no longer lets up to 8 MiB of unread body accumulate in kernel memory per connection, which is what throttled whole nodes under SDK-default multipart concurrency. Fixes #7385.
- **Helm Ingress**: `customAnnotations` are now merged with class-specific annotations (nginx/traefik) instead of being ignored when `ingress.className` is set.
+2 -2
View File
@@ -13,7 +13,7 @@ what Claude Code needs on top: commands and pointers.
cargo build --release --bin rustfs # production binary
cargo check -p <crate> # fast type-check one crate
cargo test -p <crate> # test one crate
cargo fmt --all # format (required before PR)
cargo fmt --all --check # for Rust changes; see AGENTS.md verification tiers
make pre-commit # fast gate: fmt + arch checks + quick-check (NO clippy/tests)
make pre-pr # optional full gate for broad cross-module changes
make build-docker BUILD_OS=ubuntu22.04
@@ -42,5 +42,5 @@ make build-docker BUILD_OS=ubuntu22.04
Repo-wide domain invariants (dual internal metadata keys, defensive UUID
reads, unversioned tier buckets) live in [AGENTS.md](AGENTS.md) under
"Cross-Cutting Domain Invariants" — read them before touching metadata or
"Cross-Cutting Storage Invariants" — read them before touching metadata or
tiering code.
+5 -2
View File
@@ -33,8 +33,11 @@ Applies to all paths under `crates/`.
## Type Casting
- Never use `as` for numeric conversions that may truncate or overflow. Use `try_into()` with explicit error handling, or clamp with `value.max(0) as usize` when the domain is bounded.
- `f64 as usize` saturates but is fragile; clamp to `[0, usize::MAX as f64]` first.
- Never use `as` for numeric conversions that may truncate or overflow. Use
`try_into()` with typed error handling; clamp or saturate only when the domain
explicitly requires it.
- Before converting floating-point input to an integer, validate finiteness,
sign, and the destination range. A lower-bound clamp alone is insufficient.
- Treat every `as` cast in a PR review as a potential bug; require justification.
## Testing
@@ -356,3 +356,238 @@ async fn tampered_presigned_put_returns_signature_does_not_match() -> Result<(),
);
Ok(())
}
/// GHSA-g8w9-qw9q-fghr: a presigned PUT signed with `SignedHeaders=host` must
/// not honour `x-amz-*` headers the uploader adds afterwards. The presign
/// authorised one plain upload; the extra headers would set tags, storage
/// class and a website redirect the presigner never covered. AWS S3 rejects
/// this with 403 `AccessDenied`, and so must RustFS — and the object must not
/// be stored at all, not merely stored without the properties.
#[tokio::test]
async fn ghsa_g8w9_presigned_put_rejects_unsigned_x_amz_headers() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
setup(&mut env).await?;
let key = "presigned-put-unsigned-amz-headers.txt";
let pr = env
.create_s3_client()
.put_object()
.bucket(BUCKET)
.key(key)
.presigned(valid_config())
.await?;
assert!(
!pr.headers().any(|(name, _)| name.eq_ignore_ascii_case("x-amz-tagging")),
"fixture must presign a plain PutObject without tagging so the header below is unsigned"
);
let unsigned: Vec<(&str, &str)> = vec![
("x-amz-tagging", "owner=attacker&classification=public"),
("x-amz-website-redirect-location", "https://attacker.example/phish"),
("x-amz-storage-class", "REDUCED_REDUNDANCY"),
];
let headers = pr.headers().chain(unsigned.iter().copied());
let resp = send_raw(pr.method(), pr.uri(), headers, Some(b"should-not-be-stored".to_vec())).await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(
status.as_u16(),
403,
"presigned PUT with unsigned x-amz-* headers must be 403, body:\n{body}"
);
assert_error_code(&body, "AccessDenied");
assert!(
body.contains("were not signed"),
"rejection must name unsigned headers as the cause, got:\n{body}"
);
let error = env
.create_s3_client()
.head_object()
.bucket(BUCKET)
.key(key)
.send()
.await
.expect_err("presigned PUT with unsigned x-amz-* headers must not store the object");
assert_eq!(
error.raw_response().map(|response| response.status().as_u16()),
Some(404),
"absence probe after the rejected upload must return HTTP 404, got {error:?}"
);
Ok(())
}
/// GHSA-g8w9-qw9q-fghr positive control: when the presigner itself sets the
/// property, the SDK lists `x-amz-tagging` in `SignedHeaders`, the uploader
/// replays it, and the upload succeeds with the tags applied. Without this the
/// negative test above could pass because the server rejects every tagged
/// presigned upload.
#[tokio::test]
async fn ghsa_g8w9_presigned_put_accepts_signed_x_amz_headers() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
setup(&mut env).await?;
let key = "presigned-put-signed-tagging.txt";
let pr = env
.create_s3_client()
.put_object()
.bucket(BUCKET)
.key(key)
.tagging("owner=app")
.presigned(valid_config())
.await?;
assert!(
pr.headers().any(|(name, _)| name.eq_ignore_ascii_case("x-amz-tagging")),
"fixture must carry x-amz-tagging as a signed header"
);
assert!(
pr.uri().contains("x-amz-tagging"),
"X-Amz-SignedHeaders must list x-amz-tagging, uri: {}",
pr.uri()
);
let resp = send_presigned(&pr, Some(b"stored-with-signed-tagging".to_vec())).await?;
let status = resp.status();
let body = resp.text().await?;
assert!(
status.is_success(),
"presigned PUT with signed x-amz-tagging must succeed, got {status}, body:\n{body}"
);
let tags = env
.create_s3_client()
.get_object_tagging()
.bucket(BUCKET)
.key(key)
.send()
.await?;
let tag_set: Vec<(String, String)> = tags
.tag_set()
.iter()
.map(|tag| (tag.key().to_string(), tag.value().to_string()))
.collect();
assert_eq!(tag_set, vec![("owner".to_string(), "app".to_string())], "signed tagging must be applied");
info!("signed presigned tagging control passed");
Ok(())
}
/// GHSA-g8w9-qw9q-fghr on the read side: a presigned GET signed with
/// `SignedHeaders=host` must not accept an unsigned SSE-C header. The header
/// would otherwise select a decryption path the presigner never authorised.
#[tokio::test]
async fn ghsa_g8w9_presigned_get_rejects_unsigned_x_amz_headers() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
setup(&mut env).await?;
let pr = env
.create_s3_client()
.get_object()
.bucket(BUCKET)
.key(CANONICAL_KEY)
.presigned(valid_config())
.await?;
let unsigned: Vec<(&str, &str)> = vec![("x-amz-server-side-encryption-customer-algorithm", "AES256")];
let headers = pr.headers().chain(unsigned.iter().copied());
let resp = send_raw(pr.method(), pr.uri(), headers, None).await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(
status.as_u16(),
403,
"presigned GET with an unsigned x-amz-* header must be 403, body:\n{body}"
);
assert_error_code(&body, "AccessDenied");
assert!(
!body.contains(std::str::from_utf8(CANONICAL_BODY)?),
"rejected GET must not leak the object body"
);
Ok(())
}
/// GHSA-g8w9-qw9q-fghr: an unsigned `x-amz-copy-source` would turn a presigned
/// PutObject into a CopyObject of an arbitrary readable key, since operation
/// routing happens before authorization. The presigned upload must fail and
/// leave nothing behind.
#[tokio::test]
async fn ghsa_g8w9_presigned_put_rejects_unsigned_copy_source() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
setup(&mut env).await?;
let key = "presigned-put-unsigned-copy-source.txt";
let pr = env
.create_s3_client()
.put_object()
.bucket(BUCKET)
.key(key)
.presigned(valid_config())
.await?;
let copy_source = format!("/{BUCKET}/{CANONICAL_KEY}");
let unsigned: Vec<(&str, &str)> = vec![("x-amz-copy-source", copy_source.as_str())];
let headers = pr.headers().chain(unsigned.iter().copied());
let resp = send_raw(pr.method(), pr.uri(), headers, None).await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(
status.as_u16(),
403,
"presigned PUT with an unsigned copy source must be 403, body:\n{body}"
);
assert_error_code(&body, "AccessDenied");
let error = env
.create_s3_client()
.head_object()
.bucket(BUCKET)
.key(key)
.send()
.await
.expect_err("rejected copy must not create the destination object");
assert_eq!(error.raw_response().map(|response| response.status().as_u16()), Some(404));
Ok(())
}
/// GHSA-g8w9-qw9q-fghr boundary control: the rule covers `x-amz-*` only. A
/// plain `Content-Type` on a `SignedHeaders=host` presigned PUT is outside
/// SigV4's signed-header requirement (AWS S3 accepts it too) and must keep
/// working, so the negative tests above cannot pass by rejecting every
/// unsigned header.
#[tokio::test]
async fn ghsa_g8w9_presigned_put_still_accepts_unsigned_non_amz_headers() -> Result<(), Box<dyn std::error::Error + Send + Sync>>
{
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
setup(&mut env).await?;
let key = "presigned-put-unsigned-content-type.txt";
let pr = env
.create_s3_client()
.put_object()
.bucket(BUCKET)
.key(key)
.presigned(valid_config())
.await?;
let unsigned: Vec<(&str, &str)> = vec![("content-type", "text/x-rustfs-test")];
let headers = pr.headers().chain(unsigned.iter().copied());
let resp = send_raw(pr.method(), pr.uri(), headers, Some(b"plain-header-upload".to_vec())).await?;
let status = resp.status();
let body = resp.text().await?;
assert!(
status.is_success(),
"presigned PUT with an unsigned Content-Type must succeed, got {status}, body:\n{body}"
);
let head = env.create_s3_client().head_object().bucket(BUCKET).key(key).send().await?;
assert_eq!(
head.content_type(),
Some("text/x-rustfs-test"),
"unsigned Content-Type must still be applied"
);
Ok(())
}
+5 -148
View File
@@ -20,7 +20,7 @@ use crate::heal::{
task::{HealOptions, HealPriority, HealRequest, HealTask, HealTaskStatus, HealType, demote_to_debug_when},
};
use crate::{Error, Result};
use metrics::{counter, gauge, histogram};
use metrics::{counter, gauge};
use rustfs_concurrency::WorkloadAdmissionSnapshotProvider;
use rustfs_concurrency::workload::{ForegroundPressure, foreground_pressure};
#[cfg(test)]
@@ -34,7 +34,7 @@ use std::sync::LazyLock;
use std::{
collections::{BinaryHeap, HashMap, HashSet},
sync::{Arc, Mutex as StdMutex, MutexGuard as StdMutexGuard},
time::{Duration, Instant, SystemTime},
time::{Duration, SystemTime},
};
use tokio::{
sync::{Mutex, Notify, RwLock},
@@ -181,13 +181,6 @@ fn lock_displaced_terminals(
}
}
fn lock_admission_telemetry(registry: &StdMutex<HealAdmissionTelemetry>) -> StdMutexGuard<'_, HealAdmissionTelemetry> {
match registry.lock() {
Ok(guard) => guard,
Err(poisoned) => poisoned.into_inner(),
}
}
fn record_displaced_terminal(
registry: &StdMutex<HashMap<String, Arc<CompletedHealStatus>>>,
request: &HealRequest,
@@ -391,61 +384,6 @@ impl HealSourceCounts {
}
}
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct HealAdmissionTelemetry {
pub accepted: u64,
pub merged: u64,
pub full: u64,
pub dropped: u64,
pub duplicate: u64,
pub overlap_rejected: u64,
pub displaced: u64,
pub force_start: u64,
pub max_start_duration_micros: u64,
pub max_lock_phase_micros: u64,
}
impl HealAdmissionTelemetry {
fn record(&mut self, observation: HealAdmissionObservation) {
match observation.result {
HealAdmissionResult::Accepted => self.accepted = self.accepted.saturating_add(1),
HealAdmissionResult::Merged => self.merged = self.merged.saturating_add(1),
HealAdmissionResult::Full => self.full = self.full.saturating_add(1),
HealAdmissionResult::Dropped(_) => self.dropped = self.dropped.saturating_add(1),
}
if observation.context == "duplicate" {
self.duplicate = self.duplicate.saturating_add(1);
}
if observation.context == "overlap_rejected" {
self.overlap_rejected = self.overlap_rejected.saturating_add(1);
}
if observation.displaced {
self.displaced = self.displaced.saturating_add(1);
}
if observation.force_start {
self.force_start = self.force_start.saturating_add(1);
}
self.max_start_duration_micros = self
.max_start_duration_micros
.max(duration_micros_saturated(observation.start_duration));
self.max_lock_phase_micros = self
.max_lock_phase_micros
.max(duration_micros_saturated(observation.lock_phase));
}
}
#[derive(Debug, Clone, Copy)]
struct HealAdmissionObservation {
source: HealRequestSource,
result: HealAdmissionResult,
context: &'static str,
force_start: bool,
displaced: bool,
start_duration: Duration,
lock_phase: Duration,
}
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct HealOperationsSnapshot {
@@ -458,18 +396,12 @@ pub struct HealOperationsSnapshot {
pub queued_by_source: HealSourceCounts,
pub active_by_source: HealSourceCounts,
pub retrying_by_source: HealSourceCounts,
#[serde(default)]
pub admission: HealAdmissionTelemetry,
}
fn usize_to_u64_saturated(value: usize) -> u64 {
u64::try_from(value).unwrap_or(u64::MAX)
}
fn duration_micros_saturated(duration: Duration) -> u64 {
u64::try_from(duration.as_micros()).unwrap_or(u64::MAX)
}
fn heal_type_matches_path(heal_type: &HealType, heal_path: &str) -> bool {
let heal_path = heal_path.trim_matches('/');
if heal_path.is_empty() || heal_path == LEGACY_ROOT_HEAL_PATH {
@@ -832,9 +764,6 @@ pub struct HealManager {
notify: Arc<Notify>,
/// Optional runtime workload snapshot provider used to protect foreground data-plane work.
workload_provider: Option<WorkloadSnapshotProviderRef>,
/// Bounded, low-cardinality admission telemetry exposed through the
/// existing operations snapshot for cluster E2E assertions.
admission_telemetry: Arc<StdMutex<HealAdmissionTelemetry>>,
}
/// Where a task-id lookup resolved. The variants carry the resolved state
@@ -990,33 +919,6 @@ impl HealManager {
.increment(1);
}
fn record_admission_observation(&self, observation: HealAdmissionObservation) {
let result = observation.result.result_label().to_string();
let reason = observation.result.reason_label().to_string();
let source = observation.source.as_str().to_string();
let context = observation.context.to_string();
let force_start = observation.force_start.to_string();
histogram!(
"rustfs_heal_admission_start_duration_seconds",
"source" => source.clone(),
"result" => result.clone(),
"reason" => reason.clone(),
"context" => context.clone(),
"force_start" => force_start.clone()
)
.record(observation.start_duration.as_secs_f64());
histogram!(
"rustfs_heal_admission_lock_phase_seconds",
"source" => source,
"result" => result,
"reason" => reason,
"context" => context,
"force_start" => force_start
)
.record(observation.lock_phase.as_secs_f64());
lock_admission_telemetry(&self.admission_telemetry).record(observation);
}
fn remove_mrf_repair_notice_targets_for_task(&self, task_id: &str) {
let targets = lock_mrf_repair_notice_targets(&self.mrf_repair_notice_targets).remove(task_id);
if let Some(targets) = targets {
@@ -1363,7 +1265,6 @@ impl HealManager {
statistics: Arc::new(RwLock::new(HealStatistics::new())),
notify: Arc::new(Notify::new()),
workload_provider,
admission_telemetry: Arc::new(StdMutex::new(HealAdmissionTelemetry::default())),
}
}
@@ -1554,9 +1455,6 @@ impl HealManager {
preserve_alias: bool,
mrf_notice_target: Option<MrfRepairNoticeTarget>,
) -> Result<HealAdmissionReceipt> {
let admission_start = Instant::now();
let source = request.source;
let force_start = request.force_start;
// HS-06 forceStart semantics (admin only): MinIO stops the old task
// first and then starts the new one. Cancel any active admin task
// overlapping this request's path before entering admission, so the
@@ -1607,7 +1505,6 @@ impl HealManager {
// Match the scheduler's active -> queue order and keep retry ownership
// in the same atomic view. Otherwise queue -> active and
// active -> retrying transitions can slip between duplicate checks.
let lock_phase_start = Instant::now();
let active_heals = self.active_heals.lock().await;
#[cfg(test)]
pause_duplicate_admission_after_active_lock(&request.id).await;
@@ -1642,17 +1539,7 @@ impl HealManager {
drop(retrying_heals);
drop(queue);
drop(active_heals);
let lock_phase = lock_phase_start.elapsed();
Self::record_admission_metric(request.source, admission, "duplicate");
self.record_admission_observation(HealAdmissionObservation {
source,
result: admission,
context: "duplicate",
force_start,
displaced: false,
start_duration: admission_start.elapsed(),
lock_phase,
});
match admission {
HealAdmissionResult::Merged => {
@@ -1731,17 +1618,7 @@ impl HealManager {
drop(retrying_heals);
drop(queue);
drop(active_heals);
let lock_phase = lock_phase_start.elapsed();
Self::record_admission_metric(request.source, HealAdmissionResult::Dropped(reason), "overlap_rejected");
self.record_admission_observation(HealAdmissionObservation {
source,
result: HealAdmissionResult::Dropped(reason),
context: "overlap_rejected",
force_start,
displaced: false,
start_duration: admission_start.elapsed(),
lock_phase,
});
warn!(
target: "rustfs::heal::manager",
event = EVENT_HEAL_QUEUE_ADMISSION,
@@ -1786,8 +1663,6 @@ impl HealManager {
drop(retrying_heals);
drop(queue);
drop(active_heals);
let lock_phase = lock_phase_start.elapsed();
let displaced = displaced_terminal.is_some();
if let (Some(displaced_task_id), Some(displaced_terminal)) = (displaced_task_id, displaced_terminal) {
// The queue has already removed the displaced request, so the
@@ -1801,16 +1676,6 @@ impl HealManager {
self.notify.notify_one();
}
self.record_admission_observation(HealAdmissionObservation {
source,
result: admission,
context: "submit",
force_start,
displaced,
start_duration: admission_start.elapsed(),
lock_phase,
});
Ok(HealAdmissionReceipt {
result: admission,
task_id,
@@ -2246,25 +2111,17 @@ impl HealManager {
}
publish_active_heal_count(&active_heals);
publish_heal_queue_length(&queue);
let queue_length = usize_to_u64_saturated(queue.len());
let active_tasks = usize_to_u64_saturated(active_heals.len());
let retrying_tasks = usize_to_u64_saturated(retrying_heals.len());
drop(retrying_heals);
drop(queue);
drop(active_heals);
let admission = *lock_admission_telemetry(&self.admission_telemetry);
HealOperationsSnapshot {
queue_length,
active_tasks,
retrying_tasks,
queue_length: usize_to_u64_saturated(queue.len()),
active_tasks: usize_to_u64_saturated(active_heals.len()),
retrying_tasks: usize_to_u64_saturated(retrying_heals.len()),
queued_by_priority,
active_by_priority,
retrying_by_priority,
queued_by_source,
active_by_source,
retrying_by_source,
admission,
}
}
-82
View File
@@ -2792,88 +2792,6 @@ async fn admin_force_start_cancels_overlapping_active_task_first() {
);
}
#[tokio::test]
async fn admission_snapshot_tracks_start_duplicate_force_start_and_displacement() {
let storage: Arc<dyn HealStorageAPI> = Arc::new(MockStorage);
let manager = Arc::new(HealManager::new(
storage,
Some(HealConfig {
queue_size: 1,
..Default::default()
}),
));
let mut paused = admin_prefix_request("bucket-a", "logs/");
paused.priority = HealPriority::Low;
let hook = Arc::new(DuplicateAdmissionTestHook {
request_id: paused.id.clone(),
active_lock_reached: Notify::new(),
active_lock_release: Notify::new(),
});
*DUPLICATE_ADMISSION_TEST_HOOK.lock().await = Some(hook.clone());
let submit_manager = Arc::clone(&manager);
let mut paused_submission = tokio::spawn(async move { submit_manager.submit_heal_request(paused).await });
tokio::time::timeout(Duration::from_secs(1), hook.active_lock_reached.notified())
.await
.expect("admission should reach the test-only lock phase hook");
assert!(
tokio::time::timeout(Duration::from_millis(10), &mut paused_submission)
.await
.is_err(),
"admission must wait while the lock-phase hook is held"
);
hook.active_lock_release.notify_one();
assert_eq!(
paused_submission
.await
.expect("paused admission task should join")
.expect("paused admission should succeed"),
HealAdmissionResult::Accepted
);
*DUPLICATE_ADMISSION_TEST_HOOK.lock().await = None;
let duplicate = admin_prefix_request("bucket-a", "logs/");
let duplicate_receipt = manager
.submit_heal_request_with_receipt(duplicate)
.await
.expect("duplicate admission should return a canonical receipt");
assert_eq!(duplicate_receipt.result, HealAdmissionResult::Merged);
let mut high = admin_prefix_request("bucket-b", "logs/");
high.priority = HealPriority::High;
assert_eq!(
manager
.submit_heal_request(high)
.await
.expect("higher priority admin request should displace queued low-priority work"),
HealAdmissionResult::Accepted
);
let mut forced = admin_prefix_request("bucket-c", "logs/");
forced.force_start = true;
assert_eq!(
manager
.submit_heal_request(forced)
.await
.expect("forceStart should keep explicit admission semantics"),
HealAdmissionResult::Accepted
);
let admission = manager.operations_snapshot().await.admission;
assert_eq!(admission.accepted, 3);
assert_eq!(admission.merged, 1);
assert_eq!(admission.full, 0);
assert_eq!(admission.dropped, 0);
assert_eq!(admission.duplicate, 1);
assert_eq!(admission.displaced, 1);
assert_eq!(admission.force_start, 1);
assert!(
admission.max_lock_phase_micros > 0,
"snapshot should expose a measurable queue/admission lock phase for p95-style external aggregation"
);
}
#[tokio::test]
async fn test_operations_snapshot_counts_active_by_source_and_priority() {
let storage: Arc<dyn HealStorageAPI> = Arc::new(MockStorage);
+1 -1
View File
@@ -34,7 +34,7 @@ use storage_api::owner::{
};
pub use erasure_healer::ErasureSetHealer;
pub use manager::{HealAdmissionTelemetry, HealManager, HealOperationsSnapshot, HealPriorityCounts, HealSourceCounts};
pub use manager::{HealManager, HealOperationsSnapshot, HealPriorityCounts, HealSourceCounts};
pub use resume::{CheckpointManager, ResumeCheckpoint, ResumeManager, ResumeState, ResumeUtils};
pub use task::{HealOptions, HealPriority, HealRequest, HealTask, HealType};
+19 -70
View File
@@ -516,7 +516,6 @@ async fn submit_mrf_heal_request(manager: &HealManager, intent: &MrfIntent) -> c
struct MrfRuntime {
queue: MrfQueue,
retained_replay_intents: Vec<MrfIntent>,
config: MrfConsumerConfig,
new_since_flush: usize,
/// True while the in-memory pending set has changed since the last
@@ -536,7 +535,7 @@ impl MrfRuntime {
fn snapshot(&self) -> (Vec<u8>, Vec<u8>) {
let mut authoritative = Vec::new();
let mut legacy = Vec::new();
for intent in self.retained_replay_intents.iter().chain(self.queue.intents()) {
for intent in self.queue.intents() {
let scoped_identity =
!matches!(intent.kind, rustfs_common::mrf_channel::MrfKind::MetadataCorruption) && intent.scope.is_some();
if !encode_intent(intent, &mut authoritative) {
@@ -674,11 +673,10 @@ pub async fn replay_journal_once(manager: &Arc<HealManager>) -> usize {
struct ReplayOutcome {
replayed: usize,
journal_on_disk: bool,
retained_replay_intents: Vec<MrfIntent>,
}
fn replay_must_retain_journal(rearm_incomplete: bool, pending_depth: usize, retained_replay_depth: usize) -> bool {
rearm_incomplete || pending_depth > 0 || retained_replay_depth > 0
fn replay_must_retain_journal(rearm_incomplete: bool, pending_depth: usize) -> bool {
rearm_incomplete || pending_depth > 0
}
/// Shared replay core: read + decode + re-arm, then drain what fits. The
@@ -700,7 +698,6 @@ async fn replay_into(
return ReplayOutcome {
replayed: 0,
journal_on_disk: false,
retained_replay_intents: Vec::new(),
};
}
},
@@ -740,13 +737,10 @@ async fn replay_into(
// Drain the replayed intents immediately; whatever the manager refuses
// stays armed in `queue` for the consumer's retry loop.
let mut retained_replay_intents = Vec::new();
if backoff_until.is_none() {
while let Some(mut intent) = queue.pop_front() {
match submit_mrf_heal_request(manager, &intent).await {
Ok(HealAdmissionResult::Accepted) | Ok(HealAdmissionResult::Merged) => {
retained_replay_intents.push(intent);
}
Ok(HealAdmissionResult::Accepted) | Ok(HealAdmissionResult::Merged) => {}
Ok(HealAdmissionResult::Full) | Ok(HealAdmissionResult::Dropped(HealAdmissionDropReason::QueueFull)) => {
intent.attempts = intent.attempts.saturating_add(1);
if intent.attempts < MRF_MAX_ATTEMPTS {
@@ -775,7 +769,7 @@ async fn replay_into(
}
}
}
let journal_on_disk = if replay_must_retain_journal(rearm_incomplete, queue.depth(), retained_replay_intents.len()) {
let journal_on_disk = if replay_must_retain_journal(rearm_incomplete, queue.depth()) {
true
} else {
!delete_journals().await
@@ -783,7 +777,6 @@ async fn replay_into(
ReplayOutcome {
replayed,
journal_on_disk,
retained_replay_intents,
}
}
@@ -793,7 +786,6 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
let config = MrfConsumerConfig::default();
let mut runtime = MrfRuntime {
queue: MrfQueue::new(config.queue_capacity, config.journal_max_bytes),
retained_replay_intents: Vec::new(),
config: config.clone(),
new_since_flush: 0,
dirty: false,
@@ -805,7 +797,6 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
// on disk whenever any replayed intent still needs a successor snapshot.
let replay = replay_into(&manager, &mut runtime.queue, &mut runtime.backoff_until).await;
runtime.journal_on_disk = replay.journal_on_disk;
runtime.retained_replay_intents = replay.retained_replay_intents;
// Anything still pending (e.g. the manager was full and backoff armed)
// must be re-persisted by the next flush before replay can delete the
// startup anchor.
@@ -823,7 +814,7 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
// provably current AND idle (a dirty or pending state
// gets one last persist attempt, matching the shutdown
// retry the unconditional flush used to provide).
if runtime.dirty || runtime.queue.depth() > 0 || !runtime.retained_replay_intents.is_empty() {
if runtime.dirty || runtime.queue.depth() > 0 {
runtime.flush().await;
}
tracing::info!(
@@ -852,7 +843,6 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
match tick_action(
runtime.dirty,
runtime.queue.depth(),
runtime.retained_replay_intents.len(),
runtime.journal_on_disk,
) {
TickAction::Flush => {
@@ -867,8 +857,8 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
runtime.dispatch(manager.as_ref()).await;
}
TickAction::DeleteJournal => {
// Only remove a stale journal after every replayed
// intent has a durable successor proof.
// All replayed intents have either been accepted,
// merged, or replaced by a pending successor snapshot.
if delete_journals().await {
runtime.journal_on_disk = false;
gauge!("rustfs_heal_mrf_journal_bytes").set(0.0);
@@ -897,13 +887,11 @@ enum TickAction {
Idle,
}
fn tick_action(dirty: bool, depth: usize, retained_replay_depth: usize, journal_on_disk: bool) -> TickAction {
fn tick_action(dirty: bool, depth: usize, journal_on_disk: bool) -> TickAction {
if dirty {
TickAction::Flush
} else if depth > 0 {
TickAction::Retry
} else if retained_replay_depth > 0 {
TickAction::Idle
} else if journal_on_disk {
TickAction::DeleteJournal
} else {
@@ -936,73 +924,34 @@ mod tests {
// Dirty dominates: a changed pending set flushes even when idle
// otherwise.
assert!(matches!(tick_action(true, 0, 0, false), Flush));
assert!(matches!(tick_action(true, 3, 0, true), Flush));
assert!(matches!(tick_action(true, 0, false), Flush));
assert!(matches!(tick_action(true, 3, true), Flush));
// Clean backlog: no rewrite, but keep draining so an expired
// admission backoff retries on time.
assert!(matches!(tick_action(false, 1, 0, false), Retry));
assert!(matches!(tick_action(false, 2, 0, true), Retry));
// Replayed records accepted by the manager are still restart anchors
// until a durable successor proof can tombstone them.
assert!(matches!(tick_action(false, 0, 1, true), Idle));
assert!(matches!(tick_action(false, 1, false), Retry));
assert!(matches!(tick_action(false, 2, true), Retry));
// Quiescent with a stale journal file on disk: remove it.
assert!(matches!(tick_action(false, 0, 0, true), DeleteJournal));
assert!(matches!(tick_action(false, 0, true), DeleteJournal));
// Fully quiescent: nothing to do.
assert!(matches!(tick_action(false, 0, 0, false), Idle));
assert!(matches!(tick_action(false, 0, false), Idle));
}
#[test]
fn replay_cleanup_retains_journal_for_unarmed_or_refused_records() {
assert!(
replay_must_retain_journal(true, 0, 0),
replay_must_retain_journal(true, 0),
"a rejected replay record still needs its disk anchor"
);
assert!(
replay_must_retain_journal(false, 1, 0),
replay_must_retain_journal(false, 1),
"a Full admission retry must keep the startup journal until the next snapshot"
);
assert!(
replay_must_retain_journal(false, 0, 1),
"an accepted replay record still needs a durable successor before cleanup"
);
assert!(
!replay_must_retain_journal(false, 0, 0),
"only a fully consumed replay snapshot with no retained anchors may be deleted"
);
}
#[test]
fn retained_replay_anchor_remains_in_successor_snapshot() {
let retained = intent("accepted-replay", "object", 0);
let mut runtime = MrfRuntime {
queue: MrfQueue::new(8, 8192),
retained_replay_intents: vec![retained.clone()],
config: MrfConsumerConfig::default(),
new_since_flush: 0,
dirty: false,
journal_on_disk: true,
backoff_until: None,
};
assert_eq!(
runtime.queue.try_push_typed(intent("new-pending", "object", 0)),
MrfQueuePushResult::Enqueued
);
let (authoritative, legacy) = runtime.snapshot();
let (decoded, truncated) = decode_journal(&authoritative);
let (legacy_decoded, legacy_truncated) = decode_journal(&legacy);
assert_eq!(truncated, 0);
assert_eq!(legacy_truncated, 0);
assert_eq!(decoded.len(), 2);
assert_eq!(legacy_decoded.len(), 2);
assert!(
decoded.iter().any(|intent| intent.bucket == retained.bucket),
"accepted replay anchor must remain crash-replayable"
!replay_must_retain_journal(false, 0),
"only a fully consumed replay snapshot may be deleted"
);
}
+1 -2
View File
@@ -19,8 +19,7 @@ pub mod heal;
pub use error::{Error, Result};
pub use heal::{
HealAdmissionTelemetry, HealManager, HealOperationsSnapshot, HealOptions, HealPriority, HealPriorityCounts, HealRequest,
HealSourceCounts, HealType,
HealManager, HealOperationsSnapshot, HealOptions, HealPriority, HealPriorityCounts, HealRequest, HealSourceCounts, HealType,
channel::HealChannelProcessor,
progress::{HealProgress, aggregate_heal_progress},
resume::{ReplacementRecoveryRecord, ReplacementRecoveryState, ResumeUtils},
+6 -3
View File
@@ -51,14 +51,17 @@ prior key versions would go green.
The lane creates real keys under unique names (`behavior-kv2-*`,
`behavior-transit-*`) and does not remove them, so a dev Vault accumulates them
across runs. Clear them out periodically — against a dev server only:
across runs. On a dev server, remove only exact keys confirmed to belong to the
current task. A shared prefix does not prove ownership; preserve another run's
keys and leave ambiguous keys for the operator.
```bash
vault list -format=json transit/keys | jq -r '.[] | select(startswith("behavior-transit-"))' | while read -r k; do vault write "transit/keys/$k/config" deletion_allowed=true >/dev/null && vault delete "transit/keys/$k"; done
vault write transit/keys/<task-owned-transit-key>/config deletion_allowed=true
vault delete transit/keys/<task-owned-transit-key>
```
```bash
vault list -format=json secret/metadata/rustfs/kms/keys | jq -r '.[] | select(startswith("behavior-kv2-"))' | xargs -I{} vault kv metadata delete secret/rustfs/kms/keys/{}
vault kv metadata delete secret/rustfs/kms/keys/<task-owned-kv2-key>
```
## Local Key Export for SSE-S3 Migration Tests
+7 -7
View File
@@ -15,14 +15,14 @@ Keep changes narrow and source-driven.
## Source of Truth
Before changing behavior, read these files first:
Start with the file that owns the changed behavior; follow callers and shared
types only as needed:
- `src/scan.rs`
- `src/capacity_manager.rs`
- `src/capacity_scope.rs`
- `src/types.rs`
- `../../rustfs/src/capacity/service.rs`
- `../config/src/constants/capacity.rs`
- Scan/sampling: `src/scan.rs`.
- Refresh and per-disk cache state: `src/capacity_manager.rs`.
- Scope propagation: `src/capacity_scope.rs`; shared data shapes: `src/types.rs`.
- Admin integration: `../../rustfs/src/capacity/service.rs`.
- Configuration/defaults: `../config/src/constants/capacity.rs`.
Do not treat shell scripts or old docs as the authoritative behavior definition when the Rust code says otherwise.
+1 -1
View File
@@ -60,6 +60,6 @@ Required headings and strings in these files are asserted by `scripts/check_arch
| [minio-rustfs-router-compatibility.md](minio-rustfs-router-compatibility.md) | a client or `mc` call that works against MinIO fails against RustFS and you need to know whether the endpoint is missing, stubbed, or deliberately different |
| [minio-file-format-compat.md](minio-file-format-compat.md) | deciding whether a MinIO drive set, bucket-metadata blob, or SSE object can be read or imported by a given RustFS build, or before touching a listed version anchor |
Operations runbooks live in [../operations/](../README.md#operations) and testing references in [../testing/README.md](../testing/README.md).
Operations runbooks live in [../operations/](../operations/) and testing references in [../testing/README.md](../testing/README.md).
For per-node HTTP failure ratios and cached storage probe provenance, see [S3 write failure diagnostics](../operations/s3-write-failure-diagnostics.md).
+1 -1
View File
@@ -1,4 +1,3 @@
| Outbound target matrix | Replication of every object shape (empty, plain, retention, legal hold, multipart) against every remote-target failure mode the fake target models; an explicit expectation table pins known-red cells to an open issue | `cargo nextest run -p e2e_test -E 'test(/^replication_target_matrix_test::/)'` (build `target/debug/rustfs` first) | With `e2e-repl-nightly`; required locally for any change to outbound client defaults (SOP: [`docs/postmortems/2026-09-03-replication-checksum-default-regression.md`](../postmortems/2026-09-03-replication-checksum-default-regression.md)) |
# RustFS Testing
**Use this when:** you need to pick a test layer for a change, name a test so a gate keeps selecting it, understand why `#[serial]` does nothing under nextest, or handle a flaky test.
@@ -13,6 +12,7 @@ Pick the lowest layer that can prove the change; add a higher-layer test only wh
| Unit & crate integration | Per-crate logic and in-process integration tests | `cargo nextest run --all --exclude e2e_test` (or `-p <crate>`); `make test` wraps it | Every PR, required (`Test and Lint`, `ci` profile) |
| ecstore black-box | Erasure-coded read/write/recovery validation; profiles `quick` / `full` / `destructive` / `fuzz` | `scripts/run_ecstore_validation_suite.sh --profile quick` | Local and release validation only; not wired into any workflow. Contract: [ecstore-validation-suite-design.md](ecstore-validation-suite-design.md) |
| e2e (`e2e_test` crate) | A real `rustfs` binary per test, driven over the S3, admin, and protocol APIs | `cargo nextest run --profile e2e-smoke -p e2e_test` | PR: `e2e-smoke` (report-only); merge queue / main push: `e2e-full`; nightly: `e2e-repl-nightly`, `e2e-nightly`, `e2e-protocols`, `e2e-distributed`. Guide: [`crates/e2e_test/README.md`](../../crates/e2e_test/README.md); 4-node 4-disk map: [distributed-e2e.md](distributed-e2e.md) |
| Outbound target matrix | Replication of every object shape (empty, plain, retention, legal hold, multipart) against every remote-target failure mode the fake target models; an explicit expectation table pins known-red cells to an open issue | `cargo nextest run -p e2e_test -E 'test(/^replication_target_matrix_test::/)'` (build `target/debug/rustfs` first) | With `e2e-repl-nightly`; required locally for any change to outbound client defaults (SOP: [`docs/postmortems/2026-09-03-replication-checksum-default-regression.md`](../postmortems/2026-09-03-replication-checksum-default-regression.md)) |
| s3s-e2e conformance | External S3 conformance tool against a live server | `./scripts/e2e-run.sh ./target/debug/rustfs <data-dir>` | PR, report-only (second half of the `End-to-End Tests` job) |
| S3 compatibility | `ceph/s3-tests` (boto3; allow-list `scripts/s3-tests/implemented_tests.txt`) and MinIO `mint` | `scripts/s3-tests/run.sh`; mint via `.github/workflows/mint.yml` | s3-tests: PR report-only plus a weekly full sweep; mint: weekly, report-only |
| Chaos / fault-injection | Single-node disk fault injection (`crates/e2e_test/src/chaos.rs`, `crates/e2e_test/src/fault_proxy.rs`) plus the 4-node kill/fresh-drive/blackhole cases in `crates/e2e_test/src/distributed/chaos_test.rs` | Part of the e2e crate (`e2e-reliability` and `e2e-distributed`) | Reliability cases with `e2e-full`; 4-node chaos on storage-sensitive PRs and nightly via `e2e-distributed` |
+2 -1
View File
@@ -17,13 +17,14 @@ Every fixed RustFS GitHub Security Advisory maps to at least one named regressio
| [GHSA-v9cp-qfw9-9pfp](https://github.com/rustfs/rustfs/security/advisories/GHSA-v9cp-qfw9-9pfp) | `ForAllValues:`/`ForAnyValue:` negated string operators applied negation to the aggregate instead of the per-value predicate | fixed, GHSA private-fork merge | `ghsa_v9cp_for_all_values_not_equals_partial_overlap`, `ghsa_v9cp_for_any_value_not_equals_partial_overlap` and the absent-key/positive-quantifier cases beside them (`crates/policy/tests/quantified_negation.rs`); the value set must partially overlap the policy set, since contained or disjoint sets cannot tell the quantifiers apart | crate test |
| [GHSA-6r96-hmgc-726c](https://github.com/rustfs/rustfs/security/advisories/GHSA-6r96-hmgc-726c) | Request headers must not populate server-derived IAM condition keys (`userid`, `groups`, `jwt:`/`ldap:` claims) | fixed, GHSA private-fork merge | `ghsa_6r96_identity_condition_keys_ignore_spoofed_headers`, `ghsa_6r96_claim_condition_keys_ignore_spoofed_headers`, and `test_request_headers_still_reach_conditions`, which keeps the reserved set from growing too broad (`rustfs/src/auth.rs`) | unit |
| [GHSA-x298-9x87-fvjq](https://github.com/rustfs/rustfs/security/advisories/GHSA-x298-9x87-fvjq) | Anonymous ListObjectVersions -> `s3:ListBucket` fallback must reach the same public-access gates as a direct grant | fixed, GHSA private-fork merge | `ghsa_x298_anonymous_list_object_versions_denied_when_restrict_public_buckets_enabled` (`crates/e2e_test/src/anonymous_access_test.rs`); asserts 200 before the public-access block is applied so it proves the gate, not a broken fallback | e2e (`e2e-smoke`) |
| [GHSA-g8w9-qw9q-fghr](https://github.com/rustfs/rustfs/security/advisories/GHSA-g8w9-qw9q-fghr) | A SigV4 presigned request must reject `x-amz-*` headers missing from `X-Amz-SignedHeaders` (tags, storage class, ACL, metadata, redirect, Object Lock, SSE) instead of applying them | this fix | `ghsa_g8w9_presigned_request_rejects_unsigned_x_amz_headers`, `ghsa_g8w9_presigned_request_accepts_signed_or_exempt_x_amz_headers`, `ghsa_g8w9_check_ignores_header_signed_sigv2_and_anonymous_requests` (`rustfs/src/auth.rs`); `ghsa_g8w9_check_access_rejects_unsigned_amz_header_on_presigned_custom_route` for routes that bypass `S3Access::check` (`rustfs/src/admin/router.rs`); `ghsa_g8w9_presigned_put_rejects_unsigned_x_amz_headers`, `ghsa_g8w9_presigned_get_rejects_unsigned_x_amz_headers`, `ghsa_g8w9_presigned_put_rejects_unsigned_copy_source`, plus the signed-tagging control `ghsa_g8w9_presigned_put_accepts_signed_x_amz_headers` and the unsigned-`Content-Type` boundary control `ghsa_g8w9_presigned_put_still_accepts_unsigned_non_amz_headers` (`crates/e2e_test/src/presigned_negative_test.rs`) | unit; e2e (`e2e-smoke`) |
| [GHSA-g3vq-vv42-f647](https://github.com/rustfs/rustfs/security/advisories/GHSA-g3vq-vv42-f647) | FTPS `MKD` must clear the `s3:CreateBucket` authorization boundary before reaching the backend | fixed, GHSA private-fork merge | `ghsa_g3vq_mkd_denied_before_reaching_backend` (`crates/protocols/src/ftps/driver.rs`); primes `create_bucket` to succeed so the assertion distinguishes "denied at authorization" from "backend refused" | unit (`ftps` feature) |
## Where these run
| Layer | Command | Lane | Guard |
| --- | --- | --- | --- |
| Unit and crate tests (`ghsa_r5qv_*`, the m77q pins, `ghsa_5354_*`, `ghsa_3ppv_*`, `ghsa_6r96_*`, `ghsa_v9cp_*`, `ghsa_g3vq_*`) | `cargo nextest run --profile ci --all --exclude e2e_test` | every PR, `Test and Lint` (required) | none needed; the workspace pass runs every unit and crate test |
| Unit and crate tests (`ghsa_r5qv_*`, the m77q pins, `ghsa_5354_*`, `ghsa_3ppv_*`, `ghsa_6r96_*`, `ghsa_v9cp_*`, `ghsa_g3vq_*`, `ghsa_g8w9_*`) | `cargo nextest run --profile ci --all --exclude e2e_test` | every PR, `Test and Lint` (required) | none needed; the workspace pass runs every unit and crate test |
| S3-API negative-auth e2e (`negative_sigv4_test`, `presigned_negative_test`, `admin_auth_test`) | `cargo nextest run --profile e2e-smoke -p e2e_test` | every PR, `End-to-End Tests` (report-only) | `scripts/check_security_smoke_count.sh` with the floor in `.config/security-smoke-floor.txt`, run in the `e2e-tests` job; fails when a rename drops one of these modules out of the smoke filter |
| Other S3 e2e guards (`anonymous_access_test`) | `cargo nextest run --profile e2e-smoke -p e2e_test` | every PR, `End-to-End Tests` (report-only) | `scripts/check_test_wiring.py --check-profile e2e-smoke` digest |
| Protocol e2e (`protocols::test_protocol_core_suite`, GHSA-3p3x) | `RUSTFS_BUILD_FEATURES=ftps,webdav,sftp cargo nextest run -j 1 --profile e2e-protocols -p e2e_test` | nightly, `e2e-replication-nightly.yml` job `protocols-nightly`; not PR-gated | `scripts/check_test_wiring.py --check-profile e2e-protocols` digest |
-30
View File
@@ -339,19 +339,6 @@ fn add_source_counts(total: &mut rustfs_heal::HealSourceCounts, next: rustfs_hea
total.mrf = total.mrf.saturating_add(next.mrf);
}
fn add_admission_telemetry(total: &mut rustfs_heal::HealAdmissionTelemetry, next: rustfs_heal::HealAdmissionTelemetry) {
total.accepted = total.accepted.saturating_add(next.accepted);
total.merged = total.merged.saturating_add(next.merged);
total.full = total.full.saturating_add(next.full);
total.dropped = total.dropped.saturating_add(next.dropped);
total.duplicate = total.duplicate.saturating_add(next.duplicate);
total.overlap_rejected = total.overlap_rejected.saturating_add(next.overlap_rejected);
total.displaced = total.displaced.saturating_add(next.displaced);
total.force_start = total.force_start.saturating_add(next.force_start);
total.max_start_duration_micros = total.max_start_duration_micros.max(next.max_start_duration_micros);
total.max_lock_phase_micros = total.max_lock_phase_micros.max(next.max_lock_phase_micros);
}
fn add_operations(total: &mut rustfs_heal::HealOperationsSnapshot, next: rustfs_heal::HealOperationsSnapshot) {
total.queue_length = total.queue_length.saturating_add(next.queue_length);
total.active_tasks = total.active_tasks.saturating_add(next.active_tasks);
@@ -362,7 +349,6 @@ fn add_operations(total: &mut rustfs_heal::HealOperationsSnapshot, next: rustfs_
add_source_counts(&mut total.queued_by_source, next.queued_by_source);
add_source_counts(&mut total.active_by_source, next.active_by_source);
add_source_counts(&mut total.retrying_by_source, next.retrying_by_source);
add_admission_telemetry(&mut total.admission, next.admission);
}
fn aggregate_cluster_heal_status(snapshots: Vec<NodeHealStatusSnapshot>) -> ClusterHealStatusSnapshot {
@@ -2321,10 +2307,6 @@ mod tests {
assert!(json["healOperations"]["queuedBySource"]["admin"].is_u64());
assert!(json["healOperations"]["queuedByPriority"]["low"].is_u64());
assert!(json["healOperations"]["queuedByPriority"]["high"].is_u64());
assert!(json["healOperations"]["admission"]["accepted"].is_u64());
assert!(json["healOperations"]["admission"]["duplicate"].is_u64());
assert!(json["healOperations"]["admission"]["forceStart"].is_u64());
assert!(json["healOperations"]["admission"]["maxLockPhaseMicros"].is_u64());
assert_eq!(json["state"], "active");
assert_eq!(json["clusterStatusComplete"], true);
assert!(json["progress"].is_null());
@@ -2504,18 +2486,6 @@ mod tests {
queued_by_source: sources(value),
active_by_source: sources(value),
retrying_by_source: sources(value),
admission: rustfs_heal::HealAdmissionTelemetry {
accepted: value,
merged: value,
full: value,
dropped: value,
duplicate: value,
overlap_rejected: value,
displaced: value,
force_start: value,
max_start_duration_micros: value,
max_lock_phase_micros: value,
},
};
let progress = |value| NodeHealProgress {
objects_scanned: value,
+38 -1
View File
@@ -34,7 +34,7 @@ use crate::admin::runtime_sources::{
};
use crate::admin::storage_api::access::{ReqInfo, authorize_request, spawn_traced};
use crate::admin::storage_api::contract::bucket::{BucketOperations, BucketOptions};
use crate::auth::{check_key_valid, constant_time_eq, get_session_token};
use crate::auth::{check_key_valid, constant_time_eq, get_session_token, reject_unsigned_amz_headers_on_presigned_request};
use crate::error::ApiError;
use crate::license::license_check;
use crate::server::{
@@ -3269,6 +3269,11 @@ where
// check_access before call
async fn check_access(&self, req: &mut S3Request<Body>) -> S3Result<()> {
// GHSA-g8w9-qw9q-fghr: custom routes bypass `S3Access::check`, so the
// presigned signed-header rule is enforced here as well. A request
// without a presigned signature passes through untouched.
reject_unsigned_amz_headers_on_presigned_request(&req.headers, req.uri.query())?;
if let Some(server_ctx) = &self.server_ctx {
req.extensions.insert(server_ctx.clone());
if !is_public_health_path(req.uri.path()) && server_ctx.installed_app_context().is_none() {
@@ -5611,6 +5616,38 @@ mod tests {
assert_eq!(err.code(), &S3ErrorCode::AccessDenied);
}
/// GHSA-g8w9-qw9q-fghr: custom routes must apply the presigned
/// signed-header rule too, since they never reach `S3Access::check`.
#[tokio::test]
async fn ghsa_g8w9_check_access_rejects_unsigned_amz_header_on_presigned_custom_route() {
let router: S3Router<AdminOperation> = S3Router::new(false);
let mut headers = HeaderMap::new();
headers.insert("x-amz-tagging", HeaderValue::from_static("owner=attacker"));
let mut req = S3Request {
input: Body::from(String::new()),
method: Method::GET,
uri: "/demo-bucket?replication-metrics&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20260827T000000Z&X-Amz-Expires=900&X-Amz-SignedHeaders=host&X-Amz-Credential=test%2F20260827%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Signature=signature"
.parse()
.expect("uri should parse"),
headers,
extensions: http::Extensions::new(),
credentials: Some(s3s::auth::Credentials {
access_key: "test".into(),
secret_key: s3s::auth::SecretKey::from("secret".to_string()),
}),
region: None,
service: None,
trailing_headers: None,
};
let err = router
.check_access(&mut req)
.await
.expect_err("presigned custom-route request with an unsigned x-amz header must be denied");
assert_eq!(err.code(), &S3ErrorCode::AccessDenied);
assert_eq!(err.message(), Some(crate::auth::UNSIGNED_HEADERS_MESSAGE));
}
// backlog#1052 S2: the router hands its server's context slot to every
// dispatched request via extensions, so the static admin operations can
// resolve their server's store instead of the process default.
+217
View File
@@ -50,6 +50,7 @@ const EVENT_KEYSTONE_CREDENTIALS_DETECTED: &str = "keystone_credentials_detected
const EVENT_KEYSTONE_CREDENTIALS_VALIDATED: &str = "keystone_credentials_validated";
const EVENT_KEYSTONE_CONTEXT_MISSING: &str = "keystone_context_missing";
const EVENT_SESSION_TOKEN_EXTRACTION: &str = "session_token_extraction";
const EVENT_PRESIGNED_UNSIGNED_AMZ_HEADER: &str = "presigned_unsigned_amz_header";
/// RustFS-specific query capability for a single presigned PutObject request.
pub(crate) const RUSTFS_MAX_CONTENT_LENGTH_QUERY: &str = "x-rustfs-max-content-length";
@@ -1031,6 +1032,102 @@ pub fn get_query_param<'a>(query: &'a str, param_name: &str) -> Option<&'a str>
None
}
/// `x-amz-*` request headers a SigV4 presigned request may carry without
/// listing them in `X-Amz-SignedHeaders`.
///
/// CloudFront stamps `x-amz-cf-id` on every origin request it forwards, so a
/// presigned URL served through a CDN could never be honoured if that header
/// had to be signed; nothing in RustFS reads it, so it cannot change what the
/// request does.
const PRESIGNED_UNSIGNED_AMZ_HEADER_ALLOWLIST: &[&str] = &["x-amz-cf-id"];
pub(crate) const UNSIGNED_HEADERS_MESSAGE: &str = "There were headers present in the request which were not signed";
/// GHSA-g8w9-qw9q-fghr: reject `x-amz-*` request headers that a SigV4 presigned
/// URL did not sign.
///
/// A presigned URL is a bounded capability: the presigner authorises one
/// method, key, expiry and the header set named in `X-Amz-SignedHeaders`. The
/// upstream verifier only proves that the signed headers match; any other
/// `x-amz-*` header (tagging, storage class, ACL, metadata, website redirect,
/// Object Lock, SSE selection) would still reach the handlers and take effect,
/// so the untrusted holder of an upload URL could set object properties the
/// presign never covered. AWS S3 rejects such a request with `AccessDenied`
/// ("There were headers present in the request which were not signed"); this
/// check mirrors that at the access boundary, before any handler reads a
/// header.
///
/// Only query-string SigV4 requests are checked. SigV2 canonicalises every
/// `x-amz-*` header into the string to sign, so adding one there already breaks
/// the signature, and a header-signed SigV4 request is sent by the credential
/// holder itself, so an unsigned header there is not a delegation bypass.
///
/// Detection keys on the query, not on the derived [`AuthType`], because the
/// upstream verifier dispatches to the presigned path whenever the query
/// carries `X-Amz-Signature`, even if an `Authorization` header is present too.
/// The rule relies on the verifier signing every query parameter except the
/// signature itself, so neither `X-Amz-SignedHeaders` nor a property-carrying
/// query parameter can be added after presigning.
pub(crate) fn reject_unsigned_amz_headers_on_presigned_request(header: &HeaderMap, query: Option<&str>) -> S3Result<()> {
let Some(query) = query else {
return Ok(());
};
// Presence detection is case-insensitive so a query the upstream verifier
// would not treat as presigned still fails closed here; the signed list is
// read with the exact key the verifier uses (`X-Amz-SignedHeaders`, unique),
// so both sides always see the same list. A duplicate or missing key
// yields an empty list, which signs nothing.
let mut is_presigned_v4 = false;
let mut signed_headers: Option<String> = None;
let mut duplicate_signed_headers = false;
for (name, value) in form_urlencoded::parse(query.as_bytes()) {
if name.eq_ignore_ascii_case("x-amz-signature") {
is_presigned_v4 = true;
} else if name == "X-Amz-SignedHeaders" {
if signed_headers.is_some() {
duplicate_signed_headers = true;
}
signed_headers = Some(value.into_owned());
}
}
if !is_presigned_v4 {
return Ok(());
}
if duplicate_signed_headers {
signed_headers = None;
}
let signed: Vec<String> = signed_headers
.as_deref()
.unwrap_or_default()
.split(';')
.map(|name| name.trim().to_ascii_lowercase())
.filter(|name| !name.is_empty())
.collect();
for name in header.keys() {
// `HeaderName` is already lowercase.
let name = name.as_str();
if !name.starts_with("x-amz-") || PRESIGNED_UNSIGNED_AMZ_HEADER_ALLOWLIST.contains(&name) {
continue;
}
if !signed.iter().any(|signed_name| signed_name == name) {
warn!(
event = EVENT_PRESIGNED_UNSIGNED_AMZ_HEADER,
component = LOG_COMPONENT_AUTH,
subsystem = LOG_SUBSYSTEM_REQUEST,
reason = "unsigned_amz_header",
header = name,
"Presigned request rejected"
);
return Err(S3Error::with_message(S3ErrorCode::AccessDenied, UNSIGNED_HEADERS_MESSAGE.to_string()));
}
}
Ok(())
}
/// Parse the RustFS presigned PutObject size capability after authentication.
///
/// The query value is covered by SigV4 when it is present before presigning, but
@@ -1914,6 +2011,126 @@ mod tests {
);
}
/// GHSA-g8w9-qw9q-fghr: `x-amz-*` request headers that are not listed in
/// `X-Amz-SignedHeaders` must not survive the presigned access boundary.
#[test]
fn ghsa_g8w9_presigned_request_rejects_unsigned_x_amz_headers() {
let presigned_host_only = "X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20260827T000000Z&X-Amz-Expires=900&X-Amz-SignedHeaders=host&X-Amz-Credential=test/20260827/us-east-1/s3/aws4_request&X-Amz-Signature=signature";
for header in [
"x-amz-tagging",
"x-amz-website-redirect-location",
"x-amz-storage-class",
"x-amz-acl",
"x-amz-meta-owner",
"x-amz-object-lock-mode",
"x-amz-server-side-encryption",
] {
let mut headers = HeaderMap::new();
headers.insert("content-type", HeaderValue::from_static("text/plain"));
headers.insert(header, HeaderValue::from_static("attacker-controlled"));
let error = reject_unsigned_amz_headers_on_presigned_request(&headers, Some(presigned_host_only)).unwrap_err();
assert_eq!(error.code(), &S3ErrorCode::AccessDenied, "{header} must be rejected when unsigned");
assert_eq!(error.message(), Some(UNSIGNED_HEADERS_MESSAGE));
}
// Non-`x-amz-*` headers are outside the SigV4 rule and stay allowed.
let mut headers = HeaderMap::new();
headers.insert("content-type", HeaderValue::from_static("text/plain"));
headers.insert("cache-control", HeaderValue::from_static("no-store"));
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(presigned_host_only)).unwrap();
// A missing SignedHeaders list signs nothing and still fails closed.
let missing_signed_headers = presigned_host_only.replace("&X-Amz-SignedHeaders=host", "");
let mut headers = HeaderMap::new();
headers.insert("x-amz-tagging", HeaderValue::from_static("a=b"));
assert_eq!(
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&missing_signed_headers))
.unwrap_err()
.code(),
&S3ErrorCode::AccessDenied
);
// Detection follows the upstream dispatch: any query carrying the
// signature is a presigned request, whatever the key's case.
let lowercase_query = presigned_host_only.to_ascii_lowercase();
assert_eq!(
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&lowercase_query))
.unwrap_err()
.code(),
&S3ErrorCode::AccessDenied
);
// Only the exact key the upstream verifier reads counts; a second
// (or differently cased) list must not widen the signed set, and a
// duplicate exact key signs nothing at all.
let widened_by_case = format!("{presigned_host_only}&x-amz-signedheaders=host%3Bx-amz-tagging");
assert_eq!(
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&widened_by_case))
.unwrap_err()
.code(),
&S3ErrorCode::AccessDenied
);
let duplicated = format!("{presigned_host_only}&X-Amz-SignedHeaders=host%3Bx-amz-tagging");
assert_eq!(
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&duplicated))
.unwrap_err()
.code(),
&S3ErrorCode::AccessDenied
);
}
#[test]
fn ghsa_g8w9_presigned_request_accepts_signed_or_exempt_x_amz_headers() {
let signed_tagging = "X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20260827T000000Z&X-Amz-Expires=900&X-Amz-SignedHeaders=host%3Bx-amz-tagging%3Bx-amz-meta-owner&X-Amz-Credential=test/20260827/us-east-1/s3/aws4_request&X-Amz-Signature=signature";
let mut headers = HeaderMap::new();
headers.insert("x-amz-tagging", HeaderValue::from_static("owner=app"));
headers.insert("X-Amz-Meta-Owner", HeaderValue::from_static("app"));
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(signed_tagging)).unwrap();
// Case differences in the signed list do not matter; header names are
// canonicalised to lowercase on both sides.
let uppercase_list = signed_tagging.replace("x-amz-tagging", "X-Amz-Tagging");
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&uppercase_list)).unwrap();
// The CDN request id is the only unsigned `x-amz-*` header tolerated.
headers.insert("x-amz-cf-id", HeaderValue::from_static("cloudfront-request-id"));
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(signed_tagging)).unwrap();
// Adding one more unsigned header on top of signed ones still fails.
headers.insert("x-amz-storage-class", HeaderValue::from_static("REDUCED_REDUNDANCY"));
assert_eq!(
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(signed_tagging))
.unwrap_err()
.code(),
&S3ErrorCode::AccessDenied
);
}
#[test]
fn ghsa_g8w9_check_ignores_header_signed_sigv2_and_anonymous_requests() {
let mut headers = HeaderMap::new();
headers.insert("x-amz-tagging", HeaderValue::from_static("owner=app"));
headers.insert("x-amz-storage-class", HeaderValue::from_static("STANDARD"));
// No query at all: nothing to bind against.
reject_unsigned_amz_headers_on_presigned_request(&headers, None).unwrap();
// Header-signed SigV4 and SigV2 carry no `X-Amz-Signature` query.
headers.insert(
"authorization",
HeaderValue::from_static(
"AWS4-HMAC-SHA256 Credential=test/20260827/us-east-1/s3/aws4_request, SignedHeaders=host, Signature=abc",
),
);
reject_unsigned_amz_headers_on_presigned_request(&headers, Some("versioning=")).unwrap();
// SigV2 presigned URLs sign every `x-amz-*` header in the string to sign.
let sigv2_query = "AWSAccessKeyId=test&Expires=1893456000&Signature=abc";
reject_unsigned_amz_headers_on_presigned_request(&headers, Some(sigv2_query)).unwrap();
}
#[test]
fn presigned_put_max_content_length_rejects_unsigned_or_invalid_values() {
let headers = HeaderMap::new();
+8 -2
View File
@@ -20,6 +20,7 @@ use crate::auth::{
VerifiedSigV4Request, check_key_valid_with_context, get_condition_values_with_client_info,
get_condition_values_with_query_and_client_info, get_request_auth_type_with_query, get_session_token,
parse_presigned_multipart_max_total_object_size, parse_presigned_put_max_content_length,
reject_unsigned_amz_headers_on_presigned_request,
};
use crate::error::ApiError;
use crate::license::license_check;
@@ -1792,6 +1793,11 @@ fn validate_post_object_success_controls(input: &PostObjectInput) -> S3Result<()
#[async_trait::async_trait]
impl S3Access for FS {
async fn check(&self, cx: &mut S3AccessContext<'_>) -> S3Result<()> {
// GHSA-g8w9-qw9q-fghr: a presigned URL only authorises the headers it
// signed. Reject unsigned `x-amz-*` headers first, before the session
// token lookup below or any handler reads a request header.
reject_unsigned_amz_headers_on_presigned_request(cx.headers(), cx.uri().query())?;
// Upper layer has verified ak/sk
// info!(
// "s3 check uri: {:?}, method: {:?} path: {:?}, s3_op: {:?}, cred: {:?}, headers:{:?}",
@@ -1836,11 +1842,11 @@ impl S3Access for FS {
..Default::default()
};
// Publish this server's context slot so downstream data-plane handlers
// resolve the same store (backlog#1052 S6).
let auth_type = get_request_auth_type_with_query(cx.headers(), cx.uri().query());
let verified_presigned = matches!(auth_type, AuthType::Presigned);
let verified_sigv4 = matches!(auth_type, AuthType::Presigned | AuthType::Signed);
// Publish this server's context slot so downstream data-plane handlers
// resolve the same store (backlog#1052 S6).
{
let ext = cx.extensions_mut();
ext.insert(self.server_ctx().clone());
@@ -726,7 +726,6 @@ mod tests {
assert_eq!(decoded.info().bitrot_start_cycle, 9);
assert_eq!(decoded.operations.queue_length, 2);
assert_eq!(decoded.operations.queued_by_source.mrf, 0);
assert_eq!(decoded.operations.admission, rustfs_heal::HealAdmissionTelemetry::default());
let progress = decoded.progress.expect("legacy progress should decode");
assert_eq!(progress.objects_scanned, 7);
assert!(!progress.baseline_known);