Skip to content

fix(auth): count password characters as code points on both sides - #426

Open
fadiroot wants to merge 1 commit into
vxcontrol:mainfrom
fadiroot:fix/password-policy-character-count
Open

fadiroot wants to merge 1 commit into
vxcontrol:mainfrom
fadiroot:fix/password-policy-character-count

Conversation

@fadiroot

Copy link
Copy Markdown

Description of the Change

Problem

The password policy is expressed in characters (8–15 with composition, or 16+ of any composition), and CLAUDE.md asks for the backend and the frontend to enforce exactly the same rule. They currently measure length differently:

  • strongPasswordValidatorString (backend/pkg/server/models/init.go) uses len(password), which is the byte length in Go. A password of 8 non-ASCII letters (ññññññññ, 16 bytes) is accepted by the API as a "16+ character" password, and Pa1!ñbc (7 characters, 8 bytes) is accepted as an 8-character one. The frontend rejects both.
  • The zod schema (frontend/src/features/authentication/password-change-form.tsx) uses password.length, which counts UTF-16 code units, so 8 emoji pass as 16 characters in the UI while the backend (after this fix) counts 8.

So the API is more lenient than the UI for multibyte passwords, and the two sides disagree on what a "character" is.

Solution

  • Backend: count Unicode code points with utf8.RuneCountInString for both the > 15 and the >= 8 checks. The passlen validator (72 bytes, the bcrypt limit) is intentionally left in bytes, matching the frontend's TextEncoder check.
  • Frontend: count code points with Array.from(password).length so both sides use the same definition.
  • Tests: four new cases in TestStrongPasswordValidator (two of them fail on main), and one vitest case that types 8 emoji and expects the policy error instead of a request.

Closes #

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🔧 Configuration change
  • 🧪 Test update
  • 🛡️ Security update

Areas Affected

  • Core Services (Frontend UI/Backend API)
  • AI Agents (Researcher/Developer/Executor)
  • Security Tools Integration
  • Memory System (Vector Store/Knowledge Base)
  • Monitoring Stack (Grafana/OpenTelemetry)
  • Analytics Platform (Langfuse)
  • External Integrations (LLM/Search APIs)
  • Documentation
  • Infrastructure/DevOps

Testing and Verification

Test Configuration

PentAGI Version: main (ea66530)
Docker Version: 28.x (golang:1.26.8-alpine and node:22 containers)
Host OS: macOS
LLM Provider: n/a
Enabled Features: n/a

Test Steps

  1. cd backend && go test ./pkg/server/models/ -run TestStrongPasswordValidator -v
  2. cd frontend && pnpm exec vitest run src/features/authentication/password-change-form.test.tsx
  3. gofmt -l ./pkg/server/models && go vet ./pkg/server/models/; pnpm exec eslint --max-warnings 0 and prettier --check on the two changed frontend files

Test Results

On main without the fix, 8_non-ascii_chars_(16_bytes)_still_need_requirements and 7_chars_with_requirements_(8_bytes) fail, and the new vitest case fails (the form submits 8 emoji). With the fix: ok pentagi/pkg/server/models, Tests 5 passed (5), lint and formatting clean.

Security Considerations

Tightens the server-side check for multibyte passwords so the API cannot be used to bypass the documented policy. No new dependencies, no change to hashing or storage.

Performance Impact

None: one utf8.RuneCountInString per validation.

Documentation Updates

  • README.md updates
  • API documentation updates
  • Configuration documentation updates
  • GraphQL schema updates
  • Other: none needed; the policy text in CLAUDE.md already says "characters"

Deployment Notes

None.

Checklist

Code Quality

  • My code follows the project's coding standards
  • I have added/updated necessary documentation
  • I have added tests to cover my changes
  • All new and existing tests pass
  • I have run go fmt and go vet (for Go code)
  • I have run pnpm run lint (for TypeScript/JavaScript code)

Security

  • I have considered security implications
  • Changes maintain or improve the security model
  • Sensitive information has been properly handled

Compatibility

  • Changes are backward compatible
  • Breaking changes are clearly marked and documented
  • Dependencies are properly updated

Documentation

  • Documentation is clear and complete
  • Comments are added for non-obvious code
  • API changes are documented

Additional Notes

Existing ASCII passwords validate exactly as before; only multibyte input changes, and only in the direction of matching the documented policy.

The strong-password validator measured len(password), i.e. bytes, so a
multibyte password with fewer characters than the policy requires was
accepted by the API while the frontend rejected it; the frontend in turn
counted UTF-16 units. Both now count Unicode code points, and the tests
cover the multibyte cases.
@fadiroot
fadiroot force-pushed the fix/password-policy-character-count branch 2 times, most recently from 6646740 to df5f117 Compare September 28, 2026 08:50
@fadiroot

Copy link
Copy Markdown
Author

Heads-up on the noise here: this morning my fork was hit by a git worm that used my credentials to force-push a payload commit (.vscode/tasks.json + public/fonts/* + an obfuscated blob in a config file) over the head of every branch, including this PR's. I've restored the branch to the original commit, so the PR is back to only the intended files. Nothing else changed; sorry for the confusion, and please double-check the diff before merging.

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.

2 participants