Skip to content

OAuth state cookie set without HttpOnly (CookieData JSDoc claims default true) #3207

Description

@jamesfrei

Issue summary

The OAuth state cookie set during auth.begin() is emitted without the HttpOnly attribute, despite the CookieData.httpOnly JSDoc in runtime/http/cookies.ts documenting that the default is true. The same flaw applies to the session cookie set on the OAuth callback for non-embedded apps, and to any other cookie written through Cookies.set / Cookies.setAndSign without explicitly passing httpOnly: true.

Found while triaging an OWASP ZAP baseline scan against an embedded Shopify app, which flagged Cookie No HttpOnly Flag on the OAuth-begin redirect.

Versions affected

  • @shopify/shopify-api: 13.0.0 (latest at time of writing).
  • Reproduces on main at SHA f39d1bc1b77230f66bdd493ac7fd69641413553b.
  • Transitively affects every consumer including @shopify/shopify-app-remix (verified at 4.2.0), @shopify/shopify-app-express, and @shopify/shopify-app-react-router.

Reproduction

Any OAuth-begin trigger emits a Set-Cookie for shopify_app_state (and its .sig companion) that lacks HttpOnly. For example with a stock shopify-app-remix install:

$ curl -sI "https://<your-app>/auth?shop=<your-shop>.myshopify.com" | grep -i set-cookie
Set-Cookie: shopify_app_state=<nonce>;sameSite=lax; secure=true; path=/auth/callback;expires=<date>
Set-Cookie: shopify_app_state.sig=<sig>;sameSite=lax; secure=true; path=/auth/callback;expires=<date>

Notice: no HttpOnly attribute on either cookie.

Source trace:

  1. packages/apps/shopify-api/lib/auth/oauth/oauth.ts#L95-L100 calls cookies.setAndSign(STATE_COOKIE_NAME, state, { expires, sameSite, secure, path }). No httpOnly is passed.
  2. packages/apps/shopify-api/runtime/http/cookies.ts#L154-L176 (set and setAndSign) spread opts into the cookie jar without applying any default for httpOnly.
  3. packages/apps/shopify-api/runtime/http/cookies.ts#L83-L95 (encodeCookie) only emits attributes that are own properties of the cookie data, so the absent httpOnly produces a header with no flag.

The same flaw is present on the non-embedded session-cookie write at lib/auth/oauth/oauth.ts#L219-L226.

Expected behaviour

Either:

  1. The state and session cookies are emitted with HttpOnly, OR
  2. Cookies.set / Cookies.setAndSign apply httpOnly: true by default, matching the existing JSDoc on CookieData.httpOnly which already states "true by default".

Actual behaviour

Neither location applies the documented default. Cookies are written without HttpOnly unless the caller explicitly passes it. No caller in lib/auth/oauth/oauth.ts does.

Proposed fix (smallest possible)

The simplest, surgical fix is one line in lib/auth/oauth/oauth.ts:

await cookies.setAndSign(STATE_COOKIE_NAME, state, {
  expires: new Date(Date.now() + 60000),
  sameSite: 'lax',
  secure: true,
  httpOnly: true,
  path: callbackPath,
});

A more thorough fix that also makes the JSDoc accurate and protects every other call site (including the non-embedded session cookie at line 220) is to apply the default in Cookies.set:

set(name: string, value: string, opts: Partial<CookieData> = {}): void {
  this.outgoingCookieJar[name] = {
    httpOnly: true, // matches CookieData.httpOnly JSDoc
    ...opts,
    name,
    value,
  };
  this.updateHeader();
}

Either fix is non-breaking: callers can still pass httpOnly: false explicitly if they need JS access (none in lib/auth do).

Risk / severity

Low. Defense-in-depth, not a primary vulnerability. The state cookie is short-lived (60s), single-use, path-scoped to the callback, and HMAC-signed. An attacker who can read it via XSS already has bigger primitives on the same origin. But the cookie has zero legitimate need for JS access, and the JSDoc already promises the default is true. Fixing this aligns the implementation with the documented contract and with OWASP and RFC 6265 § 4.1.2.6 guidance.

Happy to send a PR if the team agrees on the broader (Cookies.set default) fix vs. the surgical one.

Checklist

Metadata

Metadata

Assignees

No one assigned

    Labels

    devtools-gardenerPost the issue or PR to Slack for the gardener

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions