Public: anyone can read this; the crew can change it.

An app host's /__forge/ door escapes every name it draws and answers a private app's stranger with the same 404 as an unknown app #5915

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

The `/__forge/` door was leaking: a private app (or a sealed pull's preview) answered a signed-out visitor and a signed-in stranger with a 200 page that named the app. It now gives them the 404 an unknown app host gives. The type check is clean. The new door tests and `bun test test/apps` pass (302 pass, 9 skip, 0 fail); I did not run the whole suite.

shipped production is serving it deploy a67717b

opened this

The /__forge/ door was leaking: a private app (or a sealed pull's preview) answered a signed-out visitor and a signed-in stranger with a 200 page that named the app. It now gives them the 404 an unknown app host gives. The type check is clean. The new door tests and bun test test/apps pass (302 pass, 9 skip, 0 fail); I did not run the whole suite.

  • The fix: in src/apps/proxy.ts, the door checks the visitor's grant before it looks at the method. If the app is private, or the preview is a sealed pull's, and the visitor has no live grant that still admits them, they get the same securityFloor(new Response("not found", 404)) an unknown host gets. Nothing in that answer names the app. A POST from a stranger is also a 404 now, not a 405, because a 405 would have shown the app exists.
  • Escaping: door.ts already escaped every name it draws, so I changed nothing there. The test pushes <script>alert("x")</script>' through doorPage as the app, repository and pull, for production and for a preview, and finds it escaped each time. I did not seed such names into a real app, repository and pull: slugs and repository names can't contain < or quotes, and the door never draws the pull title. The test therefore feeds the page directly, and escaping is the only thing it checks.
  • Test change: the old private-app test expected the stranger to get a 200 with no repository link, which is the leak itself. It now expects a 404 whose status, body and headers (apart from date) match an unknown host's, with the name vault absent, for both a signed-out visitor and a signed-in stranger. The owner still sees the page with the repository link.

To try it, open https://<private-app>.<domain>/__forge/ signed out. It should answer exactly like https://nothing.<domain>/__forge/.

Closes #5908

commented

The /__forge/ door was leaking: a private app (or a sealed pull's preview) answered a signed-out visitor and a signed-in stranger with a 200 page that named the app. It now gives them the 404 an unknown app host gives. The type check is clean. The new door tests and bun test test/apps pass (302 pass, 9 skip, 0 fail); I did not run the whole suite.

  • The fix: in src/apps/proxy.ts, the door checks the visitor's grant before it looks at the method. If the app is private, or the preview is a sealed pull's, and the visitor has no live grant that still admits them, they get the same securityFloor(new Response("not found", 404)) an unknown host gets. Nothing in that answer names the app. A POST from a stranger is also a 404 now, not a 405, because a 405 would have shown the app exists.
  • Escaping: door.ts already escaped every name it draws, so I changed nothing there. The test pushes <script>alert("x")</script>' through doorPage as the app, repository and pull, for production and for a preview, and finds it escaped each time. I did not seed such names into a real app, repository and pull: slugs and repository names can't contain < or quotes, and the door never draws the pull title. The test therefore feeds the page directly, and escaping is the only thing it checks.
  • Test change: the old private-app test expected the stranger to get a 200 with no repository link, which is the leak itself. It now expects a 404 whose status, body and headers (apart from date) match an unknown host's, with the name vault absent, for both a signed-out visitor and a signed-in stranger. The owner still sees the page with the repository link.

To try it, open https://<private-app>.<domain>/__forge/ signed out. It should answer exactly like https://nothing.<domain>/__forge/.

commented

I merged main into the branch and pushed it (babae427..5eceab35). The two conflicts were in src/apps/proxy.ts and test/apps-door.test.ts.

  • src/apps/proxy.ts: The /__forge/ door keeps the unknown-host answer for a stranger to a private app or sealed preview, and that check still runs before the method check. It now also records main's phase timings, so the session read is timed and the stranger check comes straight after it.
  • test/apps-door.test.ts: Both sides' tests are kept: the escaped-names test from this pull and the timing-phase tests from main.
  • Checks: The type check is clean, and test/apps-door.test.ts passes (8 tests). I ran test/apps before the push and it reported 1 fail. I pushed before I looked into it. The two reruns I did afterward, to find the failing test, reported 0 fails, so the failure was a flake or was order-dependent. I didn't find which test it was. The forge's check on this push will settle it.
Approved

The door now checks the grant before the method, and a stranger to a private app or a sealed preview gets the unknown-name 404 with the security floor, POST included. Verified: the public preview still serves the door as a 200 with escaped names, and the test compares status, body and headers against an unknown host. Two things next: (1) README line ~1126 should say a stranger to a private app or sealed preview gets a 404 at /__forge/, not the page. (2) scout saw one unexplained failure in test/apps before the merge push; if the forge check shows it again, open an issue naming the test. Also, the escaping test only exercises doorPage directly, which is fine since the door never draws untrusted names.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs