Skip to content

fix(adt): CSRF HEAD→GET fallback + secure-cookie fix + SAP_SESSION_TYPE env var#120

Open
frd1201 wants to merge 3 commits into
oisee:mainfrom
frd1201:fix/csrf-head-fallback-and-session-type
Open

fix(adt): CSRF HEAD→GET fallback + secure-cookie fix + SAP_SESSION_TYPE env var#120
frd1201 wants to merge 3 commits into
oisee:mainfrom
frd1201:fix/csrf-head-fallback-and-session-type

Conversation

@frd1201

@frd1201 frd1201 commented Apr 23, 2026

Copy link
Copy Markdown

Relates to #88. Closes #104.

Summary

Three related fixes for systems where vsp write operations fail with CSRF or lock-handle errors. All three were reproduced and verified on a real on-premise SAP S/4HANA system accessed through an nginx reverse proxy over plain HTTP.

1. CSRF token: HEAD→GET fallback — closes #104

fetchCSRFToken() uses HEAD for performance. On systems where CL_ADT_WB_RES_APP does not implement HEAD (returns 400 or 403 without a token in the response headers), vsp fails with "no CSRF token in response" or "access forbidden (403)" for all write operations.

Fix: if HEAD returns no usable token, automatically retry with GET — which is what Eclipse ADT uses. HEAD is still attempted first; the fallback only fires when needed, so systems where HEAD works are unaffected (~0 ms overhead).

Diagnostic used to confirm:

# HEAD → 400, no token
curl -sk -o /dev/null -w "%{http_code}" -X HEAD -u USER:PASS \
  -H "X-CSRF-Token: Fetch" "http://host:1440/sap/bc/adt/core/discovery?sap-client=090"
# → 400

# GET → 200 + token (same as Eclipse ADT)
curl -sk -D - -o /dev/null -X GET -u USER:PASS \
  -H "X-CSRF-Token: Fetch" "http://host:1440/sap/bc/adt/core/discovery?sap-client=090" \
  | grep x-csrf-token
# → x-csrf-token: Pa_4WUq0iJFTXv2SbFiWeQ==

Also fixes a double-defer bug in the previous fallback implementation (both defers pointed to the GET body after resp was reassigned).

2. Secure-cookie stripping for HTTP reverse proxies

SAP systems behind nginx or other HTTP proxies often set session cookies (SAP_SESSIONID_*) with the Secure flag. Go's standard cookiejar refuses to send Secure cookies over plain HTTP, so the session cookie is never sent back on subsequent requests — the CSRF token appears "abgelaufen" (expired) because SAP can't locate the session.

Fix: httpCookieJar wraps cookiejar.Jar and strips the Secure flag when storing cookies received over HTTP. Outgoing HTTPS requests are unaffected.

Observed response from the affected system:

set-cookie: SAP_SESSIONID_D18_090=...; path=/; secure   ← Go drops this over HTTP
x-csrf-token: Pa_4WUq0iJFTXv2SbFiWeQ==

3. SAP_SESSION_TYPE env var — relates to #88

Exposes adt.SessionType via the SAP_SESSION_TYPE environment variable (stateful | stateless | keep). Setting stateful causes vsp to send X-sap-adt-sessiontype: stateful on every request, which keeps the SAP session alive across the Lock→Write sequence on systems that require it.

Invalid values emit a clear warning to stderr instead of silently falling back.

SAP_SESSION_TYPE=stateful

Note: the upstream commit 22517d4 already pins stateful on individual write paths. This env var is complementary — it provides a system-wide override for operators who need all requests to be stateful without patching individual call sites.

Two fixes for systems where standard vsp write operations fail:

