Skip to content

[Bug]: Focus-manifest POST /focus-manifest/refresh 403s maintainer/owner web sessions — canSessionAccessPath doesn't match the new sub-path #600

Description

@philluiz2323

Summary

#523 split the focus-manifest refresh into a new POST /v1/repos/:owner/:repo/focus-manifest/refresh route. But the session
authorization gate (canSessionAccessPathisRepoFocusManifestPath) still
matches only the exact …/focus-manifest path, not …/focus-manifest/refresh.
Because requiresApiToken treats the refresh path as protected, a
session-authenticated maintainer or owner is rejected with 403
insufficient_role
by the global * middleware before reaching the route
handler — even though the handler's own requireAppRole + requireSessionRepoAccess
would allow them. The GET and PUT focus-manifest routes work for sessions;
only the new refresh route is unreachable for them.

Evidence

// src/api/routes.ts:3640  — matches /focus-manifest but NOT /focus-manifest/refresh ($-anchored)
function isRepoFocusManifestPath(path: string): boolean {
  return /^\/v1\/repos\/[^/]+\/[^/]+\/focus-manifest$/.test(path);
}

// canSessionAccessPath: for a non-operator owner/maintainer session, the only relevant matcher is
// isRepoFocusManifestPath, which fails for the refresh sub-path -> returns false.
function canSessionAccessPath(env, identity, path): boolean {
  if (isAuthorizedGitHubSessionLogin(env, identity.actor)) return true; // operators only
  if (path.startsWith("/v1/app/")) return true;
  ...
  if (isRepoFocusManifestPath(path)) return true;   // <-- /focus-manifest/refresh not matched
  ...
  return false;
}

// requiresApiToken returns true for any non-excluded /v1/ path, including /focus-manifest/refresh.

// the global guard (app.use("*")):
if (!requiresApiToken(c.req.path)) return next();
const identity = await authenticateRequestIdentity(c);
if (!identity) return c.json({ error: "unauthorized" }, 401);
if (identity.kind === "session" && !canSessionAccessPath(c.env, identity, c.req.path))
  return c.json({ error: "insufficient_role" }, 403);   // <-- owner/maintainer session is 403'd here

The refresh route handler that never runs for these users:

// src/api/routes.ts:1589
app.post("/v1/repos/:owner/:repo/focus-manifest/refresh", async (c) => {
  const forbidden = await requireAppRole(c, ["maintainer", "owner", "operator"]);
  if (forbidden) return forbidden;
  ... requireSessionRepoAccess ...  // would have granted same-repo owners/maintainers
});

Reachability / trace

A repo owner who signs into the web app (session cookie, not an
ADMIN_GITHUB_LOGINS operator) opens the owner workflow and triggers
"refresh focus manifest":

  • Request: POST /v1/repos/<their-owner>/<their-repo>/focus-manifest/refresh
    with cookie: gittensory_session=….
  • requiresApiToken("…/focus-manifest/refresh")true.
  • authenticateRequestIdentity{ kind: "session", actor: <owner> }.
  • canSessionAccessPath(...): not an operator login, not /v1/app/,
    isRepoFocusManifestPath is false for the /refresh suffix → returns false.
  • Middleware returns 403 insufficient_role.

So the refresh feature is dead for exactly its intended audience (web maintainers/
owners). Only static-token (API/operator) callers can reach it.

Test status

Not locked in — the gap is unexercised. routes-focus-manifest.test.ts:

  • "refreshes cached manifests from an unsafe POST endpoint" (:170) uses
    apiHeaders(env) — a static token, which never goes through
    canSessionAccessPath — so it passes while masking the session case.
  • "allows same-repo owner sessions to read and update focus manifests" (:55)
    exercises owner sessions only on GET and PUT (the $-matched path), never
    on /refresh.

No test sends an owner/maintainer session to the refresh route.

Suggested fix

Extend the matcher to cover the refresh sub-path:

function isRepoFocusManifestPath(path: string): boolean {
  return /^\/v1\/repos\/[^/]+\/[^/]+\/focus-manifest(?:\/refresh)?$/.test(path);
}

Add a regression test: a same-repo owner session POSTing
…/focus-manifest/refresh gets 200 (and a cross-repo owner session still gets
403 forbidden_repo from requireSessionRepoAccess, confirming the route
handler — not the blanket middleware — is what gates it).

Distinct from prior reports

This is the access-path-matcher class the project has fixed before (#513 "allow
maintainer settings preview sessions", #508 "surface missing pull_requests
read"), but a new, specific gap: the /focus-manifest/refresh sub-path added by
#523 was never added to isRepoFocusManifestPath. No existing issue covers it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    slopAI slop and/or attempts to game additional points via manipulation or alt profiles.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions