fix(review): address 6 reviewer findings from pullrequestreview-4064746778

- service/user.go: fix ChangePassword docstring (no old-password verification)
- service/user.go: RevokeSession now maps db.ErrNotFound→ErrNotFound and
  all other store errors→ErrInternal, preventing internal failures from
  masquerading as 404s
- plugin/loader.go: update scanPluginDirectory comment to reflect fail-fast
  behavior; fix Lstat comment wording
- db/queries/sqlite/events.sql: CAST COALESCE result to INTEGER so sqlc
  generates int64 instead of interface{}
- api/plugins_handler.go: log install error server-side and return sanitized
  structured JSON response instead of raw err.Error()
- .github/workflows/ci.yml: remove continue-on-error from tag build steps
  so tag boundary drift fails CI"

Agent-Logs-Url: https://github.com/J3vb/OwnCord/sessions/6635420c-af26-4cc0-9397-5e5b37887437

Co-authored-by: J3vb <192430104+J3vb@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2026-04-06 22:03:35 +00:00
committed by GitHub
co-authored by J3vb
parent 86634c531e
commit 2dc9a060fc
5 changed files with 19 additions and 16 deletions
+6 -1
View File
@@ -7,6 +7,7 @@ package api
import (
"io"
"log/slog"
"net/http"
"strconv"
@@ -76,7 +77,11 @@ func (h *PluginAdminHandler) install(w http.ResponseWriter, r *http.Request) {
}
name, err := h.registry.InstallFromZip(r.Context(), body)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
slog.Error("plugin install failed", "error", err)
writeJSON(w, http.StatusBadRequest, errorResponse{
Error: "INSTALL_FAILED",
Message: "plugin installation failed",
})
return
}
writeJSON(w, http.StatusCreated, map[string]any{"name": name})
+1 -1
View File
@@ -3,7 +3,7 @@
INSERT INTO events (seq, event_type, channel_id, payload) VALUES (?, ?, ?, ?);
-- name: GetMaxEventSeq :one
SELECT COALESCE(MAX(seq), 0) FROM events;
SELECT CAST(COALESCE(MAX(seq), 0) AS INTEGER) AS max_seq FROM events;
-- name: GetEventsSince :many
SELECT seq, event_type, channel_id, payload, created_at
+5 -5
View File
@@ -29,8 +29,8 @@ type foundPlugin struct {
}
// scanPluginDirectory walks dir non-recursively and parses plugin.json from
// every immediate subdirectory. Errors on individual plugins are wrapped and
// returned alongside the successful entries.
// every immediate subdirectory. Returns on the first error encountered;
// partial results are not returned alongside errors.
func scanPluginDirectory(dir string) ([]foundPlugin, error) {
if dir == "" {
return nil, nil
@@ -75,9 +75,9 @@ func scanPluginDirectory(dir string) ([]foundPlugin, error) {
// handler enforces that resolved paths stay rooted at pluginDir, but
// http.ServeFile / os.Open follow symlinks transparently — a malicious
// plugin .zip containing `assets/index.html -> /etc/passwd` would
// otherwise serve host files. Lstat (not Stat) is used for the
// entrypoint check below so a symlink is detected instead of
// followed, even when its target is a valid .wasm file.
// otherwise serve host files. os.Lstat is used for the entrypoint
// check below so a symlink is detected instead of followed, even
// when its target is a valid .wasm file.
if err := rejectSymlinksUnder(pluginDir); err != nil {
return nil, fmt.Errorf("plugin %q: %w", e.Name(), err)
}
+6 -2
View File
@@ -2,6 +2,7 @@ package service
import (
"context"
"errors"
"fmt"
"log/slog"
"time"
@@ -50,7 +51,7 @@ func (s *UserService) UpdateProfile(userID int64, username string, avatar *strin
return user, nil
}
// ChangePassword verifies the old password hash matches, then updates.
// ChangePassword updates the user's password and revokes other sessions.
// Returns the number of other sessions revoked.
func (s *UserService) ChangePassword(userID int64, newPasswordHash string, keepSessionID int64) (int64, error) {
if err := s.st.UpdateUserPassword(userID, newPasswordHash); err != nil {
@@ -77,7 +78,10 @@ func (s *UserService) ListSessions(userID int64) ([]db.Session, error) {
// RevokeSession deletes a specific session owned by the user.
func (s *UserService) RevokeSession(userID, sessionID int64) error {
if err := s.st.DeleteSessionByID(sessionID, userID); err != nil {
return fmt.Errorf("%w: session not found", ErrNotFound)
if errors.Is(err, db.ErrNotFound) {
return fmt.Errorf("%w: session not found", ErrNotFound)
}
return fmt.Errorf("%w: failed to revoke session", ErrInternal)
}
_ = s.st.LogAudit(userID, "session_revoke", "session", sessionID, "session revoked")
slog.Info("session revoked", "user_id", userID, "session_id", sessionID)