1. CSRF token: HEAD→GET fallback (fixes oisee#104)

   fetchCSRFToken() uses HEAD for speed. On systems where the ICF
   handler CL_ADT_WB_RES_APP does not implement HEAD (returns 400 or
   403 without a token), fall back to GET automatically — which is
   what Eclipse ADT uses. HEAD is still tried first; only if it
   returns no usable token does the GET happen, so fast systems are
   unaffected.

2. Secure-cookie stripping for HTTP reverse proxies

   SAP systems behind nginx/other HTTP proxies often set session
   cookies with the Secure flag. Go's standard cookiejar refuses to
   send Secure cookies over plain HTTP, so the session cookie never
   reaches SAP on subsequent requests and the CSRF token appears
   expired. httpCookieJar strips the Secure flag when storing cookies
   received over HTTP, allowing the session to be maintained.

3. SAP_SESSION_TYPE env var (partial fix for oisee#88)

   Exposes adt.SessionType via SAP_SESSION_TYPE (stateful|stateless|
   keep). Setting stateful forces X-sap-adt-sessiontype: stateful on
   every request, which keeps lock handles valid across the
   Lock→Write sequence on systems that require it. Invalid values
   emit a warning to stderr instead of silently falling back.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
txape10 added a commit to txape10/vibing-steampunk that referenced this pull request May 27, 2026
The fallback introduced in PR oisee#120 triggered whenever the HEAD response
carried no CSRF token, including 401/403 responses where the token is
absent because credentials are invalid — not because HEAD is unsupported.

This caused TestTransport_Request_RetryOn401_ReauthFails to fail: the
extra GET request consumed a mock slot and pushed the request count from
2 to 3.

Guard the fallback with !headIsAuthFailure so it only activates when
HEAD is structurally missing the token (method not supported, 200 with
no header, etc.), not when the user is simply unauthorized.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@txape10

txape10 commented May 27, 2026

Copy link
Copy Markdown

Found a bug when applying this PR: the HEAD→GET CSRF fallback fires on any
empty token header, including 401/403 responses where the token is absent
because credentials are invalid — not because HEAD is unsupported.

This breaks TestTransport_Request_RetryOn401_ReauthFails ("Expected 2
requests, got 3") because the extra GET consumes a mock slot.

Fix — guard the fallback against auth failures:

headIsAuthFailure := headResp.StatusCode == http.StatusUnauthorized ||
                     headResp.StatusCode == http.StatusForbidden
if (headToken == "" || headToken == "Required") && !headIsAuthFailure {
    // GET fallback
}
On 401/403 retrying with GET won't helpthe credentials are wrong, not
the HTTP method. This keeps the original test passing and avoids an
unnecessary round-trip on auth failures.

frd1201 and others added 2 commits June 19, 2026 13:01
… review)

The HEAD->GET CSRF fallback in fetchCSRFToken fired on any empty/Required
token, including 401/403 responses where the token is absent because the
credentials are invalid. The extra GET wastes a round-trip and breaks
TestTransport_Request_RetryOn401_ReauthFails (3 requests vs expected 2).
Guard the fallback with headIsAuthFailure so it only retries with GET when
HEAD returns no token for a non-auth reason.
frd1201 added a commit to frd1201/vibing-steampunk that referenced this pull request Jun 19, 2026
… review)

The HEAD->GET CSRF fallback in fetchCSRFToken fired on any empty/Required
token, including 401/403 responses where the token is absent because the
credentials are invalid. The extra GET wastes a round-trip and breaks
TestTransport_Request_RetryOn401_ReauthFails (3 requests vs expected 2).
Guard the fallback with headIsAuthFailure so it only retries with GET when
HEAD returns no token for a non-auth reason.
@frd1201

frd1201 commented Jun 19, 2026

Copy link
Copy Markdown
Author

Thanks @txape10 — fixed exactly as you suggested. The HEAD→GET CSRF fallback in fetchCSRFToken now skips 401/403 (auth failures), so it only retries with GET when HEAD genuinely returns no token for a non-auth reason. TestTransport_Request_RetryOn401_ReauthFails is back to passing (2 requests, no extra round-trip on bad credentials). Pushed — would appreciate another look!

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.

CSRF token fetch fails (403) on S/4HANA public cloud and some on-premise systems — HEAD not supported by CL_ADT_WB_RES_APP

2 participants