Skip to content

feat: public note sharing (/s/<slug>) + management API - #3

Merged
chapati23 merged 7 commits into
mainfrom
feat/public-sharing
Mar 1, 2026
Merged

feat: public note sharing (/s/<slug>) + management API#3
chapati23 merged 7 commits into
mainfrom
feat/public-sharing

Conversation

@gisk0

@gisk0 gisk0 commented Mar 1, 2026

Copy link
Copy Markdown
Owner

Summary

Extends the obs.gisk0.dev Cloudflare Worker with a public note sharing feature, so Obsidian notes can be shared as clean HTML pages.

New Routes

Route Method Description
/s/<slug> GET Public: renders note as HTML
/api/publish PUT Auth: publish/update a note
/api/publish/<slug> DELETE Auth: unpublish a note
/api/published GET Auth: list all published notes

Auth is via X-Publish-Token header. Token stored in pass obsidian-redirect/publish-token.

Infrastructure

  • KV namespace: PUBLISHED_NOTES (id: cf83f0a936324351985100e78363da07)
  • KV format: { title, markdown, publishedAt, updatedAt }
  • Markdown: rendered with marked (bundled, ~62 KiB gzip 16 KiB)
  • Secret: PUBLISH_TOKEN set via wrangler secret

HTML Template

  • Responsive light theme (system fonts, 720px max-width)
  • Title as h1 + published date in header
  • Footer: "Shared via Giskard 🔮"
  • No JS, no tracking, no cookies
  • Clean 404 page

Helper Script

scripts/publish-note.sh in the Giskard vault lets you publish any vault note:

publish-note.sh "research/my-analysis" "my-analysis"
# → https://obs.gisk0.dev/s/my-analysis

Testing ✅

All tests pass on the deployed worker:

  • ✅ Health check
  • ✅ Publish note
  • ✅ Rendered HTML (markdown → HTML with marked)
  • ✅ List API
  • ✅ Delete note
  • ✅ 404 after delete
  • ✅ Existing Obsidian redirect routes unchanged
  • ✅ Unauthorized request rejected (401)

gisk0 added 2 commits March 1, 2026 11:02
## What's new

### Public note serving
- GET /s/<slug> renders Obsidian markdown notes as clean HTML
- Responsive light theme with system fonts, good typography
- Published date shown in header
- Subtle footer: 'Shared via Giskard 🔮'
- Clean 404 page for unknown slugs

### Management API (protected by X-Publish-Token header)
- PUT /api/publish — publish or update a note
- DELETE /api/publish/<slug> — unpublish a note
- GET /api/published — list all published notes with metadata

### Markdown rendering
- Uses 'marked' library (bundled via wrangler/esbuild)
- Supports: headers, bold/italic, links, code blocks, tables,
  blockquotes, images, lists

### Infrastructure
- KV namespace: PUBLISHED_NOTES (cf83f0a936324351985100e78363da07)
- PUBLISH_TOKEN secret stored in Cloudflare + pass obsidian-redirect/publish-token
- Existing Obsidian URI redirect routes preserved unchanged
## Infrastructure as Code

All Cloudflare infra for obs.gisk0.dev is now managed via Terraform
using the cloudflare/cloudflare provider (v5.17).

