Skip to content

fix(arc-paste): default-deny via allowlist + scrub stale dual-URL docs #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bfirestone merged 1 commit into from
May 1, 2026
Merged

fix(arc-paste): default-deny via allowlist + scrub stale dual-URL docs #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
52 changes: 39 additions & 13 deletions arc-paste/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"os"
"path/filepath"
"strings"

"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
Expand Down Expand Up @@ -60,7 +61,7 @@ func run() error {
}

// newRouter wires the arc-paste HTTP surface. Extracted so tests can exercise
// route precedence (/, /api/v1/*, SPA fallback) without binding a real listener.
// the allowlist without binding a real listener.
func newRouter(handlers *paste.Handlers) *echo.Echo {
e := echo.New()
e.HideBanner = true
Expand All @@ -77,25 +78,50 @@ func newRouter(handlers *paste.Handlers) *echo.Echo {
e.Use(middleware.Recover())
e.Use(middleware.CORS())

// arc-paste only owns /api/paste/*. The arc SPA shell probes /api/v1/* on
// boot; without this guard those probes fall through to the SPA fallback
// and return HTML, which the browser then fails to JSON.parse.
e.Any("/api/v1/*", func(c echo.Context) error {
return c.JSON(http.StatusNotFound, map[string]string{"error": "not found"})
})

// Paste deployments don't serve the arc app shell — the only useful
// destination is /share/<id>. Mirrors the Caddy edge config so dev and
// prod behave the same.
e.Any("/", func(c echo.Context) error {
return c.String(http.StatusNotFound, "Not found")
// Default-deny everything outside the share surface. This mirrors the
// Caddyfile allowlist (arc-paste/Caddyfile) exactly so dev (no Caddy)
// and prod behave the same — without this, the SPA wildcard registered
// by web.RegisterSPA would happily serve the arc app shell at /labels,
// /dashboard, /<projectId>/issues, etc., even though arc-paste deploys
// have no use for any of those routes.
e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
return func(c echo.Context) error {
if !arcPasteAllowedPath(c.Request().URL.Path) {
return c.String(http.StatusNotFound, "Not found")
}
return next(c)
}
})

handlers.Register(e.Group("/api/paste"))
web.RegisterSPA(e)
return e
}

// arcPasteAllowedPath mirrors the Caddyfile allowlist:
//
// /_app/* SvelteKit hashed asset bundle
// /api/paste paste API (create)
// /api/paste/* paste API (per-share routes)
// /robots.txt robots
// /share/<id> share page (exactly one segment after /share/)
//
// Anything else is rejected with the same 404 Caddy returns at the edge.
// Keep this function in sync with arc-paste/Caddyfile if either changes.
func arcPasteAllowedPath(p string) bool {
switch p {
case "/api/paste", "/robots.txt":
return true
}
if strings.HasPrefix(p, "/_app/") || strings.HasPrefix(p, "/api/paste/") {
return true
}
if rest, ok := strings.CutPrefix(p, "/share/"); ok {
return rest != "" && !strings.Contains(rest, "/")
}
return false
}

// envOr returns the value of env variable key, or defaultVal if not set.
func envOr(key, defaultVal string) string {
if v, ok := os.LookupEnv(key); ok {
Expand Down
120 changes: 92 additions & 28 deletions arc-paste/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/sentiolabs/arc/internal/paste"
Expand Down Expand Up @@ -44,37 +43,102 @@ func TestArcPasteCreate(t *testing.T) {
}
}

// arc-paste only owns /api/paste/*. The arc SPA shell calls /api/v1/projects
// on boot; without an explicit guard those requests fall through to the SPA
// fallback and return HTML, which the browser fails to JSON.parse. Return a
// clean JSON 404 so the SPA gets a parseable error instead of a crash.
func TestArcPasteRejectsArcAPIProbes(t *testing.T) {
e := newTestRouter(t)
for _, path := range []string{"/api/v1/projects", "/api/v1/workspaces", "/api/v1/anything/nested"} {
rec := httptest.NewRecorder()
e.ServeHTTP(rec, httptest.NewRequest("GET", path, nil))
if rec.Code != http.StatusNotFound {
t.Errorf("%s: expected 404, got %d", path, rec.Code)
// arc-paste deployments only legitimately serve the share surface. Anything
// outside the allowlist (paths the arc SPA shell normally owns —
// /api/v1/projects on boot, /labels, /<projectId>/issues, /dashboard, /,
// etc.) must 404. Without this, the SPA wildcard registered by
// web.RegisterSPA would happily return the arc app shell HTML for those
// paths, which is both confusing and (for /api/v1/* boot probes) actively
// breaks the SPA because it can't JSON.parse HTML. The test mirrors the
// Caddyfile allowlist exactly so the in-binary guard and the edge stay
// in lockstep.
func TestArcPasteAllowlist(t *testing.T) {
t.Run("rejected paths 404", func(t *testing.T) {
e := newTestRouter(t)
// A representative slice — not exhaustive. Includes the arc SPA boot
// probes and a sampling of the routes the arc app shell normally owns,
// plus shapes that look superficially like /share/<id> but aren't.
rejected := []string{
"/",
"/labels",
"/dashboard",
"/teams",
"/api/v1/projects",
"/api/v1/workspaces",
"/api/v1/anything/nested",
"/share", // missing id
"/share/", // empty id
"/share/abc/sub", // multi-segment after /share/
"/_app", // bare /_app (no trailing slash) is not the asset prefix
"/api/pasteX", // not /api/paste or /api/paste/
"/robots.txt.bak", // not exactly /robots.txt
}
if ct := rec.Header().Get("Content-Type"); !strings.Contains(ct, "application/json") {
t.Errorf("%s: expected JSON content-type, got %q", path, ct)
for _, path := range rejected {
rec := httptest.NewRecorder()
e.ServeHTTP(rec, httptest.NewRequest("GET", path, nil))
if rec.Code != http.StatusNotFound {
t.Errorf("%s: expected 404, got %d (body=%q)", path, rec.Code, rec.Body.String())
}
}
var body map[string]string
if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
t.Errorf("%s: body is not JSON: %v (%q)", path, err, rec.Body.String())
})

t.Run("allowed paths reach handlers", func(t *testing.T) {
e := newTestRouter(t)
// /api/paste create — the existing happy path. Round-trips a real
// CreatePasteRequest so we know the allowlist didn't accidentally
// shadow the handler.
body, _ := json.Marshal(map[string]any{
"plan_blob": []byte{1, 2, 3},
"plan_iv": []byte{4, 5, 6},
"schema_ver": 1,
})
req := httptest.NewRequest("POST", "/api/paste", bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
e.ServeHTTP(rec, req)
if rec.Code != http.StatusCreated {
t.Errorf("/api/paste: expected 201 from handler, got %d (body=%q)", rec.Code, rec.Body.String())
}
}

// Allowlisted GET paths must NOT 404. We don't assert 200 because the
// embedded SPA may not be present in CLI-only test builds — what
// matters is that the allowlist doesn't reject them.
for _, path := range []string{"/share/abc123", "/_app/anything", "/robots.txt"} {
rec := httptest.NewRecorder()
e.ServeHTTP(rec, httptest.NewRequest("GET", path, nil))
if rec.Code == http.StatusNotFound && rec.Body.String() == "Not found" {
t.Errorf("%s: rejected by allowlist (got 404 with allowlist body), want pass-through to handler", path)
}
}
})
}

// arc-paste deployments don't serve the arc app shell — the only useful
// destination is /share/<id>. Returning 404 at the root mirrors the Caddy
// edge configuration so dev (localhost) and prod (arcpaste.company.com)
// behave identically.
func TestArcPasteRootIs404(t *testing.T) {
e := newTestRouter(t)
rec := httptest.NewRecorder()
e.ServeHTTP(rec, httptest.NewRequest("GET", "/", nil))
if rec.Code != http.StatusNotFound {
t.Fatalf("expected 404 at /, got %d (body=%q)", rec.Code, rec.Body.String())
// Sanity-check the allowlist predicate directly so a regression in shape
// matching is obvious without spinning up the full router.
func TestArcPasteAllowedPath(t *testing.T) {
cases := []struct {
path string
want bool
}{
{"/api/paste", true},
{"/api/paste/abc", true},
{"/api/paste/abc/blobs", true},
{"/_app/foo.js", true},
{"/_app/immutable/chunk.abc.js", true},
{"/robots.txt", true},
{"/share/abc123", true},
{"/", false},
{"/labels", false},
{"/share", false},
{"/share/", false},
{"/share/abc/sub", false},
{"/_app", false},
{"/api/pasteX", false},
{"/api/v1/projects", false},
}
for _, tc := range cases {
if got := arcPasteAllowedPath(tc.path); got != tc.want {
t.Errorf("arcPasteAllowedPath(%q) = %v, want %v", tc.path, got, tc.want)
}
}
}
8 changes: 4 additions & 4 deletions docs/runbooks/paste-server.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ Open the **Author URL** (the one with `&t=`) in Chrome/Firefox.
3. **Pick a label** (`praise` / `issue` / `suggestion` / `question` / `nit`).
4. **Type a comment**, optionally toggle "Suggest replacement text".
5. **Post** — the first time you do this, a small modal asks for your name. (As the author you should already have a name from step 1; the modal won't fire.) The comment appears in the sidebar.
6. As the author, you'll see **Accept / Resolve / Reject** controls on every comment. Try Accept on one, Reject (with reply) on another, and Resolve on a third. If the controls don't appear, you opened the Share URL (no `&t=`) instead of the Author URL — close the tab and reopen via the Author URL.
6. As the author, you'll see **Accept / Resolve / Reject** controls on every comment. Try Accept on one, Reject (with reply) on another, and Resolve on a third. If the controls don't appear, you opened a reviewer URL (no `&t=`) instead of the Author URL — close the tab and reopen via the Author URL.
7. As a reviewer (a fresh browser profile or incognito window with NO `&t=` in the URL), find your own annotation in the sidebar. You should see an **✎ Edit** button on it. Click it; the body becomes a textarea pre-filled with your existing comment. Refine the wording — e.g. expand "expand this more" into a fully-formed suggestion — and **⌘/Ctrl-⏎** (or click Save). The card re-renders with `· edited Nm` next to the timestamp. Confirm `arc share comments <id>` prints the new body, not the original.
8. Switch back to the author window. The **✎ Edit** button should also appear on every reviewer's annotation (not just your own). Use it to sharpen a thin reviewer comment — the displayed `author_name` stays as the reviewer, only the body changes. Run `arc share comments <id>` again and confirm the refined body shows up.
9. **Click the chip** in the header — the name prompt opens prefilled with your current name (text selected). Edit and save → chip updates. This is the rename affordance.
Expand All @@ -113,7 +113,7 @@ Open the **Author URL** (the one with `&t=`) in Chrome/Firefox.

### Simulate a second reviewer

Without spinning up another machine, open the **Share URL (no `&t=`)** in an **incognito window** (or a different browser entirely). Incognito gets a fresh `localStorage`, so:
Without spinning up another machine, click the in-page **Share link** button (in the page header on the author's tab) to copy the reviewer URL (`#k=…` only, no `&t=`), then open that URL in an **incognito window** (or a different browser entirely). Incognito gets a fresh `localStorage`, so:

1. **No chip** appears in the header. There's no "Sign in" button anywhere — identity is captured lazily.
2. Highlight a paragraph and click Comment in the toolbar. A name prompt fires; type any name (e.g. "Reviewer-2"). The comment posts; the chip now reads `Reviewer-2` (no `· author`, because the URL has no `&t=`).
Expand All @@ -131,7 +131,7 @@ Walk these in order to confirm the new auth UX end-to-end:
3. **Author URL flow.** Open the Preview URL in a fresh browser profile. Header chip shows `<author_name> · author` immediately, no modal. Accept / Resolve buttons visible on existing comments.
4. **Reviewer URL flow.** From the author's browser tab, click the **Share link** button in the page header — clipboard now holds the bare reviewer URL (`#k=…` only, no `&t=`). Open that copied URL in a different fresh profile. No chip in the header, no "Sign in" button. Select text → click Comment in the toolbar → name modal opens → save a name → comment posts → chip now shows the name.
5. **Rename via chip.** Click the chip on the reviewer side. Modal opens prefilled with the saved name (text selected). Edit, save → chip updates.
6. **Author opens the bare share URL.** Open the share URL (without `&t=`) on the author's browser. They are now in reviewer mode (no Accept buttons). Reopen via the Author URL → author mode restored.
6. **Author opens the bare reviewer URL.** Click the in-page **Share link** button to copy the reviewer URL (`#k=…` only), then paste it into a new tab in the same browser. They are now in reviewer mode (no Accept buttons). Reopen via the Author URL → author mode restored.

## 3. Shared / remote review

Expand Down Expand Up @@ -224,7 +224,7 @@ Step 7 (review loop) uses `arc share approve` and `arc share pull` instead of th

| Symptom | Cause | Fix |
|---|---|---|
| Accept / Resolve / Reject controls never appear | You opened the Share URL (no `&t=`) instead of the Author URL — author detection is token-based, not name-based. Or the share was created without an author name (`git config user.name` was empty and no `--author` flag passed), so `plan.author_name` is empty and the chip never shows `· author` | Close the tab and reopen via the Author URL (run `arc share show <id> --author-url` to retrieve it). If the share has no author name, recreate it with `--author "Your Name"` |
| Accept / Resolve / Reject controls never appear | You opened a reviewer URL (no `&t=`) instead of the Author URL — author detection is token-based, not name-based. Or the share was created without an author name (`git config user.name` was empty and no `--author` flag passed), so `plan.author_name` is empty and the chip never shows `· author` | Close the tab and reopen via the Author URL (run `arc share show <id> --author-url` to retrieve it). If the share has no author name, recreate it with `--author "Your Name"` |
| `/share/<id>` returns `{"message":"Not Found"}` (Echo's default 404 JSON) | Binary built without the `webui` build tag — `web.RegisterSPA` is the no-op stub | Rebuild with `make build` (not `make build-quick`); for arc-paste use `go build -tags webui -o ./bin/arc-paste ./arc-paste` |
| `/share/<id>` returns blank HTML / cannot find static assets | `web/build/` not present at compile time, even with the `webui` tag | Re-run `bun run build` in `web/`, then rebuild the binary |
| SPA console says `missing #k=<key> in URL` | URL was pasted without its fragment | Use the full URL printed by `arc share create` — fragments are dropped by some chat apps; copy carefully |
Expand Down
12 changes: 6 additions & 6 deletions docs/runbooks/review_howto.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ The discriminator is whether the comment should *cause an edit downstream*. Acce

## URLs you'll receive

`arc share create` prints two URLs:
`arc share create` prints exactly one URL — the **Author URL** (or **Preview URL** for `--local` shares). It contains both the decryption key and the `&t=<edit_token>` that grants Accept / Resolve / Reject. Treat it like a write password: don't paste it into tickets, screenshots, or shared chat threads. The first time you open it, the page detects your role from the URL itself; you don't sign in or pick a name.

| URL | What it is | Who gets it |
|---|---|---|
| **Share URL** (`#k=…`) | Read + comment | Reviewers |
| **Author URL** (`#k=…&t=…`) | Adds Accept / Resolve / Reject | Plan author only |
| URL form | What it is | Who gets it | Source |
|---|---|---|---|
| **Author URL** (`#k=…&t=…`) | Read + comment + Accept / Resolve / Reject | Plan author only | Printed by `arc share create` |
| **Reviewer URL** (`#k=…` only, no `&t=`) | Read + comment | Reviewers | Click the in-page **Share link** button on the share page header |

The author URL is a strict superset of the share URL — anyone given the author URL can also read and comment. Treat it like a write password: don't paste it into tickets, screenshots, or shared chat threads. The first time you open the author URL, the page detects your role from the URL itself; you don't sign in or pick a name.
To send a reviewer link: open the Author URL in your browser, click the **Share link** button in the page header, and paste the resulting URL into your message. The button strips `&t=` so the URL you share grants reviewer-only access — copy-pasting the bare Author URL would hand the recipient your edit token.

Lost the URL? Run `arc share show <id> --author-url` to reprint it (uses the `edit_token` saved to `~/.arc/shares.json`).

Expand Down
Loading