Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughAdds opt-in found-item contact settings, public item and asset lookup, rate-limited contact submission, asynchronous owner email relay, and frontend configuration and contact pages. ChangesFound-item contact
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Sequence Diagram(s)sequenceDiagram
participant Finder
participant FoundPage
participant PublicAPI
participant GroupRepository
participant FoundService
Finder->>FoundPage: Open found item or asset URL
FoundPage->>PublicAPI: Request found-item details
PublicAPI->>GroupRepository: Resolve contact
GroupRepository-->>PublicAPI: Return FoundContact
PublicAPI-->>FoundPage: Return form or mailto response
Finder->>FoundPage: Submit message
FoundPage->>PublicAPI: POST contact request
PublicAPI->>FoundService: Queue owner email
FoundService-->>PublicAPI: Record delivery result
PublicAPI-->>FoundPage: Return 204 response
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
go
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@backend/internal/core/services/service_found.go`:
- Around line 57-68: Update buildFoundContactEmail to use html.EscapeString for
itemName, message, and replyTo before interpolating them into the HTML body, and
remove the redundant package-private escaping helper from
service_user_password_reset.go if it is only used for this purpose.
In `@frontend/pages/collection/index/settings.vue`:
- Around line 217-251: Gate the found-contact controls in the selectedCollection
settings block using the current user’s membership role for that group: only
owners may interact with the foundContactEnabled switch, foundContactMessage
field, and saveFoundContact button. For non-owners, disable or make the fields
read-only and replace or supplement the controls with a clear owner-only
message, while preserving the existing owner behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f93e453-3caf-4337-89de-d2702da27014
⛔ Files ignored due to path filters (18)
backend/app/api/static/docs/docs.gois excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.yamlis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.yamlis excluded by!backend/app/api/static/docs/**backend/internal/data/ent/group.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group/group.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group/where.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/migrate/schema.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/mutation.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/runtime.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/group.gois excluded by!backend/internal/data/ent/**docs/public/api/openapi-3.0.jsonis excluded by!docs/public/api/**docs/public/api/openapi-3.0.yamlis excluded by!docs/public/api/**docs/public/api/swagger-2.0.jsonis excluded by!docs/public/api/**docs/public/api/swagger-2.0.yamlis excluded by!docs/public/api/**
📒 Files selected for processing (22)
backend/app/api/app.gobackend/app/api/handlers/v1/controller.gobackend/app/api/handlers/v1/v1_ctrl_found.gobackend/app/api/handlers/v1/v1_ctrl_found_test.gobackend/app/api/handlers/v1/v1_ctrl_group.gobackend/app/api/middleware.gobackend/app/api/middleware_ratelimit_test.gobackend/app/api/routes.gobackend/internal/core/services/all.gobackend/internal/core/services/service_found.gobackend/internal/core/services/service_found_test.gobackend/internal/data/migrations/postgres/20260730120001_add_found_contact_fields.sqlbackend/internal/data/migrations/sqlite3/20260730120000_add_found_contact_fields.sqlbackend/internal/data/repo/repo_group.gobackend/internal/data/repo/repo_group_found_test.gobackend/internal/data/repo/repo_group_test.gofrontend/lib/api/public.tsfrontend/lib/api/types/data-contracts.tsfrontend/locales/en.jsonfrontend/middleware/auth.tsfrontend/pages/collection/index/settings.vuefrontend/pages/found/[kind]/[id].vue
Comment on lines
+57
to
+68
| body = fmt.Sprintf( | ||
| `<pre style="white-space: pre-wrap; font-family: inherit;">Someone scanned the label on your item "%s" and sent you a message through Homebox: | ||
|
|
||
| %s | ||
| </pre>`, | ||
| htmlEscape(itemName), htmlEscape(message), | ||
| ) | ||
| if replyTo != "" { | ||
| body += fmt.Sprintf(`<p>Reply to: %s</p>`, htmlEscape(replyTo)) | ||
| } | ||
| return subject, body | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate the current htmlEscape definition to confirm behavior before replacing it.
rg -n -B2 -A10 'func htmlEscape' backend/internal/core/services/Repository: sysadminsmedia/homebox
Length of output: 1235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the imports and calls to htmlEscape in service_found.go.
wc -l backend/internal/core/services/service_found.go
sed -n '1,90p' backend/internal/core/services/service_found.goRepository: sysadminsmedia/homebox
Length of output: 2742
🌐 Web query:
Go html package html.EscapeString documentation
💡 Result:
The html.EscapeString function is part of the Go standard library's html package. It is designed to escape special characters in a string to make it safe for inclusion in HTML [1]. Function Signature: func EscapeString(s string) string Behavior: EscapeString escapes only five specific characters, converting them into their corresponding HTML entities [1][2]: - < becomes < - > becomes > - & becomes & - ' becomes ' - " becomes " Key Characteristics: - Safety: By escaping these five characters, the function helps prevent basic cross-site scripting (XSS) attacks when inserting strings into HTML text content [1][2]. - Inversion: The function is the inverse of html.UnescapeString in the sense that UnescapeString(EscapeString(s)) == s always holds [1][2]. However, the converse (EscapeString(UnescapeString(s)) == s) is not always true, because UnescapeString is capable of decoding a much broader range of HTML entities than EscapeString produces [1][3]. - Performance: The implementation is optimized to return the original string immediately if it contains none of the characters that require escaping [3][4]. Example Usage: package main import ( "fmt" "html") func main { const s = "Fran & Freddie's Diner" <tasty@example.com> fmt.Println(html.EscapeString(s)) } Output: "Fran & Freddie's Diner" <tasty@example.com>
Citations:
- 1: https://pkg.go.dev/html
- 2: https://go.dev/src/html/escape.go
- 3: https://go.googlesource.com/net/+/refs/tags/v0.44.0/html/escape.go
- 4: https://github.com/golang/net/blob/master/html/escape.go
Use html.EscapeString for HTML escaping in the email body.
buildFoundContactEmail HTML-escapes user-controlled values before interpolation, and Go already provides this behavior through html.EscapeString. This keeps the email-path security behavior canonical and avoids maintaining package-private escape logic in service_user_password_reset.go.
🔒️ Suggested diff
+import "html"
+
func buildFoundContactEmail(itemName, message, replyTo string) (subject, body string) {
subject = fmt.Sprintf("Someone found your item: %s", itemName)
body = fmt.Sprintf(
`<pre style="white-space: pre-wrap; font-family: inherit;">Someone scanned the label on your item "%s" and sent you a message through Homebox:
%s
</pre>`,
- htmlEscape(itemName), htmlEscape(message),
+ html.EscapeString(itemName), html.EscapeString(message),
)
if replyTo != "" {
- body += fmt.Sprintf(`<p>Reply to: %s</p>`, htmlEscape(replyTo))
+ body += fmt.Sprintf(`<p>Reply to: %s</p>`, html.EscapeString(replyTo))
+ }
+ return subject, body
+}
+
+func htmlEscape(s string) string {
+ r := strings.NewReplacer(
+ "&", "&",
+ "<", "<",
+ ">", ">",
+ `"`, """,
+ "'", "&`#39`;",
+ )
+ return r.Replace(s)
}🤖 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_found.go` around lines 57 - 68, Update
buildFoundContactEmail to use html.EscapeString for itemName, message, and
replyTo before interpolating them into the HTML body, and remove the redundant
package-private escaping helper from service_user_password_reset.go if it is
only used for this purpose.
Comment on lines
+217
to
+251
|
|
||
| <div v-if="selectedCollection" class="mt-4 space-y-4 rounded-md border bg-card p-4"> | ||
| <div> | ||
| <h2 class="text-lg font-medium">{{ $t("found.settings.title") }}</h2> | ||
| <p class="text-sm text-muted-foreground">{{ $t("found.settings.description") }}</p> | ||
| </div> | ||
|
|
||
| <div class="flex items-center gap-2"> | ||
| <Switch id="found-contact-enabled" v-model="foundContactEnabled" /> | ||
| <Label for="found-contact-enabled">{{ $t("found.settings.enable") }}</Label> | ||
| </div> | ||
|
|
||
| <FormTextArea | ||
| v-model="foundContactMessage" | ||
| :label="$t('found.settings.message_label')" | ||
| :placeholder="$t('found.settings.message_placeholder')" | ||
| :max-length="500" | ||
| /> | ||
|
|
||
| <div class="rounded-md border border-accent-foreground bg-accent p-4 text-accent-foreground"> | ||
| <p class="text-sm">{{ $t("found.settings.no_smtp_warning") }}</p> | ||
| </div> | ||
|
|
||
| <div class="mt-4"> | ||
| <Button | ||
| variant="secondary" | ||
| size="sm" | ||
| :disabled="savingFoundContact || utf8Length(foundContactMessage) > 500" | ||
| @click="saveFoundContact" | ||
| > | ||
| <MdiLoading v-if="savingFoundContact" class="mr-2 inline-block animate-spin" /> | ||
| <span>{{ $t("global.save") }}</span> | ||
| </Button> | ||
| </div> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for existing ownership/role indicators available to the frontend.
rg -n -C3 'isOwner|ownerId|role' frontend/lib/api frontend/composables frontend/stores 2>/dev/null
rg -n -C3 'IsOwnerOf|isOwner' backend/app/api/handlers/v1 backend/internal/data/repoRepository: sysadminsmedia/homebox
Length of output: 7075
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^frontend/pages/collection/index/settings.vue$|^frontend|backend/app/api/handlers/v1/v1_ctrl_group.go$|^backend/internal/data/repo/repo_group.go$)' | sed -n '1,80p'
echo
echo "== frontend settings outline =="
ast-grep outline frontend/pages/collection/index/settings.vue --view compact 2>/dev/null | sed -n '1,120p' || true
echo
echo "== frontend settings relevant lines =="
sed -n '1,320p' frontend/pages/collection/index/settings.vue
echo
echo "== api get group usage =="
rg -n -C4 'group\.get|\.Groups\.Get\(|usergroupRole|role:|role:' frontend --glob '*.{ts,vue}' | sed -n '1,220p'
echo
echo "== group update handling =="
sed -n '70,115p' backend/app/api/handlers/v1/v1_ctrl_group.go
echo
echo "== repo group owner implementation =="
sed -n '460,488p' backend/internal/data/repo/repo_group.go
echo
echo "== data-contracts membership type =="
sed -n '660,700p' frontend/lib/api/types/data-contracts.tsRepository: sysadminsmedia/homebox
Length of output: 22226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all imports/usages of getMembers in settings or collection root =="
python3 - <<'PY'
from pathlib import Path
for p in [Path("frontend/pages/collection/index/settings.vue"), Path("frontend/pages/collection/index.vue")]:
print(p)
text = p.read_text(errors="replace")
for i, line in enumerate(text.splitlines(), 1):
if "getMembers" in line or "members" in line or "/members" in line or "selectedCollection" in line or "user_groups" in line or "owner" in line.lower():
print(f"{i}: {line}")
print()
PY
echo
echo "== group Get membership response backend =="
rg -n -C5 'func \(ctrl \*V1Controller\) HandleGroupGet|\.Group\.Get\(|GetGroup|getMembers|GetMembers|user_groups|UserGroups|UserGroup' backend/app/api/handlers/v1/backend/internal/data backend/internal/data/repo/repo_group.go backend/internal/data/repo/repo_user.go backend/internal/data/repo/repo_user_group.go 2>/dev/null || true
echo
echo "== generated group type fields =="
rg -n -C4 'export interface Group|user_groups|userGroups|IsOwner|owner' frontend/lib/api/types/data-contracts.ts backend/internal/data/repo/repo_group.go 2>/dev/null || trueRepository: sysadminsmedia/homebox
Length of output: 19109
Block found-contact edits for non-owner group members.
Found-contact field writes require role=owner via HandleGroupUpdate and return 403 for others. Gate the enable switch, message field, and save button with the current user’s per-group membership role, or show a clear owner-only message.
🤖 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 `@frontend/pages/collection/index/settings.vue` around lines 217 - 251, Gate
the found-contact controls in the selectedCollection settings block using the
current user’s membership role for that group: only owners may interact with the
foundContactEnabled switch, foundContactMessage field, and saveFoundContact
button. For non-owners, disable or make the fields read-only and replace or
supplement the controls with a clear owner-only message, while preserving the
existing owner behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the "lost item" page requested in #13: when someone scans a QR
label while signed out, they now see a public "Did you find this
item?" contact page instead of the login screen.
The feature is opt-in per collection and off by default, and it
addresses the privacy concern raised on #1487 (unconditional exposure
of the owner's email on an unauthenticated endpoint):
relayed server-side. The owner's email address is never sent to the
client.
guidance in the issue thread. The settings UI warns the owner about
this exposure before they enable the feature.
Design notes
found_contact_enabled, default false, andfound_contact_message) with goose migrations for sqlite andpostgres. Existing installs see zero behavior change.
GET /v1/found/{kind}/{id}andPOST /v1/found/{kind}/{id}/contact, both behind a per-IP requestrate limiter (30/min), with an additional per-item send cap
(3 per item / 10 min) so a single known item cannot be used to
mailbomb its owner even from distributed IPs. Missing items,
non-opted-in collections,
archived items, and ambiguous asset IDs (asset IDs are only unique
per collection) all return identical 404s, so a caller cannot probe
an instance's inventory. After input validation the contact POST
returns 204 whether or not the item resolved (forgot-password
pattern), and the email send is backgrounded so the response does
not wait on SMTP. Either endpoint may instead return 429 when the
rate limiter trips; that response is IP-scoped and identical across
items, so it does not leak item state.
asset_id 0).
interpolation into the (HTML-only) mailer body.
GroupUpdategains pointer-optional fields so existing API callersthat PUT only name/currency cannot silently reset the new settings.
(other collection settings remain member-editable). This prevents a
non-owner member from publishing the owner's email via mailto mode.
1,000 of the added lines are regenerated API specs
(swagger/OpenAPI/TS types); the hand-written diff is much smaller
than the total suggests.
Out of scope (future work per the issue thread)
Per-item "mark as lost" mode and reward fields, as floated by
@katosdev in the thread. Archiving an item already removes it from
the found page, which covers the per-item off switch. The per-item
lost mode layers cleanly on top of this change if wanted.
Notes for reviewers
scanning a location label on an opted-in group resolves the found
page ("Someone found your item: "). Left as-is since
returning a mislaid storage bin is arguably valid, but flagging it
in case you'd prefer to exclude
is_locationentities from thelookups.
rather than as a real
Reply-Toheader. Adding a trueReply-Towould mean extending the shared mailer
MessageBuilder(used bypassword-reset mail too), which is beyond this feature's footprint,
so I deliberately kept the change self-contained. Happy to add it as
a follow-up if you'd like the header.
Test plan
disabled/archived/ambiguous/no-owner cases (all fail closed), and
owner-resolution determinism.
finder-controlled fields.
validation boundaries, opaque-404 shape, and always-204 behavior.
go vet, and frontend lint pass (the only failingbackend tests are pre-existing, environment-specific attachment/blob
tests unrelated to this change).
Manually verified end to end against a copy of a real production
v0.26.2 database (goose migrations applied cleanly on startup):
signing in from there returns to the item.
configured) shows the contact form and never exposes the email.
(Gmail) with the finder's text and HTML-escaping intact.
delivered 3 emails and then silently stopped, while the page
continued to show the success state (no throttling signal to the
sender). The cap's per-item isolation (each item having an
independent budget) is covered by an automated unit test rather than
the manual pass.
Disclosure
This PR was developed with AI assistance (Claude), including repeated
AI review passes over the implementation. Every part of it was then
tested by a human on a real deployment to confirm it actually works
end to end, including live SMTP delivery. Happy to adjust anything in
review.