Close remaining low-priority review follow-ups by removing hardcoded help URLs, improving help drawer accessibility, raising explicit bcrypt cost, and adding validation/injection coverage; also bound postqueue reads with a timeout to avoid hangs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
Cursor
parent
81190a7aad
commit
4204ee279c
@@ -46,6 +46,9 @@ rate limit is correct for a non-default level-1 window. Schema migration
|
|||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
|
- postqueue reader is now time-bounded: `postqueue -p` runs under a 5-second
|
||||||
|
context timeout so a stuck queue command cannot hang the queue panel path
|
||||||
|
indefinitely.
|
||||||
- DMARC panel layout classes now exist in
|
- DMARC panel layout classes now exist in
|
||||||
`internal/web/view/static/panel.css` (`.pair`, `.attn`, `.desk-only`,
|
`internal/web/view/static/panel.css` (`.pair`, `.attn`, `.desk-only`,
|
||||||
`.phone-only`, `.phone-list`, `.check-row`), so desktop/mobile DMARC views and
|
`.phone-only`, `.phone-list`, `.check-row`), so desktop/mobile DMARC views and
|
||||||
@@ -57,6 +60,14 @@ rate limit is correct for a non-default level-1 window. Schema migration
|
|||||||
session sign-out when the password changed.
|
session sign-out when the password changed.
|
||||||
- Login/setup rate-limiter bucket eviction now prefers the nearest-expiry bucket
|
- Login/setup rate-limiter bucket eviction now prefers the nearest-expiry bucket
|
||||||
and does not evict currently blocked buckets.
|
and does not evict currently blocked buckets.
|
||||||
|
- Help page/drawer links now use `SourceURL` (`{{.SourceURL}}/blob/main/docs/...`)
|
||||||
|
instead of a hardcoded repository URL.
|
||||||
|
- Help drawer close controls include accessibility attributes (`role="button"`,
|
||||||
|
`aria-label`) for the scrim and close action.
|
||||||
|
- Password hashing now uses explicit bcrypt cost `12` in settings/user password
|
||||||
|
updates, instead of `bcrypt.DefaultCost`.
|
||||||
|
- Test coverage expanded for DMARC map token injection rejection and email
|
||||||
|
validation edge cases (ASCII/local-part/domain/freemail cases).
|
||||||
|
|
||||||
- **Port 25 no longer runs the submission milter chain.** `smtpd_milters` is
|
- **Port 25 no longer runs the submission milter chain.** `smtpd_milters` is
|
||||||
now always overridden on `smtp/inet` — to the optional antispam milter, or
|
now always overridden on `smtp/inet` — to the optional antispam milter, or
|
||||||
|
|||||||
@@ -33,3 +33,21 @@ func TestRenderDMARCMapsEmpty(t *testing.T) {
|
|||||||
t.Fatalf("expected empty maps")
|
t.Fatalf("expected empty maps")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRenderDMARCMapsRejectsInjectionTokens(t *testing.T) {
|
||||||
|
_, _, _, err := renderDMARCMaps(DMARCMapsConfig{
|
||||||
|
Recipients: []string{"dmarc@example.com\nattacker@example.com"},
|
||||||
|
Domains: []string{"example.com"},
|
||||||
|
})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected recipient token validation error")
|
||||||
|
}
|
||||||
|
|
||||||
|
_, _, _, err = renderDMARCMaps(DMARCMapsConfig{
|
||||||
|
Recipients: []string{"dmarc@example.com"},
|
||||||
|
Domains: []string{"example.com attacker"},
|
||||||
|
})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected domain token validation error")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,12 +1,16 @@
|
|||||||
package postfix
|
package postfix
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const postqueueTimeout = 5 * time.Second
|
||||||
|
|
||||||
// Queue returns Postfix's own human-readable mail-queue listing
|
// Queue returns Postfix's own human-readable mail-queue listing
|
||||||
// (architecture.md § Panel HTTP surface): active, deferred and held messages,
|
// (architecture.md § Panel HTTP surface): active, deferred and held messages,
|
||||||
// exactly as an administrator would see via the CLI. The command takes a
|
// exactly as an administrator would see via the CLI. The command takes a
|
||||||
@@ -14,8 +18,13 @@ import (
|
|||||||
// (security.md). The panel is responsible for escaping the output before
|
// (security.md). The panel is responsible for escaping the output before
|
||||||
// display (security.md); this function returns it as-is.
|
// display (security.md); this function returns it as-is.
|
||||||
func Queue() (string, error) {
|
func Queue() (string, error) {
|
||||||
cmd := exec.Command("postqueue", "-p")
|
ctx, cancel := context.WithTimeout(context.Background(), postqueueTimeout)
|
||||||
|
defer cancel()
|
||||||
|
cmd := exec.CommandContext(ctx, "postqueue", "-p")
|
||||||
out, err := cmd.CombinedOutput()
|
out, err := cmd.CombinedOutput()
|
||||||
|
if ctx.Err() == context.DeadlineExceeded {
|
||||||
|
return "", fmt.Errorf("postqueue -p timed out after %s", postqueueTimeout)
|
||||||
|
}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", fmt.Errorf("postqueue -p: %w: %s", err, strings.TrimSpace(string(out)))
|
return "", fmt.Errorf("postqueue -p: %w: %s", err, strings.TrimSpace(string(out)))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -191,7 +191,7 @@ func (h *Handlers) submitSettings(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
hash := user.PasswordHash
|
hash := user.PasswordHash
|
||||||
if repassword {
|
if repassword {
|
||||||
newHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
|
newHash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logf("panel: settings: hashing password failed: %v", err)
|
logf("panel: settings: hashing password failed: %v", err)
|
||||||
h.renderSettings(w, r, http.StatusInternalServerError,
|
h.renderSettings(w, r, http.StatusInternalServerError,
|
||||||
|
|||||||
@@ -171,7 +171,7 @@ func (h *Handlers) submitUserCreate(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
|
hash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logf("panel: create user hash: %v", err)
|
logf("panel: create user hash: %v", err)
|
||||||
h.renderUserForm(w, r, http.StatusInternalServerError, 0, userFormView{FormErr: "Internal error. Please try again."})
|
h.renderUserForm(w, r, http.StatusInternalServerError, 0, userFormView{FormErr: "Internal error. Please try again."})
|
||||||
@@ -240,7 +240,7 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st
|
|||||||
h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: err.Error(), FormUsername: username, FormRole: string(role), FormDomains: selected})
|
h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: err.Error(), FormUsername: username, FormRole: string(role), FormDomains: selected})
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
newHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
|
newHash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logf("panel: update user hash: %v", err)
|
logf("panel: update user hash: %v", err)
|
||||||
h.renderUserForm(w, r, http.StatusInternalServerError, u.ID, userFormView{FormErr: "Internal error. Please try again.", FormUsername: username, FormRole: string(role), FormDomains: selected})
|
h.renderUserForm(w, r, http.StatusInternalServerError, u.ID, userFormView{FormErr: "Internal error. Please try again.", FormUsername: username, FormRole: string(role), FormDomains: selected})
|
||||||
|
|||||||
@@ -0,0 +1,3 @@
|
|||||||
|
package handlers
|
||||||
|
|
||||||
|
const bcryptCost = 12
|
||||||
@@ -15,4 +15,19 @@ func TestValidateEmail(t *testing.T) {
|
|||||||
if err := Email("x@gmail.com"); err == nil {
|
if err := Email("x@gmail.com"); err == nil {
|
||||||
t.Error("gmail accepted")
|
t.Error("gmail accepted")
|
||||||
}
|
}
|
||||||
|
if err := Email("reports+rua@mail.example.com"); err != nil {
|
||||||
|
t.Errorf("plus-address valid: %v", err)
|
||||||
|
}
|
||||||
|
if err := Email("bad space@mail.example.com"); err == nil {
|
||||||
|
t.Error("address with space accepted")
|
||||||
|
}
|
||||||
|
if err := Email("привет@mail.example.com"); err == nil {
|
||||||
|
t.Error("non-ascii local-part accepted")
|
||||||
|
}
|
||||||
|
if err := Email("x@-example.com"); err == nil {
|
||||||
|
t.Error("invalid domain accepted")
|
||||||
|
}
|
||||||
|
if err := Email("x@protonmail.com"); err == nil {
|
||||||
|
t.Error("blocked freemail domain accepted")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -32,7 +32,7 @@ on the card itself.</p>
|
|||||||
</ul>
|
</ul>
|
||||||
<p class="muted">Topics marked as labels open the same text in the help
|
<p class="muted">Topics marked as labels open the same text in the help
|
||||||
drawer. For installation, backup, and environment variables see the
|
drawer. For installation, backup, and environment variables see the
|
||||||
<a href="https://github.com/mixeme/selfpost/blob/main/docs/guide.md">operator
|
<a href="{{.SourceURL}}/blob/main/docs/guide.md">operator
|
||||||
guide</a> in the repository.</p>
|
guide</a> in the repository.</p>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|||||||
@@ -21,14 +21,14 @@
|
|||||||
{{end}}
|
{{end}}
|
||||||
|
|
||||||
{{define "help_drawer"}}
|
{{define "help_drawer"}}
|
||||||
<label class="help-scrim" for="help-off"></label>
|
<label class="help-scrim" for="help-off" role="button" aria-label="Close help"></label>
|
||||||
<aside class="help-drawer">
|
<aside class="help-drawer">
|
||||||
<label class="help-close" for="help-off">Close</label>
|
<label class="help-close" for="help-off" role="button" aria-label="Close help">Close</label>
|
||||||
|
|
||||||
<article class="help-pane help-pane-index">
|
<article class="help-pane help-pane-index">
|
||||||
<h2>Help</h2>
|
<h2>Help</h2>
|
||||||
<p>Short notes for the card you opened — not a second copy of the
|
<p>Short notes for the card you opened — not a second copy of the
|
||||||
<a href="https://github.com/mixeme/selfpost/blob/main/docs/guide.md">operator guide</a>.</p>
|
<a href="{{.SourceURL}}/blob/main/docs/guide.md">operator guide</a>.</p>
|
||||||
{{if .IsGlobal}}
|
{{if .IsGlobal}}
|
||||||
<p class="muted">Status</p>
|
<p class="muted">Status</p>
|
||||||
<ul class="toc"><li><label for="help-status">Status checks</label></li></ul>
|
<ul class="toc"><li><label for="help-status">Status checks</label></li></ul>
|
||||||
|
|||||||
Reference in New Issue
Block a user