Skip to content

fix(security): reject empty credential secrets - #224

Merged
GatewayJ merged 2 commits into
rustfs:mainfrom
GatewayJ:agent/fix-1096-empty-credential-secret
Aug 9, 2026
Merged

fix(security): reject empty credential secrets#224
GatewayJ merged 2 commits into
rustfs:mainfrom
GatewayJ:agent/fix-1096-empty-credential-secret

Conversation

@GatewayJ

@GatewayJ GatewayJ commented Aug 5, 2026

Copy link
Copy Markdown
Member

Type of Change

  • New Feature
  • Bug Fix
  • Documentation
  • Performance Improvement
  • Test/CI
  • Refactor
  • Other:

Related Issues

Fixes rustfs/backlog#1096

Summary of Changes

  • reject configured credential Secrets whose data field is absent or empty
  • validate required credential keys after the same trimming used by the pinned RustFS runtime
  • reject invalid UTF-8 and NUL-containing credential values while preserving stable SNAFU errors and status reasons
  • make the existing byte-length contract explicit in status messages, the CRD, user guides, and examples
  • add focused coverage for valid data, absent data, missing keys, invalid UTF-8, NUL bytes, trimmed short values, and multibyte UTF-8 values

Checklist

  • I have read and followed the CONTRIBUTING.md guidelines
  • Passed make pre-commit (fmt-check + clippy + test + e2e-check + console-lint + console-build + console-fmt-check)
  • Added/updated necessary tests
  • Documentation: updated the credential validation contract in the CRD, English and Chinese user guides, and examples
  • CHANGELOG: N/A; this enforces and clarifies the existing credential Secret contract
  • CI/CD passed

Impact

  • Breaking change (CRD/API compatibility)
  • Requires doc/config/deployment update
  • Other impact: invalid configured credential Secrets now block reconciliation through existing credential status reasons

Verification

make pre-commit

Additional Notes

Kubernetes Secret retrieval remains in Context, while credential invariants are evaluated by a private pure helper. Length is measured in UTF-8 bytes after trimming to match RustFS runtime normalization. Existing byte-length compatibility is preserved, and the public contract now states that behavior explicitly.


Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.

@GatewayJ
GatewayJ marked this pull request as ready for review August 9, 2026 12:44
@GatewayJ
GatewayJ added this pull request to the merge queue Aug 9, 2026
Merged via the queue into rustfs:main with commit 59aabf7 Aug 9, 2026
3 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a9d7501e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/context.rs
Comment on lines +325 to +326
let length = value.trim().len();
if length < MIN_CREDENTIAL_LENGTH_BYTES {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Enforce the eight-character minimum

For multibyte credentials, trim().len() counts UTF-8 bytes rather than characters, so a value such as 🔐🔐 is accepted as length 8 despite containing only two characters; the added test explicitly locks in this weaker behavior. Count characters after trimming instead, as the repository security baseline requires both credential values to contain at least eight characters.

AGENTS.md reference: AGENTS.md:L58-L62

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant