Skip to content
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
df2acff
feat(integrations): add Paperless-ngx and Immich as external attachme…
May 11, 2026
2ea776d
feat(integrations): Paperless & Immich attachment integration
May 12, 2026
1562632
refactor(integrations): remove Immich, keep Paperless-only adapter re…
May 12, 2026
3617164
fix(integrations): address CodeRabbit review issues on PR #1492
May 12, 2026
0a7ce56
refactor(integrations): fully remove Immich from backend
May 12, 2026
0bce93f
remove todo file
May 12, 2026
31a387e
refactor(tests): remove immich from use-preferences test fixtures
May 12, 2026
07641a5
fix(proxy): harden integration proxy against SSRF and edge cases
May 12, 2026
d9a7bed
docs(proxy): add missing docstrings to reach coverage threshold
May 12, 2026
630bbd9
fix(proxy): bind request to handler context; redact URL in logs
May 12, 2026
73257bb
refactor: implement code review findings across all PR files
May 12, 2026
e991bfc
refactor: drop externalLinkMimeTypes IIFE, range map directly in isEx…
May 12, 2026
3762dc9
refactor(edit): remove no-op loadIntegrationSettings stub
May 16, 2026
1186d27
fix(edit): use isValidHttpURL for external-link Tooltip gate
May 17, 2026
f678969
Fix CodeRabbit review findings
Jun 8, 2026
7735eeb
Stabilize stats integration fixture
Jun 8, 2026
42ffe59
Address follow-up review findings
Jun 8, 2026
89607b6
Localize template duplicate name
Jun 9, 2026
fdbae8c
Redact external link trace identifiers
Jun 9, 2026
763fbaf
Hash external link trace paths
Jun 9, 2026
a7a918e
Reuse outbound URL validation for integration proxy
Jun 30, 2026
3fe43cc
Address follow-up review and CI findings
Jun 30, 2026
2d96300
Narrow integration extension guidance
Jun 30, 2026
ccec3d8
Fix frontend typecheck after entity merge
Jun 30, 2026
51a4f99
Address latest review findings
Jun 30, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions backend/app/api/handlers/v1/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/sysadminsmedia/homebox/backend/internal/core/services/reporting/eventbus"
"github.com/sysadminsmedia/homebox/backend/internal/data/repo"
"github.com/sysadminsmedia/homebox/backend/internal/sys/config"
"github.com/sysadminsmedia/homebox/backend/internal/sys/validate"

"github.com/olahol/melody"
)
Expand Down Expand Up @@ -78,18 +79,19 @@ func WithURL(url string) func(*V1Controller) {
}

type V1Controller struct {
cookieSecure bool
repo *repo.AllRepos
svc *services.AllServices
maxUploadSize int64
maxImportSize int64
maxParseMemory int64
isDemo bool
allowRegistration bool
bus *eventbus.EventBus
url string
config *config.Config
oidcProvider *providers.OIDCProvider
cookieSecure bool
repo *repo.AllRepos
svc *services.AllServices
maxUploadSize int64
maxImportSize int64
maxParseMemory int64
isDemo bool
allowRegistration bool
bus *eventbus.EventBus
url string
config *config.Config
oidcProvider *providers.OIDCProvider
integrationProxyTransport *http.Transport
}

type (
Expand Down Expand Up @@ -129,11 +131,12 @@ type (

func NewControllerV1(svc *services.AllServices, repos *repo.AllRepos, bus *eventbus.EventBus, config *config.Config, options ...func(*V1Controller)) *V1Controller {
ctrl := &V1Controller{
repo: repos,
svc: svc,
allowRegistration: true,
bus: bus,
config: config,
repo: repos,
svc: svc,
allowRegistration: true,
bus: bus,
config: config,
integrationProxyTransport: validate.NewOutboundHTTPTransport(&config.Notifier),
}

for _, opt := range options {
Expand Down
182 changes: 182 additions & 0 deletions backend/app/api/handlers/v1/v1_ctrl_integration_proxy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
package v1

import (
"fmt"
"io"
"net/http"
"net/url"
"path"
"regexp"
"strings"
"time"

"github.com/go-chi/chi/v5"
"github.com/hay-kot/httpkit/errchain"
"github.com/rs/zerolog/log"
"github.com/sysadminsmedia/homebox/backend/internal/core/services"
"github.com/sysadminsmedia/homebox/backend/internal/sys/validate"
"go.opentelemetry.io/otel/attribute"
)

// validIntegrationName restricts integration names to safe lower-case identifiers,
// preventing settings-key injection (e.g. "../../evil").
var validIntegrationName = regexp.MustCompile(`^[a-z][a-z0-9_-]{0,31}$`)

func (ctrl *V1Controller) integrationProxyHTTPClient() *http.Client {
transport := ctrl.integrationProxyTransport
if transport == nil {
transport = validate.NewOutboundHTTPTransport(&ctrl.config.Notifier)
}

return &http.Client{
Timeout: 30 * time.Second,
Transport: transport,
CheckRedirect: func(req *http.Request, _ []*http.Request) error {
if err := validate.ValidateOutboundHTTPURL(req.URL.String(), &ctrl.config.Notifier); err != nil {
return fmt.Errorf("integration proxy redirect blocked: %w", err)
}
return nil
},
}
}

// HandleIntegrationProxy godoc
//
// @Summary Integration Reverse Proxy
// @Description Proxies a single GET request to the configured external integration.
// The integration's credentials (base URL + API token) are read from
// user settings ({name}_url / {name}_token) and never exposed to the
// frontend. This single generic endpoint replaces all per-integration
// proxy handlers: adding a new integration only requires a Vue component
// and a settings entry — no new Go code.
// @Tags Integrations
// @Produce */*
// @Param name path string true "Integration name, e.g. paperless"
// @Param path query string true "Relative API path on the upstream service, must start with /"
// @Success 200
// @Failure 400 {object} validate.ErrorResponse
// @Failure 502 {object} validate.ErrorResponse
// @Router /v1/integrations/{name}/proxy [GET]
// @Security Bearer
func (ctrl *V1Controller) HandleIntegrationProxy() errchain.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) error {
spanCtx, span := startEntityCtrlSpan(r.Context(), "controller.V1.HandleIntegrationProxy")
defer span.End()

name := chi.URLParam(r, "name")
if !validIntegrationName.MatchString(name) {
return validate.NewRequestError(fmt.Errorf("invalid integration name"), http.StatusBadRequest)
}

rawPath := r.URL.Query().Get("path")
if rawPath == "" {
return validate.NewRequestError(fmt.Errorf("path query parameter is required"), http.StatusBadRequest)
}
if !strings.HasPrefix(rawPath, "/") || strings.Contains(rawPath, "://") {
return validate.NewRequestError(fmt.Errorf("path must be a relative path starting with /"), http.StatusBadRequest)
}

// Normalise to prevent directory traversal while preserving trailing slash
// (many REST APIs treat /foo/1/ and /foo/1 differently).
cleanPath := path.Clean(rawPath)
if !strings.HasPrefix(cleanPath, "/") {
return validate.NewRequestError(fmt.Errorf("invalid path after normalisation"), http.StatusBadRequest)
}
if strings.HasSuffix(rawPath, "/") && !strings.HasSuffix(cleanPath, "/") {
cleanPath += "/"
}

span.SetAttributes(
attribute.String("integration.name", name),
attribute.String("integration.path", cleanPath),
)

ctx := services.NewContext(spanCtx)
settings, svcErr := ctrl.svc.User.GetSettings(ctx.Context, services.UseUserCtx(ctx.Context).ID)
if svcErr != nil {
return validate.NewRequestError(svcErr, http.StatusInternalServerError)
}

baseURL, _ := settings[name+"_url"].(string)
if baseURL == "" {
return validate.NewRequestError(
fmt.Errorf("%s_url not configured – add it in Settings", name),
http.StatusBadRequest,
)
}

upstream := strings.TrimRight(baseURL, "/") + cleanPath
if err := validate.ValidateOutboundHTTPURL(upstream, &ctrl.config.Notifier); err != nil {
return validate.NewRequestError(err, http.StatusBadRequest)
}

token, _ := settings[name+"_token"].(string)
if token == "" {
return validate.NewRequestError(
fmt.Errorf("%s_token not configured – add it in Settings", name),
http.StatusBadRequest,
)
}

req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, upstream, nil)
if err != nil {
return validate.NewRequestError(err, http.StatusBadRequest)
}
req.Header.Set("Authorization", "Token "+token)
Comment on lines +65 to +124

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep outline backend/app/api/handlers/v1 --match HandleUserSelfSettingsUpdate --view expanded

rg -n -C3 'HandleUserSelfSettingsUpdate|GetSettings|SetSettings|UpdateSettings|validIntegrationName|"_url"|"_token"' \
  backend/app/api/handlers/v1 backend/internal

Repository: sysadminsmedia/homebox

Length of output: 13671


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the user settings update path and any server-side validation/allowlist for settings keys.
sed -n '205,260p' backend/app/api/handlers/v1/v1_ctrl_user.go
printf '\n---\n'
sed -n '1,220p' backend/internal/core/services/service_user.go | sed -n '790,840p'
printf '\n---\n'
sed -n '340,390p' backend/internal/data/repo/repo_users.go
printf '\n---\n'
rg -n -C2 'paperless|integrat|_url"|_token"|settings.*allow|allowlist|whitelist|validation.*settings|map\[string\]interface' backend/app backend/internal

Repository: sysadminsmedia/homebox

Length of output: 40809


🏁 Script executed:

#!/bin/bash
python3 - <<'PY'
from pathlib import Path
p = Path('backend/app/api/handlers/v1/v1_ctrl_user.go')
print(p.exists())
PY

Repository: sysadminsmedia/homebox

Length of output: 165


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '205,260p' backend/app/api/handlers/v1/v1_ctrl_user.go
printf '\n---\n'
sed -n '760,815p' backend/internal/core/services/service_user.go
printf '\n---\n'
sed -n '363,380p' backend/internal/data/repo/repo_users.go
printf '\n---\n'
rg -n -C2 'validIntegrationName|name\\+\"_url\"|name\\+\"_token\"|SetSettings\\(|GetSettings\\(' backend/app/api/handlers/v1 backend/internal

Repository: sysadminsmedia/homebox

Length of output: 5797


🏁 Script executed:

#!/bin/bash
python3 - <<'PY'
from pathlib import Path
p = Path('backend/app/api/handlers/v1/v1_ctrl_user.go')
print(p.read_text().splitlines()[204:225])
PY

Repository: sysadminsmedia/homebox

Length of output: 1350


Gate name to supported integrations. HandleUserSelfSettingsUpdate persists arbitrary settings keys, so regex-only validation still allows any lower-case {name} to become an authenticated outbound proxy via {name}_url / {name}_token.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/api/handlers/v1/v1_ctrl_integration_proxy.go` around lines 65 -
124, The proxy handler in HandleUserSelfSettingsUpdate only validates the
URL-safe format of name, which still allows arbitrary integration keys to be
used for authenticated outbound requests. Add an explicit allowlist check for
supported integration names before reading {name}_url and {name}_token, and
return a bad request error for anything unsupported. Keep the existing name
validation, but gate the proxy path earlier in the handler that uses
validIntegrationName, settings, and validate.ValidateOutboundHTTPURLWithContext.


resp, err := ctrl.integrationProxyHTTPClient().Do(req)
if err != nil {
// Log only host+path to avoid leaking query strings or embedded credentials.
var safeURL string
if u, parseErr := url.Parse(upstream); parseErr == nil {
safeURL = u.Host + u.Path
}
log.Err(err).Str("integration", name).Str("upstream", safeURL).Msg("integration proxy: upstream request failed")
return validate.NewRequestError(err, http.StatusBadGateway)
}
defer func() { _ = resp.Body.Close() }()

if resp.StatusCode == http.StatusNotFound {
return validate.NewRequestError(fmt.Errorf("resource not found at upstream"), http.StatusNotFound)
}
if resp.StatusCode >= 400 {
return validate.NewRequestError(
fmt.Errorf("upstream returned %d", resp.StatusCode),
http.StatusBadGateway,
)
}

const maxResponseSize int64 = 10 * 1024 * 1024 // 10 MB

// Reject known-oversized responses before writing any bytes to the client.
if resp.ContentLength > maxResponseSize {
return validate.NewRequestError(
fmt.Errorf("upstream response too large (%d bytes)", resp.ContentLength),
http.StatusBadGateway,
)
}

// Buffer up to maxResponseSize+1 bytes so we can detect true truncation
// and return a clean 502 rather than a partial 200 with invalid JSON.
buf, readErr := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize+1))
if readErr != nil {
log.Err(readErr).Str("integration", name).Msg("integration proxy: failed to read response")
return validate.NewRequestError(fmt.Errorf("failed to read upstream response"), http.StatusBadGateway)
}
if int64(len(buf)) > maxResponseSize {
log.Warn().Str("integration", name).Msg("integration proxy: upstream response exceeded 10 MB limit")
return validate.NewRequestError(
fmt.Errorf("upstream response exceeds 10 MB limit"),
http.StatusBadGateway,
)
}

if ct := resp.Header.Get("Content-Type"); ct != "" {
w.Header().Set("Content-Type", ct)
}
if _, writeErr := w.Write(buf); writeErr != nil {
log.Err(writeErr).Str("integration", name).Msg("integration proxy: failed to write response")
}
return nil
}
}
29 changes: 29 additions & 0 deletions backend/app/api/handlers/v1/v1_ctrl_integration_proxy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package v1

import (
"net/http"
"testing"

"github.com/sysadminsmedia/homebox/backend/internal/sys/config"
)

func TestIntegrationProxyHTTPClientValidatesRedirectTargets(t *testing.T) {
ctrl := &V1Controller{
config: &config.Config{
Notifier: config.NotifierConf{
BlockCloudMetadata: true,
Dns64Nets: []string{"64:ff9b::/96", "64:ff9b:1::/48"},
},
},
}

client := ctrl.integrationProxyHTTPClient()
req, err := http.NewRequest(http.MethodGet, "http://169.254.169.254/latest/meta-data", nil)
if err != nil {
t.Fatalf("failed to build redirect request: %v", err)
}

if err := client.CheckRedirect(req, nil); err == nil {
t.Fatal("expected redirect to cloud metadata endpoint to be blocked")
}
}
3 changes: 3 additions & 0 deletions backend/app/api/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ func (a *app) mountRoutes(r *chi.Mux, chain *errchain.ErrChain, repos *repo.AllR
r.Delete("/notifiers/{id}", chain.ToHandlerFunc(v1Ctrl.HandleDeleteNotifier(), userMW...))
r.Post("/notifiers/test", chain.ToHandlerFunc(v1Ctrl.HandlerNotifierTest(), append(userMW, a.notifierTestLimiter.middleware)...))

// Integration proxy endpoints
r.Get("/integrations/{name}/proxy", chain.ToHandlerFunc(v1Ctrl.HandleIntegrationProxy(), userMW...))

// Asset-Like endpoints
assetMW := []errchain.Middleware{
a.mwAuthToken,
Expand Down
2 changes: 2 additions & 0 deletions backend/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1x
github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw=
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5 h1:6xNmx7iTtyBRev0+D/Tv1FZd4SCg8axKApyNyRsAt/w=
github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5/go.mod h1:KdCmV+x/BuvyMxRnYBlmVaq4OLiKW6iRQfvC62cvdkI=
github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2 h1:aBangftG7EVZoUb69Os8IaYg++6uMOdKK83QtkkvJik=
github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2/go.mod h1:qwXFYgsP6T7XnJtbKlf1HP8AjxZZyzxMmc+Lq5GjlU4=
github.com/coder/websocket v1.8.14 h1:9L0p0iKiNOibykf283eHkKUHHrpG7f65OE3BhhO7v9g=
Expand Down
19 changes: 13 additions & 6 deletions backend/internal/core/services/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,24 @@ func MainNoExit(m *testing.M) int {
log.Fatalf("failed opening connection to sqlite: %v", err)
}

go func() {
_ = tbus.Run(context.Background())
}()

err = client.Schema.Create(context.Background())
if err != nil {
log.Fatalf("failed creating schema resources: %v", err)
}

busCtx, cancelBus := context.WithCancel(context.Background())
busErr := make(chan error, 1)
go func() {
busErr <- tbus.Run(busCtx)
}()
defer func() {
cancelBus()
if err := <-busErr; err != nil {
log.Printf("event bus error: %v", err)
}
_ = client.Close()
}()

tClient = client
tRepos = repo.New(tClient, tbus, config.Storage{
PrefixPath: "/",
Expand All @@ -93,8 +102,6 @@ func MainNoExit(m *testing.M) int {
ConnString: "file://" + os.TempDir(),
}, "mem://{{ .Topic }}", "sqlite3"),
)
defer func() { _ = client.Close() }()

bootstrap()
tCtx = Context{
Context: context.Background(),
Expand Down
9 changes: 1 addition & 8 deletions backend/internal/core/services/service_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,11 @@ func (svc *GroupService) CreateGroup(ctx Context, name string) (repo.Group, erro
return repo.Group{}, errors.New("user ID cannot be empty when creating a group")
}

group, err := svc.repos.Groups.GroupCreate(ctx.Context, name, ctx.UID)
group, err := svc.repos.Groups.GroupCreateWithEntityTypeDefaults(ctx.Context, name, ctx.UID)
if err != nil {
return repo.Group{}, err
}

// Unlike registration, this path doesn't seed default locations/tags, so
// nothing would lazily create the entity types — leaving the collection
// unable to create items or locations until a type was added by hand.
if err := ensureDefaultEntityTypes(ctx.Context, svc.repos, group.ID); err != nil {
return repo.Group{}, err
}

return group, nil
}

Expand Down
23 changes: 22 additions & 1 deletion backend/internal/core/services/service_items_attachments.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ package services

import (
"context"
"crypto/sha256"
"encoding/hex"
"fmt"
"io"
"net/url"
"strings"

"github.com/google/uuid"
"github.com/rs/zerolog/log"
Expand All @@ -14,6 +18,23 @@ import (
"go.opentelemetry.io/otel/trace"
)

func redactExternalIdentifierForTrace(sourceType, externalID string) string {
if sourceType != "link" {
return externalID
}

u, err := url.Parse(strings.TrimSpace(externalID))
if err != nil {
return ""
}
u.User = nil
u.RawQuery = ""
u.Fragment = ""

pathHash := sha256.Sum256([]byte(u.EscapedPath()))
return fmt.Sprintf("%s://%s/path:%s", u.Scheme, u.Host, hex.EncodeToString(pathHash[:8]))
}
Comment on lines +21 to +36

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Redact the hostname too before tracing integration.external_id.

For "link" sources this still writes the raw host/port into OTel spans. Those URLs are user-supplied, so traces can still leak private Paperless or intranet hostnames even after path/query stripping.

🔐 Proposed fix
 func redactExternalIdentifierForTrace(sourceType, externalID string) string {
 	if sourceType != "link" {
 		return externalID
 	}
 
 	u, err := url.Parse(strings.TrimSpace(externalID))
 	if err != nil {
 		return ""
 	}
 	u.User = nil
 	u.RawQuery = ""
 	u.Fragment = ""
 
+	hostHash := sha256.Sum256([]byte(strings.ToLower(u.Host)))
 	pathHash := sha256.Sum256([]byte(u.EscapedPath()))
-	return fmt.Sprintf("%s://%s/path:%s", u.Scheme, u.Host, hex.EncodeToString(pathHash[:8]))
+	return fmt.Sprintf(
+		"%s://host:%s/path:%s",
+		u.Scheme,
+		hex.EncodeToString(hostHash[:8]),
+		hex.EncodeToString(pathHash[:8]),
+	)
 }

Also applies to: 144-144

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/internal/core/services/service_items_attachments.go` around lines 21
- 36, The trace redaction in redactExternalIdentifierForTrace still exposes the
raw host/port for "link" sources, so update it to redact or hash the hostname as
well before returning the value used for integration.external_id spans. Keep the
existing path hashing logic, but replace any direct use of u.Host in the
formatted trace identifier with a sanitized host representation so no
user-supplied hostname leaks through. Also make sure the call site that records
integration.external_id uses the updated redaction output.


func (svc *EntityService) AttachmentPath(ctx context.Context, gid uuid.UUID, attachmentID uuid.UUID) (*ent.Attachment, error) {
ctx, span := entityServiceTracer().Start(ctx, "service.EntityService.AttachmentPath",
trace.WithAttributes(
Expand Down Expand Up @@ -120,7 +141,7 @@ func (svc *EntityService) AttachmentAddExternalLink(ctx Context, entityID uuid.U
attribute.String("group.id", ctx.GID.String()),
attribute.String("entity.id", entityID.String()),
attribute.String("integration.source_type", sourceType),
attribute.String("integration.external_id", externalID),
attribute.String("integration.external_id", redactExternalIdentifierForTrace(sourceType, externalID)),
))
defer span.End()
ctx.Context = spanCtx
Expand Down
Loading
Loading