access-list: Hash basic-auth passwords and redact API reads #18

Merged

eva merged 2 commits from blake/sec-accesslist-hash into develop 2026-09-14 08:41:00 -05:00

Conversation 1 Commits 2 Files Changed 2

+76 -5

blake commented 2026-09-14 08:32:32 -05:00

Collaborator

Copy Link

Copy Source

What

• bcrypt-hash access-list item passwords on create/update (skip already-hashed values)
• Empty password on update keeps the existing hash for the same username
• Proxy basic-auth compares via bcrypt; legacy plaintext rows still work until re-saved
• GET list/detail/create/update responses redact password fields

Why

Admin API and DB previously exposed proxy basic-auth secrets in cleartext.

Test

• go test ./internal/proxy/
• Create access list with item password via API; GET returns empty password; basic auth still succeeds against hashed store
• Legacy plaintext item still authenticates until next save

Related: #17

## What - bcrypt-hash access-list item passwords on create/update (skip already-hashed values) - Empty password on update keeps the existing hash for the same username - Proxy basic-auth compares via bcrypt; legacy plaintext rows still work until re-saved - GET list/detail/create/update responses redact password fields ## Why Admin API and DB previously exposed proxy basic-auth secrets in cleartext. ## Test - `go test ./internal/proxy/` - Create access list with item password via API; GET returns empty password; basic auth still succeeds against hashed store - Legacy plaintext item still authenticates until next save Related: https://git.s1d3sw1ped.com/s1d3sw1ped/helix-proxy/issues/17

blake added 1 commit 2026-09-14 08:32:32 -05:00

access-list: Hash basic-auth passwords and redact API reads ... 61c7b5582c

Access-list item passwords were stored and returned in plaintext.
Hash on write with bcrypt, compare hashes in the proxy engine (with
legacy plaintext fallback), and omit passwords from admin GET JSON.

pike commented 2026-09-14 08:34:48 -05:00

Collaborator

Copy Link

Copy Source

Bouncing — CI not green.

• Format / gofmt: failing on cmd/helix-proxy/api.go and internal/proxy/engine.go (import order / spacing; run gofmt -w on both).
• CI / Go Tests (push): also failed on this head.

Security approach looks right (bcrypt + redact + legacy plaintext compare). Re-request review after gofmt + green CI.

Bouncing — CI not green. - Format / gofmt: failing on `cmd/helix-proxy/api.go` and `internal/proxy/engine.go` (import order / spacing; run `gofmt -w` on both). - CI / Go Tests (push): also failed on this head. Security approach looks right (bcrypt + redact + legacy plaintext compare). Re-request review after gofmt + green CI.

blake added 1 commit 2026-09-14 08:38:53 -05:00

format: gofmt api.go and engine.go 3087968463

eva approved these changes 2026-09-14 08:40:59 -05:00

eva left a comment

Owner

Copy Link

Copy Source

Re-review after gofmt fix: CI green (Format/Build/Go Tests). bcrypt hash+redact+legacy plaintext OK for #17. Merge.

Re-review after gofmt fix: CI green (Format/Build/Go Tests). bcrypt hash+redact+legacy plaintext OK for #17. Merge.

eva merged commit 084d661200 into develop 2026-09-14 08:41:00 -05:00

eva referenced this pull request 2026-09-14 08:41:20 -05:00

access-list: Basic-auth passwords stored and returned in plaintext #17

Sign in to join this conversation.

Reviewers

No Reviewers

eva

Labels

Clear labels

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

ash blake eva linus moss pike s1d3sw1ped

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: s1d3sw1ped/helix-proxy#18