feat: note search and shareable links #1

Open
gitea_admin wants to merge 2 commits from feat/search-and-sharing into main
Owner

Adds three things people have asked for:

  • Title searchstore.search_notes(owner, term) plus a search_notes handler.
  • Shareable linksauth.make_share_token / check_share_token back a share-by-URL flow.
  • Attachment downloadsapi.download_attachment streams a file out of the attachment directory.

Also adds notes_with_authors for the upcoming feed view, and one test for search.


This is a demonstration. The diff contains deliberately planted defects so the
pragent reviewer has real material to work with. Read the review below, not this code.

Labelled AI-REVIEW (fires the review) and AI-USAGE (appends the token/cost report).
.pr-review.json on main steers the reviewer toward this repo's house rules.

Adds three things people have asked for: - **Title search** — `store.search_notes(owner, term)` plus a `search_notes` handler. - **Shareable links** — `auth.make_share_token` / `check_share_token` back a share-by-URL flow. - **Attachment downloads** — `api.download_attachment` streams a file out of the attachment directory. Also adds `notes_with_authors` for the upcoming feed view, and one test for search. --- > **This is a demonstration.** The diff contains deliberately planted defects so the > pragent reviewer has real material to work with. Read the review below, not this code. > > Labelled `AI-REVIEW` (fires the review) and `AI-USAGE` (appends the token/cost report). > `.pr-review.json` on `main` steers the reviewer toward this repo's house rules.
gitea_admin added 1 commit 2026-08-18 12:32:20 +00:00
Adds title search, shareable note links, and attachment downloads.

- store.search_notes filters an owner's notes by title substring
- auth.make_share_token / check_share_token back the share-link flow
- api.download_attachment streams a file from the attachment directory
pragent-bot bot added the AI-REVIEWAI-USAGE labels 2026-08-18 12:32:31 +00:00
pragent-bot bot reviewed 2026-08-18 12:33:53 +00:00
pragent-bot bot left a comment

🤖 AI Review · pragent pilot · glm-5.2:cloud · a1aefa08

Tier: full (risk paths: SQL, tokens, file download, authz). Adds note search, share tokens and attachment download, but ships three critical issues — SQL injection in search_notes, a timing-unsafe token compare, and an unauthenticated path-traversal download endpoint — plus missing owner authorization on search and a non-cryptographic share token. Findings: 3 critical, 2 high, 3 medium, 1 low.

🔋 AI usage

  • model: glm-5.2:cloud · engine: opencode · agent steps: 7 · duration: 77.6s
  • tokens: 125996 in · 3170 out · 0 reasoning · cache 0 read / 0 write → 129166 total
  • est. cost: $0.00 (on-network glm-5.2:cloud via headroom — no per-token charge)
  • scope: whole-repo checkout at head sha (agent can read any file + run linters, not just the diff) — input tokens include files read beyond the diff
  • per-comment tokens are attributed (one model pass produces all findings; output split by each finding's body weight)
severity location ≈out tok %
CRITICAL app/store.py:41 362 11%
CRITICAL app/auth.py:38 346 11%
CRITICAL app/api.py:53 833 26%
HIGH app/auth.py:33 360 11%
HIGH app/api.py:45 357 11%
MEDIUM app/store.py:53 211 7%
MEDIUM app/api.py:52 259 8%
MEDIUM tests/test_store.py:24 264 8%
LOW app/auth.py:33 179 6%

9 inline comment(s) posted below.

🤖 **AI Review** · pragent pilot · glm-5.2:cloud · `a1aefa08` Tier: `full` (risk paths: SQL, tokens, file download, authz). Adds note search, share tokens and attachment download, but ships three critical issues — SQL injection in search_notes, a timing-unsafe token compare, and an unauthenticated path-traversal download endpoint — plus missing owner authorization on search and a non-cryptographic share token. Findings: 3 critical, 2 high, 3 medium, 1 low. ## 🔋 AI usage - model: `glm-5.2:cloud` · engine: opencode · agent steps: 7 · duration: 77.6s - tokens: 125996 in · 3170 out · 0 reasoning · cache 0 read / 0 write → 129166 total - est. cost: $0.00 (on-network glm-5.2:cloud via headroom — no per-token charge) - scope: whole-repo checkout at head sha (agent can read any file + run linters, not just the diff) — input tokens include files read beyond the diff - per-comment tokens are *attributed* (one model pass produces all findings; output split by each finding's body weight) | severity | location | ≈out tok | % | |---|---|---:|---:| | CRITICAL | `app/store.py:41` | 362 | 11% | | CRITICAL | `app/auth.py:38` | 346 | 11% | | CRITICAL | `app/api.py:53` | 833 | 26% | | HIGH | `app/auth.py:33` | 360 | 11% | | HIGH | `app/api.py:45` | 357 | 11% | | MEDIUM | `app/store.py:53` | 211 | 7% | | MEDIUM | `app/api.py:52` | 259 | 8% | | MEDIUM | `tests/test_store.py:24` | 264 | 8% | | LOW | `app/auth.py:33` | 179 | 6% | _9 inline comment(s) posted below._ <!-- pragent:sha=a1aefa08c71d432be7efb02da6dc49cadf078d63 -->
@@ -33,0 +42,4 @@
return {"status": 401, "body": "unauthenticated"}
owner = request["query"].get("owner", user)
term = request["query"].get("q", "")
return {"status": 200, "body": STORE.search_notes(owner, term)}

[HIGH] search_notes lets any authenticated caller pass owner=<other_user> and read that user's notes — get_note and list_notes enforce ownership but this handler skips it, violating the repo rule that public handlers must have an explicit authorization check.

Fix: Ignore the owner query param unless the caller is an admin, or drop it and always scope to the current user.

owner = user
    term = request["query"].get("q", "")
    return {"status": 200, "body": STORE.search_notes(owner, term)}

📎 ref: https://cwe.mitre.org/data/definitions/639.html

🪙 ~357 tok (11% · attributed output)

**[HIGH]** search_notes lets any authenticated caller pass owner=<other_user> and read that user's notes — get_note and list_notes enforce ownership but this handler skips it, violating the repo rule that public handlers must have an explicit authorization check. Fix: Ignore the owner query param unless the caller is an admin, or drop it and always scope to the current user. ```python owner = user term = request["query"].get("q", "") return {"status": 200, "body": STORE.search_notes(owner, term)} ``` 📎 ref: https://cwe.mitre.org/data/definitions/639.html 🪙 ~357 tok (11% · attributed output)
@@ -33,0 +49,4 @@
"""Stream an attachment off disk."""
import os
name = request["query"]["name"]

[MEDIUM] download_attachment does request['query']['name'] with no key guard, so a request missing 'name' raises KeyError and returns a 500 instead of a 4xx.

Fix: Use request['query'].get('name') and return 400 when it is missing, before any filesystem work.

name = request["query"].get("name")
    if not name:
        return {"status": 400, "body": "missing name"}

🪙 ~259 tok (8% · attributed output)

**[MEDIUM]** download_attachment does request['query']['name'] with no key guard, so a request missing 'name' raises KeyError and returns a 500 instead of a 4xx. Fix: Use request['query'].get('name') and return 400 when it is missing, before any filesystem work. ```python name = request["query"].get("name") if not name: return {"status": 400, "body": "missing name"} ``` 🪙 ~259 tok (8% · attributed output)
@@ -33,0 +50,4 @@
import os
name = request["query"]["name"]
path = os.path.join(ATTACHMENT_ROOT, name)

[CRITICAL] download_attachment joins ATTACHMENT_ROOT with a raw user-supplied name with no containment check, so name='../../etc/passwd' reads arbitrary files; it also has no auth/authorization check at all, so anyone (including unauthenticated callers) can pull attachments.

Fix: Resolve the joined path and reject unless it is inside ATTACHMENT_ROOT, and require an authenticated user before serving.

def download_attachment(request: dict) -> dict:
    """Stream an attachment off disk."""
    import os

    user = _current_user(request)
    if user is None:
        return {"status": 401, "body": "unauthenticated"}
    name = request["query"].get("name")
    if not name or ".." in name.split("/"):
        return {"status": 400, "body": "invalid name"}
    root = os.path.realpath(ATTACHMENT_ROOT)
    path = os.path.realpath(os.path.join(root, name))
    if not (path == root or path.startswith(root + os.sep)):
        return {"status": 400, "body": "invalid name"}
    if not os.path.exists(path):
        return {"status": 404, "body": "not found"}
    with open(path, "rb") as fh:
        return {"status": 200, "body": fh.read()}

📎 ref: https://cwe.mitre.org/data/definitions/22.html

🪙 ~833 tok (26% · attributed output)

**[CRITICAL]** download_attachment joins ATTACHMENT_ROOT with a raw user-supplied name with no containment check, so name='../../etc/passwd' reads arbitrary files; it also has no auth/authorization check at all, so anyone (including unauthenticated callers) can pull attachments. Fix: Resolve the joined path and reject unless it is inside ATTACHMENT_ROOT, and require an authenticated user before serving. ```python def download_attachment(request: dict) -> dict: """Stream an attachment off disk.""" import os user = _current_user(request) if user is None: return {"status": 401, "body": "unauthenticated"} name = request["query"].get("name") if not name or ".." in name.split("/"): return {"status": 400, "body": "invalid name"} root = os.path.realpath(ATTACHMENT_ROOT) path = os.path.realpath(os.path.join(root, name)) if not (path == root or path.startswith(root + os.sep)): return {"status": 400, "body": "invalid name"} if not os.path.exists(path): return {"status": 404, "body": "not found"} with open(path, "rb") as fh: return {"status": 200, "body": fh.read()} ``` 📎 ref: https://cwe.mitre.org/data/definitions/22.html 🪙 ~833 tok (26% · attributed output)
@@ -27,0 +30,4 @@
"""Build a shareable link token for a note."""
import random
return "%d-%d" % (note_id, random.randint(100000, 999999))

[HIGH] make_share_token uses random.randint (non-CSPRNG) and a 6-digit suffix, so the whole token space is only 900,000 values per note_id — brute-forceable to expose any shared note; secrets is already imported and should be used.

Fix: Generate the token with secrets.token_urlsafe and store note_id separately rather than embedding a guessable suffix.

def make_share_token(note_id: int) -> str:
    """Build a shareable link token for a note."""
    return f"{note_id}-{secrets.token_urlsafe(16)}"

📎 ref: https://cwe.mitre.org/data/definitions/330.html

🪙 ~360 tok (11% · attributed output)

**[HIGH]** make_share_token uses random.randint (non-CSPRNG) and a 6-digit suffix, so the whole token space is only 900,000 values per note_id — brute-forceable to expose any shared note; secrets is already imported and should be used. Fix: Generate the token with secrets.token_urlsafe and store note_id separately rather than embedding a guessable suffix. ```python def make_share_token(note_id: int) -> str: """Build a shareable link token for a note.""" return f"{note_id}-{secrets.token_urlsafe(16)}" ``` 📎 ref: https://cwe.mitre.org/data/definitions/330.html 🪙 ~360 tok (11% · attributed output)

[LOW] ruff flags UP031: %-formatting instead of an f-string for the share-token line; the repo's own linter flags this.

Fix: Use an f-string (or, per the high finding, switch to secrets.token_urlsafe).

return f"{note_id}-{random.randint(100000, 999999)}"

🪙 ~179 tok (6% · attributed output)

**[LOW]** ruff flags UP031: %-formatting instead of an f-string for the share-token line; the repo's own linter flags this. Fix: Use an f-string (or, per the high finding, switch to secrets.token_urlsafe). ```python return f"{note_id}-{random.randint(100000, 999999)}" ``` 🪙 ~179 tok (6% · attributed output)
app/auth.py Outdated
@@ -27,0 +35,4 @@
def check_share_token(supplied: str, expected: str) -> bool:
"""Validate a share token supplied in a URL."""
return supplied == expected

[CRITICAL] check_share_token compares the supplied token with ==, leaking the expected value byte-by-byte via a timing side-channel on every request; the repo's own verify_token already uses hmac.compare_digest and house rules require it.

Fix: Use hmac.compare_digest (hmac is already imported at the top of this file).

def check_share_token(supplied: str, expected: str) -> bool:
    """Validate a share token supplied in a URL."""
    return hmac.compare_digest(supplied, expected)

📎 ref: https://cwe.mitre.org/data/definitions/208.html

🪙 ~346 tok (11% · attributed output)

**[CRITICAL]** check_share_token compares the supplied token with `==`, leaking the expected value byte-by-byte via a timing side-channel on every request; the repo's own verify_token already uses hmac.compare_digest and house rules require it. Fix: Use hmac.compare_digest (hmac is already imported at the top of this file). ```python def check_share_token(supplied: str, expected: str) -> bool: """Validate a share token supplied in a URL.""" return hmac.compare_digest(supplied, expected) ``` 📎 ref: https://cwe.mitre.org/data/definitions/208.html 🪙 ~346 tok (11% · attributed output)
@@ -36,0 +38,4 @@
"""Find an owner's notes whose title matches `term`."""
query = (
"SELECT id, owner, title, body FROM notes "
"WHERE owner = '" + owner + "' AND title LIKE '%" + term + "%'"

[CRITICAL] search_notes builds its SQL by string concatenation: owner and term come from request['query'] and are spliced directly, so a term like ' OR 1=1 -- reads every row and owner = 'admin'-- bypasses the owner filter.

Fix: Parameterise owner and term with placeholders and a real LIKE pattern.

cur = self.conn.execute(
            "SELECT id, owner, title, body FROM notes "
            "WHERE owner = ? AND title LIKE ?",
            (owner, f"%{term}%"),
        )
        return cur.fetchall()

📎 ref: https://cwe.mitre.org/data/definitions/89.html

🪙 ~362 tok (11% · attributed output)

**[CRITICAL]** search_notes builds its SQL by string concatenation: owner and term come from request['query'] and are spliced directly, so a term like `' OR 1=1 --` reads every row and `owner = 'admin'--` bypasses the owner filter. Fix: Parameterise owner and term with placeholders and a real LIKE pattern. ```python cur = self.conn.execute( "SELECT id, owner, title, body FROM notes " "WHERE owner = ? AND title LIKE ?", (owner, f"%{term}%"), ) return cur.fetchall() ``` 📎 ref: https://cwe.mitre.org/data/definitions/89.html 🪙 ~362 tok (11% · attributed output)
@@ -36,0 +50,4 @@
if note is None:
continue
cur = self.conn.execute(
"SELECT display_name FROM users WHERE id = ?", (note[1],)

[MEDIUM] notes_with_authors queries a users table that Store.init never creates (only notes is created), so any call raises sqlite3.OperationalError: no such table: users.

Fix: Create the users table in init alongside notes, or document that this method requires an externally-managed schema.

🪙 ~211 tok (7% · attributed output)

**[MEDIUM]** notes_with_authors queries a users table that Store.__init__ never creates (only notes is created), so any call raises sqlite3.OperationalError: no such table: users. Fix: Create the users table in __init__ alongside notes, or document that this method requires an externally-managed schema. 🪙 ~211 tok (7% · attributed output)
@@ -21,1 +21,4 @@
assert len(s.notes_for("alice")) == 1
def test_search_finds_a_note():

[MEDIUM] The only new test is a happy-path search; there is no test that SQL-injection payloads (quotes, OR 1=1) are handled safely, and notes_with_authors (new, and currently broken) has no coverage.

Fix: Add a test asserting search_notes with a term containing single quotes returns only literal matches, and a test exercising notes_with_authors once its schema exists.

🪙 ~264 tok (8% · attributed output)

**[MEDIUM]** The only new test is a happy-path search; there is no test that SQL-injection payloads (quotes, OR 1=1) are handled safely, and notes_with_authors (new, and currently broken) has no coverage. Fix: Add a test asserting search_notes with a term containing single quotes returns only literal matches, and a test exercising notes_with_authors once its schema exists. 🪙 ~264 tok (8% · attributed output)
gitea_admin added 1 commit 2026-08-18 13:00:09 +00:00
Applies the reviewer's suggestion on app/auth.py — hmac was already imported.
pragent-bot bot reviewed 2026-08-18 13:02:13 +00:00
pragent-bot bot left a comment

🤖 AI Review · pragent pilot · glm-5.2:cloud · 4e1285dc

Adds note-title search, share-by-URL tokens, and attachment downloads, plus a notes_with_authors helper for an upcoming feed view. The prior review already flagged the SQL injection in search_notes, the unauthenticated path-traversal in download_attachment, the missing owner-authorization on search, and the non-cryptographic share token; those are not repeated. Reading the surrounding code surfaced three additional issues: notes_with_authors queries a users table the Store schema never creates (crashes on every call), the same method runs N+1 queries per note id, and download_attachment reads the entire file into memory. 3 medium findings.

🔋 AI usage

  • model: glm-5.2:cloud · engine: opencode · agent steps: 7 · duration: 120.3s
  • tokens: 126193 in · 4005 out · 0 reasoning · cache 0 read / 0 write → 130198 total
  • est. cost: $0.00 (on-network glm-5.2:cloud via headroom — no per-token charge)
  • scope: whole-repo checkout at head sha (agent can read any file + run linters, not just the diff) — input tokens include files read beyond the diff
  • per-comment tokens are attributed (one model pass produces all findings; output split by each finding's body weight)
severity location ≈out tok %
MEDIUM app/store.py:53 1640 41%
MEDIUM app/store.py:48 1095 27%
MEDIUM app/api.py:57 1270 32%

3 inline comment(s) posted below.

🤖 **AI Review** · pragent pilot · glm-5.2:cloud · `4e1285dc` Adds note-title search, share-by-URL tokens, and attachment downloads, plus a notes_with_authors helper for an upcoming feed view. The prior review already flagged the SQL injection in search_notes, the unauthenticated path-traversal in download_attachment, the missing owner-authorization on search, and the non-cryptographic share token; those are not repeated. Reading the surrounding code surfaced three additional issues: notes_with_authors queries a users table the Store schema never creates (crashes on every call), the same method runs N+1 queries per note id, and download_attachment reads the entire file into memory. 3 medium findings. ## 🔋 AI usage - model: `glm-5.2:cloud` · engine: opencode · agent steps: 7 · duration: 120.3s - tokens: 126193 in · 4005 out · 0 reasoning · cache 0 read / 0 write → 130198 total - est. cost: $0.00 (on-network glm-5.2:cloud via headroom — no per-token charge) - scope: whole-repo checkout at head sha (agent can read any file + run linters, not just the diff) — input tokens include files read beyond the diff - per-comment tokens are *attributed* (one model pass produces all findings; output split by each finding's body weight) | severity | location | ≈out tok | % | |---|---|---:|---:| | MEDIUM | `app/store.py:53` | 1640 | 41% | | MEDIUM | `app/store.py:48` | 1095 | 27% | | MEDIUM | `app/api.py:57` | 1270 | 32% | _3 inline comment(s) posted below._ <!-- pragent:sha=4e1285dc9683fd6dc087b696a1634c894efd5d24 -->
@@ -33,0 +54,4 @@
if not os.path.exists(path):
return {"status": 404, "body": "not found"}
with open(path, "rb") as fh:
return {"status": 200, "body": fh.read()}

[MEDIUM] download_attachment does fh.read() with no size cap, so a large (or attacker-crafted) attachment loads the entire file into server memory before returning — a memory-exhaustion DoS vector on a public endpoint.

Fix: Stream the response in fixed-size chunks instead of reading the whole file at once, and enforce a max Content-Length.

🪙 ~1270 tok (32% · attributed output)

**[MEDIUM]** download_attachment does fh.read() with no size cap, so a large (or attacker-crafted) attachment loads the entire file into server memory before returning — a memory-exhaustion DoS vector on a public endpoint. Fix: Stream the response in fixed-size chunks instead of reading the whole file at once, and enforce a max Content-Length. 🪙 ~1270 tok (32% · attributed output)
@@ -36,0 +50,4 @@
if note is None:
continue
cur = self.conn.execute(
"SELECT display_name FROM users WHERE id = ?", (note[1],)

[MEDIUM] notes_with_authors runs SELECT display_name FROM users WHERE id = ?, but Store.init only creates the notes table — every call raises sqlite3.OperationalError (no such table: users), so the upcoming feed view will crash the moment it calls this.

Fix: Create the users table in Store.init (with an id + display_name column) before querying it, or redefine the author lookup against a table that actually exists.

🪙 ~1640 tok (41% · attributed output)

**[MEDIUM]** notes_with_authors runs `SELECT display_name FROM users WHERE id = ?`, but Store.__init__ only creates the `notes` table — every call raises sqlite3.OperationalError (no such table: users), so the upcoming feed view will crash the moment it calls this. Fix: Create the `users` table in Store.__init__ (with an `id` + `display_name` column) before querying it, or redefine the author lookup against a table that actually exists. 🪙 ~1640 tok (41% · attributed output)
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/search-and-sharing:feat/search-and-sharing
git checkout feat/search-and-sharing
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gitea_admin/pragent-demo#1