### Resources managed
- cloudflare_workers_kv_namespace.published_notes — PUBLISHED_NOTES KV
- cloudflare_workers_script.obsidian_redirect — bundled TS worker
- cloudflare_workers_route.obs_gisk0_dev — obs.gisk0.dev/* route

### Workflow
  make tf-init    # first time only
  make tf-plan    # see changes
  make tf-apply   # deploy
  make deploy     # build + apply in one shot

### Secrets
- CLOUDFLARE_API_TOKEN from pass cloudflare/workers-api-token
- PUBLISH_TOKEN from pass obsidian-redirect/publish-token
- Never hardcoded, read at plan/apply time via pass

### State
State stored locally in terraform/terraform.tfstate (gitignored).
Provider lock file committed for reproducible builds.

### Tool
Uses OpenTofu (open-source Terraform fork, full HCL/provider compat).
Install: https://opentofu.org
@gisk0

gisk0 commented Mar 1, 2026

Copy link
Copy Markdown
Owner Author

♻️ Updated: Terraform IaC added

All Cloudflare infrastructure is now managed via Terraform (OpenTofu) in terraform/.

Resources under Terraform control:

  • cloudflare_workers_kv_namespace.published_notes — KV namespace
  • cloudflare_workers_script.obsidian_redirect — worker deployment
  • cloudflare_workers_route.obs_gisk0_devobs.gisk0.dev/* route

Workflow:

make tf-init    # first time
make tf-plan    # preview changes
make tf-apply   # deploy (build → apply)

Secrets read from pass at apply time — never hardcoded. State gitignored (local, private repo).

tofu plan and tofu apply verified clean ✅

@gisk0

gisk0 commented Mar 1, 2026

Copy link
Copy Markdown
Owner Author

PR Review: #3 — feat: public note sharing (/s/) + management API

Verdict: Approve with suggestions

Summary: Clean, well-structured extension of the existing worker. The code is readable, the Terraform setup is solid, secrets are handled properly via pass, and the HTML template is tasteful. A few security and robustness items worth addressing.


🔴 Blockers (must fix before merge)

  • [src/index.ts:224] handlePutPublish calls req.json() without a try/catch. If the body is not valid JSON (or content-type is wrong), this throws an unhandled exception → 500. Wrap in try/catch and return a 400.

  • [src/index.ts:213] XSS via marked output. The marked library renders raw HTML in markdown by default. A malicious publish request could include <script> tags in the markdown field, which get rendered verbatim into the page. Since publishing requires auth, this is low-probability but still a real stored XSS vector (compromised token = XSS on your domain). Fix: Configure marked to escape raw HTML, e.g. use a custom renderer that escapes HTML tokens, or strip HTML tags before rendering.

🟡 Warnings (should fix, not blocking)

  • [src/index.ts:230] No slug validation. Slugs are used as KV keys and in URL paths. A slug containing special characters or path traversal patterns could cause unexpected behavior. Add a regex check like /^[a-z0-9][a-z0-9-]*$/.

  • [src/index.ts:242] No size limit on the request body. A valid auth token could PUT a multi-MB markdown blob into KV. Consider checking Content-Length or limiting markdown length (e.g. 512KB).

  • [src/index.ts:253-266] handleListPublished fetches all KV keys then does N individual get() calls. KV list() returns max 1000 keys by default and won't paginate beyond that. Also, N sequential gets is slow — consider storing metadata separately or using list() with metadata.

  • [terraform/variables.tf:2-4] Cloudflare account ID and zone ID are hardcoded as defaults. Not secrets, but environment-specific. Consider moving them to terraform.tfvars (already gitignored) to keep the TF files portable.

  • [terraform/main.tf:50-53] The cloudflare_workers_route resource uses script attribute — verify this matches the Cloudflare provider v5 schema (some v5 resources changed to script_name). If tofu plan works, ignore this.

  • [wrangler.toml vs terraform/main.tf] The KV namespace ID in wrangler.toml (cf83f0a...) differs from the Terraform-managed resource (which will get a new ID on first apply if not imported). The README covers importing, but this mismatch could bite during initial setup.

💡 Suggestions (optional but worth considering)

  • [src/index.ts] Consider adding Cache-Control headers to public note responses. Even max-age=60 would reduce KV reads significantly for popular notes.

  • [src/index.ts:280-290] The vault path regex now matches /api/anything and /s/slug/extra if they fall through. Route ordering prevents issues today, but an explicit exclusion of reserved prefixes (/s/, /api/) in the vault matcher would be more defensive.

  • [package.json] @types/marked is listed but marked v17 ships its own types. You can likely drop @types/marked (it's for older versions).

✅ Good Stuff

  • Secrets management is excellent — pass integration, sensitive = true in TF, no hardcoded values anywhere.
  • The Makefile workflow (build → plan → apply) is clean and operator-friendly.
  • HTML/CSS template is responsive, minimal, and looks good. No JS, no tracking — exactly right for shared notes.
  • Preserving publishedAt on updates while bumping updatedAt is a nice touch.
  • Terraform README is thorough with import instructions for existing resources.

Test Coverage: Manual integration tests documented in PR — solid for this scope. No unit tests, but the worker is thin enough that integration tests cover the surface.
Security: XSS via marked is the main concern (see blocker above). Auth model is simple but appropriate.
Ready to merge? After fixing the JSON parse error handling and the XSS vector.

gisk0 added 3 commits March 1, 2026 11:25
Blockers:
- Wrap req.json() in try/catch, return 400 on malformed JSON
- Add HTML sanitizer (strips script/iframe/object/embed tags, event
  handlers, javascript:/vbscript:/data: URLs) — CF Workers compatible

Warnings:
- Validate slugs: only [a-z0-9-], max 128 chars, reject path traversal
- Reject request bodies > 1MB via Content-Length check
- Replace N+1 KV reads in list endpoint with _meta:index key
  (single KV read, updated on publish/delete)
- Move hardcoded account_id/zone_id to terraform.tfvars (gitignored)
- KV namespace ID already consistent between wrangler.toml and TF state
- TF provider v5.17 schema verified compatible

Suggestions:
- Add Cache-Control: public, max-age=300 on public note responses
- Add defensive 404 for unmatched /api/ and /s/ paths
- Drop @types/marked (marked v17 has built-in types)

Ref: PR #3 review comment
- CI on PRs: TruffleHog secret scan, typecheck + build, tofu validate + fmt
- Secret scan on all pushes via TruffleHog
chapati23
chapati23 previously approved these changes Mar 1, 2026
- Add Trunk with TruffleHog, Prettier, ESLint, shellcheck, tflint, checkov
- Replace standalone secret-scan.yml with trunk-io/trunk-action
- Unified CI workflow: Quality Checks (trunk + build + typecheck + terraform)
- Add package-lock.json (fixes npm ci cache error)
- Terraform validate now runs after build (fixes missing dist/index.js)
- Add required_version to terraform block (fixes tflint)
- Add permissions: contents: read to deploy.yml (fixes checkov)
- Auto-format all files with trunk fmt (prettier, shfmt, taplo)
@gisk0

gisk0 commented Mar 1, 2026

Copy link
Copy Markdown
Owner Author

PR Review: #3 — feat: public note sharing (/s/) + management API

Verdict: Approve ✅

Summary: Clean, well-structured extension of the Cloudflare Worker adding public note sharing with token-authed management API, Terraform IaC, and Trunk CI. Code quality is high — good input validation, sanitization, defensive routing, and the meta-index pattern avoids N+1 KV reads. No blockers.


🔴 Blockers (must fix before merge)

None.

🟡 Warnings (should fix, not blocking)

  • [src/index.ts:113-126] Regex-based HTML sanitizer is bypassable. The sanitizer strips dangerous tags/attrs via regex, which is inherently fragile against crafted payloads (e.g. nested tags, null bytes, encoding tricks). Since the input is markdown from an authenticated author (yourself), the risk is low. But if this ever becomes multi-tenant, swap to a proper sanitizer. Worth a // TODO comment noting the trust boundary.

  • [src/index.ts:283] Content-Length body size check is spoofable. The Content-Length header check can be omitted or faked by clients. Consider also reading the body with a size-limited stream, or at minimum checking body.markdown.length after parsing. Again, since the only caller is your own publish script behind a token, low practical risk.

  • [src/index.ts:256] Token comparison is not constant-time. === string comparison leaks timing information. For a single-user tool behind a secret token this is academic, but if you ever want defense-in-depth: crypto.subtle.timingSafeEqual or compare SHA-256 hashes.

  • [terraform/README.md:38-46] Cloudflare account ID and zone ID in README. The account ID 8a85c2c4aef344232708c5b962fdaf78 and zone ID 2909c763f12cb23ad4c40f40f131b747 are in the README import examples. These aren't secrets per se (they're in API URLs), but they're unnecessary exposure in a public repo. Consider using placeholders.

💡 Suggestions (optional but worth considering)

  • [src/index.ts:140] escHtml missing single-quote escaping. The function escapes &<>" but not '. Adding '&#39; would make it universally safe.

  • [wrangler.toml:8] KV namespace ID hardcoded. The KV namespace ID is duplicated between wrangler.toml and Terraform. If Terraform ever recreates the namespace, these will drift. A comment noting this dependency would help.

  • [.github/workflows/ci.yml:34-51] Terraform validate job rebuilds the worker. Consider caching the build artifact from the build job via artifact upload/download instead of rebuilding.

✅ Good Stuff

  • Meta-index pattern for listing notes — single KV read instead of list+get-each.
  • Defensive catch-all for /api/ and /s/ unmatched paths prevents route leakage.
  • Slug validation with regex + length limit is solid.
  • Terraform setup is clean — secrets via pass, state gitignored, sensitive = true on token var.
  • Trunk + CI with actionlint, trufflehog, checkov, osv-scanner — good security tooling.
  • PR description is excellent — clear routes table, test results, architecture decisions documented.

Test Coverage: No automated tests in repo (manual curl verification noted in PR). Acceptable for a personal tool of this size.
Security: Clean — no secrets in code, auth on all management endpoints, HTML sanitization present (regex-based, see warning).
Ready to merge? Yes ✅

Security (warnings):
1. Replace regex-based HTML sanitizer with allowlist-based approach —
   only permitted tags/attributes survive, everything else is stripped
2. Add post-parse body size checks (markdown 512KB, title 1KB) —
   Content-Length header alone is spoofable
3. Use crypto.subtle.timingSafeEqual() for token comparison —
   prevents timing side-channel attacks
4. Replace real CF account/zone IDs with placeholders in terraform/README.md

Improvements (suggestions):
5. Add single-quote escaping (&#39;) to escHtml()
6. Add comments in wrangler.toml documenting KV namespace ID must match
   Terraform state, with command to retrieve it
7. Eliminate redundant npm ci + build in CI terraform job — use
   upload/download-artifact from the build job instead
@chapati23
chapati23 merged commit 18c4110 into main Mar 1, 2026
3 checks passed
@chapati23
chapati23 deleted the feat/public-sharing branch March 1, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants