From 29a257b284639ec996c213178473b9926f9dae90 Mon Sep 17 00:00:00 2001 From: frd1201 Date: Thu, 23 Apr 2026 18:05:00 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(adt):=20CSRF=20HEAD=E2=86=92GET=20fallb?= =?UTF-8?q?ack=20+=20SAP=5FSESSION=5FTYPE=20env=20var?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes for systems where standard vsp write operations fail: 1. CSRF token: HEAD→GET fallback (fixes #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 #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 --- cmd/vsp/main.go | 5 +++++ internal/mcp/server.go | 13 +++++++++++++ pkg/adt/config.go | 26 +++++++++++++++++++++++++- pkg/adt/http.go | 38 ++++++++++++++++++++++++++++++++------ 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/cmd/vsp/main.go b/cmd/vsp/main.go index 92acdb48..162cabc0 100755 --- a/cmd/vsp/main.go +++ b/cmd/vsp/main.go @@ -469,6 +469,11 @@ func resolveConfig(cmd *cobra.Command) { } } + // Session type: SAP_SESSION_TYPE env (stateful | stateless | keep) + if v := viper.GetString("SESSION_TYPE"); v != "" { + cfg.SessionType = v + } + // Keep-alive interval: flag > SAP_KEEPALIVE env if !cmd.Flags().Changed("keepalive") { if v := viper.GetString("KEEPALIVE"); v != "" { diff --git a/internal/mcp/server.go b/internal/mcp/server.go index b0cd7c75..8eda7f21 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -4,6 +4,7 @@ package mcp import ( "context" "fmt" + "os" "strings" "sync" "time" @@ -87,6 +88,9 @@ type Config struct { // Graph / co-change configuration TransportAttribute string // E070A attribute name for CR-level co-change aggregation + // Session type: "stateful" keeps SAP session across requests (required for lock/write flows) + SessionType string + // Debugger configuration TerminalID string // SAP GUI terminal ID for cross-tool breakpoint sharing @@ -126,6 +130,15 @@ func NewServer(cfg *Config) *Server { if cfg.Verbose { opts = append(opts, adt.WithVerbose()) } + if cfg.SessionType != "" { + st := adt.SessionType(cfg.SessionType) + switch st { + case adt.SessionStateful, adt.SessionStateless, adt.SessionKeep: + opts = append(opts, adt.WithSessionType(st)) + default: + fmt.Fprintf(os.Stderr, "[vsp] warning: unknown SAP_SESSION_TYPE %q, using default (stateless)\n", cfg.SessionType) + } + } if cfg.ReauthFunc != nil { opts = append(opts, adt.WithReauthFunc(cfg.ReauthFunc)) } diff --git a/pkg/adt/config.go b/pkg/adt/config.go index 719d6e3e..1aa0e8c4 100755 --- a/pkg/adt/config.go +++ b/pkg/adt/config.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/http/cookiejar" + "net/url" "time" ) @@ -227,9 +228,32 @@ func WithTerminalID(terminalID string) Option { } } +// httpCookieJar wraps cookiejar.Jar and strips the Secure flag when storing cookies +// received over plain HTTP. This is required for SAP systems accessed via HTTP reverse +// proxies that still set Secure cookies (e.g. nginx in front of SAP ICM). Go's standard +// jar won't send Secure cookies on HTTP requests, causing CSRF tokens to appear expired +// because the session cookie never reaches the server on subsequent requests. +type httpCookieJar struct { + *cookiejar.Jar +} + +func (j *httpCookieJar) SetCookies(u *url.URL, cookies []*http.Cookie) { + if u.Scheme == "http" { + stripped := make([]*http.Cookie, len(cookies)) + for i, c := range cookies { + copy := *c + copy.Secure = false + stripped[i] = © + } + cookies = stripped + } + j.Jar.SetCookies(u, cookies) +} + // NewHTTPClient creates an http.Client configured for the given Config. func (c *Config) NewHTTPClient() *http.Client { - jar, _ := cookiejar.New(nil) + base, _ := cookiejar.New(nil) + jar := &httpCookieJar{base} transport := &http.Transport{ Proxy: http.ProxyFromEnvironment, // Honor HTTP_PROXY/HTTPS_PROXY env vars diff --git a/pkg/adt/http.go b/pkg/adt/http.go index d3b30227..5d88fdad 100755 --- a/pkg/adt/http.go +++ b/pkg/adt/http.go @@ -308,17 +308,43 @@ func (t *Transport) fetchCSRFToken(ctx context.Context) error { req.Header.Set("X-sap-adt-sessiontype", "stateful") } - resp, err := t.httpClient.Do(req) + headResp, err := t.httpClient.Do(req) if err != nil { return fmt.Errorf("executing request: %w", err) } - defer resp.Body.Close() - - // Drain body to allow connection reuse - _, _ = io.Copy(io.Discard, resp.Body) + _, _ = io.Copy(io.Discard, headResp.Body) + headResp.Body.Close() + + // Use the HEAD response by default; fall back to GET when HEAD returns no usable token. + // Some SAP systems (on-premise, S/4HANA cloud) reject HEAD or return no token — + // CL_ADT_WB_RES_APP may not implement HEAD. GET is what Eclipse ADT uses. + resp := headResp + headToken := headResp.Header.Get("X-CSRF-Token") + if headToken == "" || headToken == "Required" { + reqGet, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) + if err != nil { + return fmt.Errorf("creating GET fallback request: %w", err) + } + if t.config.HasBasicAuth() { + reqGet.SetBasicAuth(t.config.Username, t.config.Password) + } + t.addCookies(reqGet) + reqGet.Header.Set("X-CSRF-Token", "fetch") + reqGet.Header.Set("Accept", "*/*") + if t.config.SessionType == SessionStateful { + reqGet.Header.Set("X-sap-adt-sessiontype", "stateful") + } + getResp, err := t.httpClient.Do(reqGet) + if err != nil { + return fmt.Errorf("executing GET fallback: %w", err) + } + _, _ = io.Copy(io.Discard, getResp.Body) + getResp.Body.Close() + resp = getResp + } // Note: HEAD may return 400 but still provides CSRF token in headers - // But 401/403 indicates auth failure and won't have a valid token + // But 401 indicates auth failure and won't have a valid token token := resp.Header.Get("X-CSRF-Token") if token == "" || token == "Required" { From 59b401b2544e99a3f0e4024c56233e685cf7b887 Mon Sep 17 00:00:00 2001 From: frd1201 Date: Fri, 19 Jun 2026 19:13:14 +0200 Subject: [PATCH 2/2] fix(adt): skip CSRF GET fallback on 401/403 auth failures (PR #120 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. --- pkg/adt/http.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/adt/http.go b/pkg/adt/http.go index 5d88fdad..a7dd7394 100755 --- a/pkg/adt/http.go +++ b/pkg/adt/http.go @@ -318,9 +318,16 @@ func (t *Transport) fetchCSRFToken(ctx context.Context) error { // Use the HEAD response by default; fall back to GET when HEAD returns no usable token. // Some SAP systems (on-premise, S/4HANA cloud) reject HEAD or return no token — // CL_ADT_WB_RES_APP may not implement HEAD. GET is what Eclipse ADT uses. + // + // But skip the GET fallback on 401/403: there the token is absent because the + // credentials/authorizations are invalid, not because HEAD is unsupported — a GET + // retry can't help and only wastes a round-trip (and would swallow a mock slot in + // the re-auth-failure path). resp := headResp headToken := headResp.Header.Get("X-CSRF-Token") - if headToken == "" || headToken == "Required" { + headIsAuthFailure := headResp.StatusCode == http.StatusUnauthorized || + headResp.StatusCode == http.StatusForbidden + if (headToken == "" || headToken == "Required") && !headIsAuthFailure { reqGet, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) if err != nil { return fmt.Errorf("creating GET fallback request: %w", err)