From 2fb174bbe3e8199a6af0b68e98e28b344cce7010 Mon Sep 17 00:00:00 2001 From: HKX BDDEV001 Date: Thu, 6 Aug 2026 18:18:10 +0900 Subject: [PATCH] Enforce the declared SAP target on the RFC transport d7942b5 gave the GUI transport a target guard: every attach stamps GUI_TARGET: and AssertSapGuiTarget hard-refuses (exit 2) when SAPDEV_EXPECT_SYSTEM / SAPDEV_EXPECT_CLIENT are set and mismatch; 01efb00 wired the expectation into every write block. The RFC transport had no equivalent: Connect-SapRfc silently connected to whatever profile its resolution chain (pin -> GUI-active -> default -> sole-profile) landed on, and nothing in its output said which system that was. Live incident 2026-08-06 (second cross-system contamination, RFC transport this time): the m365-copilot adaptive driver spawned a headless Claude session; the child's CLAUDE_CODE_SESSION_ID made its skills resolve a fresh pinless AI session, Connect-SapRfc fell through to the saved DEFAULT profile (S4G = S4H/400), and the session faithfully read Z_EXCEPTION_1 from S4H/400 while the driver's pin, syntax check and verdict all pointed at S4D/100. The staged "fix" was the wrong system's source and still passed the S4D syntax gate (it compiles on both). The m365 side now hands the child the driver's session id via SAPDEV_AI_SESSION_ID (which Get-SapAiSessionId already honors), fixing the resolution -- but nothing ENFORCED the expectation on this transport, so a future resolution drift would again be silent. Fixes: - sap_rfc_lib.ps1: Connect-SapRfc mirrors AssertSapGuiTarget. When the expectation is exported and the resolved target mismatches, it refuses hard (ERROR naming both systems, "continuing would read/write TWO DIFFERENT SAP SYSTEMS in one run", + $null return) -- BEFORE logon when the identity is known pre-connect, so the wrong system is never even logged onto with the profile's saved password. Every successful connect stamps RFC_TARGET: system= client= user= endpoint=<..> via= (via = pin | gui-active | default | single-profile | explicit-params), the sibling of GUI_TARGET:, so transcripts carry provenance for both transports. Unset expectation = legacy behaviour, still stamped. - Identity comparison uses the resolved profile's system_name/client, never RfcDestination.SystemID -- that is the configured R3NAME and is blank on a direct -Server/-Sysnr connection (the d7942b5 trap). A caller-supplied endpoint is attributed by exact endpoint match against the saved connection store (so the post-activate verifiers, which resolve the pinned profile themselves and pass -Server explicitly, stay checkable before logon), then by the live logon identity in RfcDestination.SystemAttributes (best-effort, the same read sap_rfc_read_source.ps1 ships); expectation set + no identity readable = refusal, mirroring the GUI side's unverified-target rule. Deliberate cross-system legs (/sap-compare, /sap-transport-sequencer, /sap-cc-* source reads) set no expectation and are unaffected; under an inherited expectation they must clear the pair first (Set-SapGuiTargetExpectation -Clear). - sap_connection_lib.ps1: Get-SapCurrentConnectionProfile gains an optional [ref] -ResolvedVia out-param reporting which resolution step won -- the stamp's via= is exactly what the incident forensics were missing (a child resolving via=default while the run believed it was pinned). Additive; all existing callers use named parameters. Set-SapGuiTargetExpectation docs + console lines now state that BOTH transports enforce the pair (children inherit the environment, so one export covers every RFC call in the same process tree). Verified offline (NCo stubbed, no logon possible): pre-logon refusal on system and on client mismatch, via=gui-active provenance through the [ref], endpoint-match attribution refusing an explicit -Server call under a bogus expectation, matching expectations passing, and the no-expectation legacy path untouched. Verified live (32-bit PS, read-only connect+ping on S4D/100): RFC_TARGET stamps system=S4D client=100 endpoint=sap1.vicp.cc:70 via=gui-active, SystemAttributes.SystemID reads 'S4D' on a direct connection, and the function still returns a clean RfcDestination (the stamp is Write-Host, not pipeline output). Full log: temp/testReport/ rfc_target_guard_20260806.md (local). npm run validate green. Docs: gotcha 5 in parallel_safe_session_attach.md grows the RFC side; CLAUDE.md rows for sap_rfc_lib.ps1 / sap_attach_lib.vbs updated; the attach-lib header cross-references the RFC mirror. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 4 +- contributing/parallel_safe_session_attach.md | 4 + .../shared/scripts/sap_attach_lib.vbs | 6 + .../shared/scripts/sap_connection_lib.ps1 | 49 +++++- .../shared/scripts/sap_rfc_lib.ps1 | 152 +++++++++++++++++- 5 files changed, 202 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2719049..fc72a1d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -323,7 +323,7 @@ was deleted the same day. | `shared/tables/required_authorizations.tsv` | sap-doctor (auth probe), docs/security.md (§1 mirror) | Per-capability required SAP authorizations (`capability · auth_object · field · values`). Rows sharing `(capability, auth_object)` form one AUTHORITY-CHECK group. Read by `sap-doctor/references/sap_doctor_authz_probe.ps1` (calls `SUSR_USER_AUTH_FOR_OBJ_GET` for the logged-in user). The comprehensive per-skill probe set — a large superset of the coarse 12-row core-developer summary in `docs/security.md §1` (which stays a human overview; this TSV carries the per-capability groups across all four plugins). | | `shared/tables/perf_antipattern_map.tsv` | sap-trace | Maps a trace-detected performance signal to the matching ABAP code-quality rule + fix template. Read by `sap-trace/references/sap_trace.ps1`. | | `shared/tables/sap_release_markers.tsv` | sap-login (via `sap_rfc_system_info.ps1`) | Release-marker lookup — maps (software component, release range) → the canonical `server_release_marker` stored on the connection profile; the input `sap_select_vbs_variant.ps1` scores VBS variants against. | -| `shared/scripts/sap_rfc_lib.ps1` | **All RFC-using PowerShell scripts (shared library)** | Reusable NCo 3.1 connect/disconnect helpers (`Connect-SapRfc`, `Disconnect-SapRfc`, `Add-RfcField`, `Add-RfcOption`). Dot-sourced via `%%RFC_LIB_PS1%%` token to keep the GAC discovery + `RfcConfigParameters` boilerplate in one place. | +| `shared/scripts/sap_rfc_lib.ps1` | **All RFC-using PowerShell scripts (shared library)** | Reusable NCo 3.1 connect/disconnect helpers (`Connect-SapRfc`, `Disconnect-SapRfc`, `Add-RfcField`, `Add-RfcOption`). Dot-sourced via `%%RFC_LIB_PS1%%` token to keep the GAC discovery + `RfcConfigParameters` boilerplate in one place. **Target stamp + assertion (2026-08-06, RFC side)**: every successful connect emits `RFC_TARGET: system= client= user= endpoint=<...> via=` — the sibling of the attach lib's `GUI_TARGET:` line — and when `SAPDEV_EXPECT_SYSTEM`/`_CLIENT` are exported (`Set-SapGuiTargetExpectation`; children inherit them) a mismatching resolution is a **hard refusal** (`ERROR` + `$null`, before logon on the profile path) instead of a silent wrong-system connect — closing the 2026-08-06 headless-child drift where a pinless session fell through to the saved DEFAULT profile. Identity compares the resolved profile's `system_name`/`client`, never `RfcDestination.SystemID` (configured R3NAME — blank on direct `-Server`/`-Sysnr` connections); caller-supplied endpoints are attributed via store endpoint-match, then live `SystemAttributes` best-effort. Deliberate cross-system legs (`/sap-compare`, `/sap-transport-sequencer`, `/sap-cc-*`) set no expectation and are unaffected. | | `shared/scripts/sap_settings_lib.ps1` | **All PowerShell skill wrappers that need a userConfig value (mandatory — Rule 7)** | Settings reader/writer. Dot-source via `%%SETTINGS_LIB_PS1%%`. Functions: `Get-SapSettings` (returns merged object), `Get-SapSettingValue '' ''` (resolved string), `Set-SapUserSetting '' ''` (writes to `{work_dir}\runtime\userconfig.json`). Reads merge per-key on the `value` field: env `SAPDEV_AI_WORK_DIR` (work_dir only) > `settings.local.json` > `userconfig.json` > `settings.json`. All skill writes go to `userconfig.json` — never to `settings.json` or the hand-edited `settings.local.json`. | | `shared/scripts/sap_tadir_delete.ps1` | sap-se21 (Step 8a), sap-dev-clean (Step 5) | **TADIR orphan cleanup ("P2" fix)** — deletes an orphaned object-directory row (definition gone, `TADIR` row survives, blocks the package delete) via the dev-init wrapper FM → `TR_TADIR_INTERFACE` (the SAP write API for TADIR; **not** remote-enabled, so it is reached through `Z_GENERIC_RFC_WRAPPER_TBL` as an asXML-serialized dynamic call — no raw SQL on TADIR). Forces `WI_TEST_MODUS=' '` (FM default is `'X'`=dry-run) + `WI_DELETE_TADIR_ENTRY='X'`. **Safety-guarded**: deletes a row ONLY when the object's definition is verifiably gone (DOMA→DD01L, DTEL→DD04L, TABL→DD02L, TTYP→DD40L, VIEW→DD25L, SHLP→DD30L, FUNC→TFDIR, FUGR→TLIBG, PROG/REPS→TRDIR; unmapped→`REFUSED_UNMAPPED`), `REFUSED_DEF_EXISTS` for a live object — so it can never orphan a live object. Authoritative success = a post-delete RFC re-read of TADIR returning zero rows (NOT the wrapper's echo). Args: `-Object/-ObjName` or `-Entries "OBJECT:NAME[,PGMID:OBJECT:NAME]"`, `-Force` (skip the def-gone guard), `-TestOnly` (classify only). 32-bit PS. Stdout: `TADIR: …` + `STATUS: OK deleted= would= gone= refused= failed=`; exit 0/1/2. **Circular-teardown caveat**: cleaning the dev-init package's OWN orphans fails (the wrapper FM was deleted with it) — redeploy via `/sap-dev-init` or clean manually (SE03 / `RSWBO052`). | | `shared/scripts/sap_rfc_connect.ps1` | sap-login | Standalone RFC connection probe (NCo 3.1) — thin wrapper around `sap_rfc_lib.ps1` | @@ -347,7 +347,7 @@ was deleted the same day. | `shared/scripts/sap_log_lib.vbs` | **All VBScript skill scripts (optional)** | Structured logger. Include via `ExecuteGlobal FSO.OpenTextFile("%%LOG_LIB_VBS%%",1).ReadAll()`. Functions: `LogStart(skill, paramsArray)`, `LogStep(runId, level, step, msg)`, `LogEnd(runId, status, exitCode, errorMsg)`. Same JSONL/TSV/TEXT formats and redaction as the PS lib. Writes UTF-8 (no BOM) via ADODB.Stream so files concatenate cleanly with PS-emitted lines. | | `shared/scripts/sap_session_lock.vbs` | **All GUI-scripting VBS reference scripts that perform multi-step writes (mandatory per Rule 7)** | Session-lock helpers. Include via `ExecuteGlobal FSO.OpenTextFile("%%SESSION_LOCK_VBS%%",1).ReadAll()`. Functions: `TryLockSession(sess)` → returns Boolean (False if API unavailable on this SAP GUI build); `ReleaseSession(sess, wasLocked)` — idempotent unlock that ALSO sweeps up to 5 chained orphan modal popups via `sendVKey 12` (F12 / Cancel) before unlocking, so the user never gets a frozen popup on session handover. Wrap source-paste / save / activate / popup-driving critical sections to block in-session focus stealing. Pair with the existing AppActivate-loop guards for SendKeys-based pastes (defence in depth: AppActivate blocks external focus stealing, LockSessionUI blocks internal, the pre-unlock sweep covers leftover modals). | | `shared/scripts/sap_delete_popups.vbs` | **The delete VBS of sap-se37 / sap-se11 / sap-se24 / sap-se38 / sap-se21** | Shared post-delete popup walker. Included by deriving its path from the already-substituted `%%ATTACH_LIB_VBS%%` token (same dir, so no extra generator token): `sDpDir = oDpFso.GetParentFolderName("%%ATTACH_LIB_VBS%%") : ExecuteGlobal oDpFso.OpenTextFile(oDpFso.BuildPath(sDpDir, "sap_delete_popups.vbs"),1).ReadAll()`. Exposes `Function WalkDeletePopups(oSession, objdirPkg, objdirLang, sapTr)` → walks the active window (cap 10), dispatching each modal by DDIC control id ONLY (locale-independent): SAPLSETX language (`ctxtRSETX-MASTERLANG`/`btnPUSH1`), KO007 "Create Object Directory Entry" (ECC6 — fill empty package from `objdirPkg` + 1-char `objdirLang`, else accept pre-filled, else Local Object `btn[7]`), TR prompt (`ctxtKO008-TRKORR`; returns `"ABORT_EMPTY_TR"` when `sapTr` is empty so the caller releases its lock + `WScript.Quit 1`), and a confirm cascade (`btnSPOP-OPTION1` / `btnBUTTON_1` / `tbar[0]/btn[0]` / Enter). Each branch is gated by its control id, so the union is a strict superset of every per-skill loop it replaced and cannot misfire on a screen lacking that control. Pure function library (receives an already-attached `oSession`; does NOT bind the Scripting engine / declare `SESSION_PATH` / include the attach lib / call `AttachSapSession`), so — like `sap_session_lock.vbs` — it is not a "driving" VBS and lives in `shared/scripts/`, outside the `skills/*/references/` scan scope of `scripts/check-consistency.mjs` (no baseline required). se19 (classic + new) and cmod keep their own popup handling (divergent `For pass` / sequential structure + lenient-TR semantics). | -| `shared/scripts/sap_attach_lib.vbs` | **All GUI-scripting VBS reference scripts that drive SAP GUI** — mandatory for the Tier 3 (parallel-safe session attach) contract. | Shared session-attach primitive — **multi-connection aware (Phase 3.5)** + **pin-file-free (Phase 4.2)**. Include via `ExecuteGlobal FSO.OpenTextFile("%%ATTACH_LIB_VBS%%",1).ReadAll()`. Exposes `Function AttachSapSession(sHint)` which resolves the target session in this order: (1) `sHint` (typically the `%%SESSION_PATH%%` token from the calling wrapper); (2) `SAPDEV_SESSION_PATH` env var — set by the SKILL.md wrapper to `Get-SapCurrentSessionPath`'s return; (3) sole-connection + sole-session safe default; (4) **refuse loud** with `ERROR: N SAP connections attached; cannot pick one safely. Run /sap-login to pin a connection, or pass --session ...`. **Strategies 1 and 2 also work cross-connection** — they take full `/app/con[N]/ses[M]` paths and never silently retarget. Strategies 3 and 4 keep single-connection callers simple while multi-connection callers get safe refusal instead of silent miss-targeting. The convention: each migrated VBS declares `Const SESSION_PATH = "%%SESSION_PATH%%"`, includes this lib, and calls `Set oSession = AttachSapSession(SESSION_PATH)`. Calling skill wrappers (PowerShell) substitute `%%SESSION_PATH%%` with the parsed `--session` argument (or empty), `%%ATTACH_LIB_VBS%%` with the absolute path to this file, AND set `$env:SAPDEV_SESSION_PATH = Get-SapCurrentSessionPath -WorkTemp '{WORK_TEMP}'` (from `sap_connection_lib.ps1`) so the AI session's pin propagates. Unsubstituted-token sentinel via a `Chr(37)`-built runtime string so global wrapper substitution cannot corrupt the comparison. Pairs with the broker: the broker decides which session belongs to this AI session; the helper lets every VBS attach to that decision safely. **Target stamp + assertion (2026-08-06)**: every success path now echoes `GUI_TARGET: system= client= user= path=<...> via=` (read from `GuiSession.Info` — IDs, not localised text), and when the wrapper exports `SAPDEV_EXPECT_SYSTEM` / `SAPDEV_EXPECT_CLIENT` a mismatch is a **hard refusal** (exit 2) instead of a silent retarget. Set them via `Set-SapGuiTargetExpectation` (`sap_connection_lib.ps1`), which resolves the *same* profile `Connect-SapRfc` picks — so the GUI leg and the RFC leg of a skill can no longer land on different SAP systems. **Both must be exported in the same process that launches `cscript`** (env vars die with the generator block). Unset = legacy behaviour, still stamped. | +| `shared/scripts/sap_attach_lib.vbs` | **All GUI-scripting VBS reference scripts that drive SAP GUI** — mandatory for the Tier 3 (parallel-safe session attach) contract. | Shared session-attach primitive — **multi-connection aware (Phase 3.5)** + **pin-file-free (Phase 4.2)**. Include via `ExecuteGlobal FSO.OpenTextFile("%%ATTACH_LIB_VBS%%",1).ReadAll()`. Exposes `Function AttachSapSession(sHint)` which resolves the target session in this order: (1) `sHint` (typically the `%%SESSION_PATH%%` token from the calling wrapper); (2) `SAPDEV_SESSION_PATH` env var — set by the SKILL.md wrapper to `Get-SapCurrentSessionPath`'s return; (3) sole-connection + sole-session safe default; (4) **refuse loud** with `ERROR: N SAP connections attached; cannot pick one safely. Run /sap-login to pin a connection, or pass --session ...`. **Strategies 1 and 2 also work cross-connection** — they take full `/app/con[N]/ses[M]` paths and never silently retarget. Strategies 3 and 4 keep single-connection callers simple while multi-connection callers get safe refusal instead of silent miss-targeting. The convention: each migrated VBS declares `Const SESSION_PATH = "%%SESSION_PATH%%"`, includes this lib, and calls `Set oSession = AttachSapSession(SESSION_PATH)`. Calling skill wrappers (PowerShell) substitute `%%SESSION_PATH%%` with the parsed `--session` argument (or empty), `%%ATTACH_LIB_VBS%%` with the absolute path to this file, AND set `$env:SAPDEV_SESSION_PATH = Get-SapCurrentSessionPath -WorkTemp '{WORK_TEMP}'` (from `sap_connection_lib.ps1`) so the AI session's pin propagates. Unsubstituted-token sentinel via a `Chr(37)`-built runtime string so global wrapper substitution cannot corrupt the comparison. Pairs with the broker: the broker decides which session belongs to this AI session; the helper lets every VBS attach to that decision safely. **Target stamp + assertion (2026-08-06)**: every success path now echoes `GUI_TARGET: system= client= user= path=<...> via=` (read from `GuiSession.Info` — IDs, not localised text), and when the wrapper exports `SAPDEV_EXPECT_SYSTEM` / `SAPDEV_EXPECT_CLIENT` a mismatch is a **hard refusal** (exit 2) instead of a silent retarget. Set them via `Set-SapGuiTargetExpectation` (`sap_connection_lib.ps1`), which resolves the *same* profile `Connect-SapRfc` picks — so the GUI leg and the RFC leg of a skill can no longer land on different SAP systems (since 2026-08-06 `Connect-SapRfc` enforces the same pair itself and stamps `RFC_TARGET:` — see the `sap_rfc_lib.ps1` row). **Both must be exported in the same process that launches `cscript`** (env vars die with the generator block). Unset = legacy behaviour, still stamped. | | `shared/scripts/sap_session_broker.ps1` | **All GUI-scripting skills that may run in parallel** (today: `/sap-gui-skill-scaffold` parallel path + the 4 Phase-3.1 migrated read-only skills; will become broadly mandatory after Tier 3 migration). Full contract: `shared/rules/sap_session_broker.md`. | SAP GUI Session Broker — **multi-connection aware (v2 schema, Phase 3.5)**. PowerShell, ~600 LOC. Single-binary CLI with five actions — `acquire` / `release` / `discover` / `gc` / `list` — driven by `-Action -WorkTemp ` + per-action args. State lives in `{WORK_TEMP}\session_registry.json` (UTF-8 no BOM, nested `connections[]` shape); cross-process concurrency serialized by a named Windows mutex (`SapDevSessionBroker_v2`) acquired through `System.Threading.Mutex` with a 10s timeout for crash recovery. **Connection isolation**: a claim resolved against connection N never returns a session of connection M. Reactive cleanup + identity-reconciliation sweep runs inside every acquire/release/discover/gc across ALL connections: it mirrors live SAP identity onto each block (live is source of truth) then drops entries on these failure modes — session closed, owner PID dead, TTL expired, relogin, entire connection closed. A reused `/app/con[N]` slot now hosting a DIFFERENT system is detected by the `(system,client,user)` tuple — NOT `SystemSessionId`, which on the tested kernels is per-workstation, not per-logon, and stays identical across an A→B swap on one slot (the 2026-06-07 stale-identity bug); on a tuple change the block is reset to the live identity and its stale `connection_id` cleared (re-bound on next finalize). Idempotent on re-acquire by `task_id`. Connection-targeting acquire args (Phase 4.1+): broker auto-resolves `-AiSessionId` via parent-PID walk and reads its `ai_sessions[].connection_id` pin. Explicit `-SessionPath` / `-ConnectionPath` / `-SystemName -Client -User` still override; resolution falls through to sole-connection auto-default or DENIED. Spawns on demand on the target connection via `/oSESSION_MANAGER` (the only OK-code mechanism verified on S/4HANA 1909 kernel 754; `CreateSession` and bare `/o` no-op). Stdout last line: `ACQUIRED: path=

sessionNumber= connection= reused=` / `RELEASED: path=

connection=` / `NOT_FOUND` / `DENIED: ` (exit 1) / `ERROR: ` (exit 2). Auto-rebuilds a v1 registry on first call after upgrade with a `WARN: v1 registry detected` line. Shells out to `sap_session_broker_com.vbs` for every SAP-side operation because PowerShell 7+/.NET 5+ cannot bind the SAP GUI Scripting Engine directly (`Marshal::GetActiveObject` removed in .NET 5+; even 32-bit Windows PowerShell 5.1 fails to resolve the SAPGUI ProgID through the ROT). | | `shared/scripts/sap_session_broker_com.vbs` | **Internal helper for `sap_session_broker.ps1`** — not intended for direct calls by other skills. | SAP COM helper for the broker. VBScript run via 32-bit `cscript`. Single argv command + JSON-on-stdout protocol. **Multi-connection aware** (Phase 3.5): `INFO` returns ALL attached SAP connections (each with `connection_path` / `description` / `system_name` / `client` / `user` / `language` / `logon_id` + a `sessions[]` array); `SPAWN ` spawns on a SPECIFIC connection (drives `/n` + `/oSESSION_MANAGER` on that connection's anchor, returns the newcomer's path + `SessionNumber`); `RESET ` drives `/n` on a specific session (used by `release` to return to SAP Easy Access); `PROBE ` does a single-session `findById` + `Info` read (used by acquire's pre-allocation Easy-Access verification). Exit codes: 0 success, 1 usage error, 2 SAP-unreachable, 3 command-level failure (details in JSON `error` field). JSON output is one line per invocation — broker parses with `ConvertFrom-Json`. | | `shared/scripts/sap_activation_log.vbs` | **SE11 / DDIC GUI-scripting VBS only — do NOT include in SE38/SE37/SE24/SE91 (no equivalent menu in those transactions)** | Activation-log capture. Include via `ExecuteGlobal FSO.OpenTextFile("%%ACTIVATION_LOG_VBS%%",1).ReadAll()`. Functions: `CaptureActivationLog(oSess, sObjectName, sOutDir, kEnter, kBack)` → returns "" on failure or absolute path of saved log file on success; `ExtractTopActivationError(sLogPath)` → returns the top error line from the log (empty string if none). After Activate, when `sbar.MessageType = "E"` or `"A"`, call `CaptureActivationLog` then echo `ACTIVATION_LOG: ` and `ACTIVATION_ERROR: ` so the operator sees the specific failure instead of the generic "refer to log" SAP popup. Walks Utilities > Activation Log → Log > Save Local File via menu indices captured from a SAP GUI recording of the SE11 activation-log walk (`Record_SE11_ActivateErrorLog_01.vbs`, S/4HANA 1909). Re-record on releases that move the menus. The `Utilities > Activation Log` menu is a DDIC-worklist concept and exists ONLY in SE11; SE38/SE37/SE24/SE91 surface activation errors inline in the source-code editor + status bar (read via `wnd[0]/sbar.Text` — already done in those skills). | diff --git a/contributing/parallel_safe_session_attach.md b/contributing/parallel_safe_session_attach.md index b7c1fd5..9b13686 100644 --- a/contributing/parallel_safe_session_attach.md +++ b/contributing/parallel_safe_session_attach.md @@ -183,6 +183,10 @@ If you write a new bootstrap-style file that legitimately needs custom attach, a ``` `AssertSapGuiTarget` in the attach lib then refuses (exit 2) any session that is not that system, instead of retargeting silently. Every attach — expectation or not — now also emits `GUI_TARGET: system=… client=… user=… path=… via=…`, so a skill's output always records which SAP system it actually drove. **Surface that line in the skill's report; never state a system you did not read off it.** This matters most on the **write** paths — a create/update/delete aimed at the wrong system is not a bad read, it is a bad write — so it is CI-enforced as a hard error for every write-capable skill since 2026-08-06 (gate 14 below). Read-only GUI skills are not gated, but declaring the expectation there is still recommended: the incident that motivated all of this *was* a read. + **The RFC transport enforces the same declaration** (added 2026-08-06 after the *second* cross-system incident, this one on the RFC leg: a headless child session's fresh session id resolved a pinless AI session, `Connect-SapRfc` fell through to the saved DEFAULT profile and read another system's `Z_EXCEPTION_1` while the driver's pin, syntax gate and verdict all pointed at the intended system — the staged "fix" was the wrong system's source and still compiled). After resolving its profile, `Connect-SapRfc` refuses hard (`ERROR` + `$null` return, **before logon** when the identity is known pre-connect) when `SAPDEV_EXPECT_SYSTEM` / `SAPDEV_EXPECT_CLIENT` mismatch the resolution, and stamps `RFC_TARGET: system=… client=… user=… endpoint=… via=` on every successful connect — surface it exactly like `GUI_TARGET:`. Because child processes inherit the environment, one `Set-SapGuiTargetExpectation` covers every RFC call in the same process tree, including 32-bit PowerShell children. Two sharp edges: + - **Identity comes from the resolved profile's `system_name`/`client`, never `RfcDestination.SystemID`** — that property is the configured R3NAME and is *blank* on a direct `-Server`/`-Sysnr` connection. For caller-supplied endpoints the guard attributes the SID by exact endpoint match against the saved connection store, then falls back to the live logon identity in `RfcDestination.SystemAttributes` (best-effort); expectation set + no identity readable = refusal, mirroring the GUI side's unverified-target refusal. + - **Deliberate cross-system connects** (the second legs of `/sap-compare`, `/sap-transport-sequencer`, `/sap-cc-*` source reads) set no expectation today and are unaffected. If such a block ever runs under an inherited expectation, it must clear the pair in its own process first (`Set-SapGuiTargetExpectation -Clear`) before opening the other system's destination. + --- ## Verifying compliance diff --git a/plugins/sap-dev-core/shared/scripts/sap_attach_lib.vbs b/plugins/sap-dev-core/shared/scripts/sap_attach_lib.vbs index 3c6ad1e..6f4f62b 100644 --- a/plugins/sap-dev-core/shared/scripts/sap_attach_lib.vbs +++ b/plugins/sap-dev-core/shared/scripts/sap_attach_lib.vbs @@ -70,6 +70,12 @@ ' Unset = today's behaviour (identity still echoed), so this is additive ' for every VBS that has not been wired up yet. ' +' Since 2026-08-06 (second incident, RFC transport) the RFC leg enforces +' the SAME pair: Connect-SapRfc (sap_rfc_lib.ps1) refuses a resolution +' that mismatches the expectation and stamps "RFC_TARGET: ..." on every +' successful connect, the sibling of the GUI_TARGET line below -- so both +' transports answer to one declared target. +' ' Why a helper rather than every skill rolling its own ' ---------------------------------------------------- ' The legacy idiom: diff --git a/plugins/sap-dev-core/shared/scripts/sap_connection_lib.ps1 b/plugins/sap-dev-core/shared/scripts/sap_connection_lib.ps1 index 58d9ec8..f8013f6 100644 --- a/plugins/sap-dev-core/shared/scripts/sap_connection_lib.ps1 +++ b/plugins/sap-dev-core/shared/scripts/sap_connection_lib.ps1 @@ -1739,9 +1739,11 @@ function Get-SapCurrentSessionPath { function Set-SapGuiTargetExpectation { <# .SYNOPSIS - Declare which SAP system the GUI leg of this skill is allowed to drive, - by exporting SAPDEV_EXPECT_SYSTEM / SAPDEV_EXPECT_CLIENT for the VBS - attach helper (sap_attach_lib.vbs) to enforce. + Declare which SAP system this run is allowed to target, by exporting + SAPDEV_EXPECT_SYSTEM / SAPDEV_EXPECT_CLIENT. Enforced by BOTH + transports: the VBS attach helper (sap_attach_lib.vbs, + AssertSapGuiTarget) and the RFC connector (Connect-SapRfc in + sap_rfc_lib.ps1). .DESCRIPTION A skill that reads over BOTH transports resolves its SAP target twice: Connect-SapRfc walks pin -> GUI-active -> default -> sole-profile, while @@ -1766,8 +1768,23 @@ function Set-SapGuiTargetExpectation { the pre-existing behaviour, whereas a wrong expectation would block legitimate work. + Second incident, same day (RFC transport): a headless child session's + fresh CLAUDE_CODE_SESSION_ID resolved a PINLESS AI session, so + Connect-SapRfc fell through to the saved DEFAULT profile and read the + wrong system's source while the driver's pin, syntax check and verdict + all pointed elsewhere. Connect-SapRfc therefore now enforces the same + pair: a resolution that mismatches the exported expectation is a hard + refusal (before logon on the profile path), and every successful + connect stamps `RFC_TARGET: ...` alongside the attach lib's + `GUI_TARGET: ...`. Because child processes inherit the environment, + exporting the expectation here covers every RFC call in the same + process tree. + Call this in the skill wrapper right next to the existing `$env:SAPDEV_SESSION_PATH = Get-SapCurrentSessionPath ...` line. + A DELIBERATE cross-system connect (e.g. the second leg of + /sap-compare or /sap-transport-sequencer) must clear the pair first + (`-Clear`, or `$env:SAPDEV_EXPECT_SYSTEM = $null`) in its own process. .PARAMETER Clear Remove both env vars instead of setting them (disables enforcement for the rest of this process). @@ -1800,13 +1817,13 @@ function Set-SapGuiTargetExpectation { # safety gate already treats as fail-closed). Do NOT invent one. $env:SAPDEV_EXPECT_SYSTEM = $null $env:SAPDEV_EXPECT_CLIENT = $null - [Console]::Error.WriteLine("WARN: no SAP connection profile resolved; the GUI leg will run UNVERIFIED (it may attach to a different system than the RFC leg). Run /sap-login to pin a connection.") + [Console]::Error.WriteLine("WARN: no SAP connection profile resolved; the GUI and RFC legs will run UNVERIFIED (they may target different SAP systems). Run /sap-login to pin a connection.") return $null } $env:SAPDEV_EXPECT_SYSTEM = $sid $env:SAPDEV_EXPECT_CLIENT = $cli - [Console]::Error.WriteLine("INFO: GUI target expectation = $sid/$cli (matches the RFC leg); a GUI session on any other system will be refused.") + [Console]::Error.WriteLine("INFO: SAP target expectation = $sid/$cli; both transports (AssertSapGuiTarget and Connect-SapRfc) will refuse any other system in this process tree.") return $prof } @@ -1993,12 +2010,20 @@ function Get-SapCurrentConnectionProfile { Opt-in because it may shell a 32-bit cscript COM read -- Connect-SapRfc opts in (the write-hazard path); cheap callers (banner / release marker) leave it off and keep today's pin-or-default behaviour. + .PARAMETER ResolvedVia + Optional [ref] out-parameter. On a non-null return, receives which + resolution step won: 'pin' | 'gui-active' | 'default' | + 'single-profile'. Untouched when nothing resolves. Connect-SapRfc + passes this so its RFC_TARGET stamp can name the resolution source + (the 2026-08-06 incident forensics hinged on exactly this: a headless + child resolving via 'default' when the run believed it was pinned). #> param( [string]$WorkTemp = '', [string]$RuntimeDir = '', [switch]$StrictMode, - [switch]$PreferGuiActive + [switch]$PreferGuiActive, + [ref]$ResolvedVia = $null ) if ([string]::IsNullOrWhiteSpace($RuntimeDir)) { @@ -2018,7 +2043,10 @@ function Get-SapCurrentConnectionProfile { if ($pinnedConnId) { $p = Find-SapConnectionById -Id $pinnedConnId - if ($p) { return $p } + if ($p) { + if ($ResolvedVia) { $ResolvedVia.Value = 'pin' } + return $p + } } # Step 2 -- GUI-active preference (opt-in; only reached with no usable pin). @@ -2035,6 +2063,7 @@ function Get-SapCurrentConnectionProfile { # target (GUI-active != default) -- that is the hazard case. [Console]::Error.WriteLine("INFO: RFC target = GUI-active connection $($gui.identity.system_name)/$($gui.identity.client)/$($gui.identity.user) (via $($gui.identity.source)), overriding the saved default -- no AI-session pin is set. Run '/sap-login --switch $($gui.identity.system_name)' to pin it and silence this.") } + if ($ResolvedVia) { $ResolvedVia.Value = 'gui-active' } return $gui.profile } if ($gui -and -not $gui.matched) { @@ -2046,7 +2075,10 @@ function Get-SapCurrentConnectionProfile { } $defp = Get-SapDefaultConnection - if ($defp) { return $defp } + if ($defp) { + if ($ResolvedVia) { $ResolvedVia.Value = 'default' } + return $defp + } if ($StrictMode) { return $null } @@ -2068,6 +2100,7 @@ function Get-SapCurrentConnectionProfile { # Stderr so the line surfaces above the skill's normal output # without contaminating stdout that downstream JSON parsers consume. [Console]::Error.WriteLine("INFO: auto-bootstrap pinned single saved profile id=$($only.id) description='$($only.description)' (no default; password present)") + if ($ResolvedVia) { $ResolvedVia.Value = 'single-profile' } return $only } } diff --git a/plugins/sap-dev-core/shared/scripts/sap_rfc_lib.ps1 b/plugins/sap-dev-core/shared/scripts/sap_rfc_lib.ps1 index 3a55d7f..8c35388 100644 --- a/plugins/sap-dev-core/shared/scripts/sap_rfc_lib.ps1 +++ b/plugins/sap-dev-core/shared/scripts/sap_rfc_lib.ps1 @@ -53,6 +53,36 @@ # free. Callers that still use `$dest.Repository.CreateFunction("RFC_READ_TABLE")` # directly MUST invoke `Assert-RfcReadTableAllowed -QueryTable ` after # the first `SetValue("QUERY_TABLE", ...)`. +# +# CROSS-SYSTEM TARGET GUARD (SAPDEV_EXPECT_SYSTEM / SAPDEV_EXPECT_CLIENT) +# ----------------------------------------------------------------------- +# Connect-SapRfc mirrors AssertSapGuiTarget (sap_attach_lib.vbs) on the RFC +# transport. Every successful connect stamps one line: +# +# RFC_TARGET: system= client= user= endpoint=<...> via= +# +# (via = pin | gui-active | default | single-profile | explicit-params -- +# which step of the resolution chain chose the target), matching the +# GUI_TARGET convention so transcripts carry provenance for BOTH transports. +# When the calling wrapper exported SAPDEV_EXPECT_SYSTEM / SAPDEV_EXPECT_CLIENT +# (Set-SapGuiTargetExpectation in sap_connection_lib.ps1 -- child processes +# inherit them), a mismatching resolution is a HARD REFUSAL (ERROR + $null, +# before logon when the identity is known pre-connect): continuing would +# read/write TWO DIFFERENT SAP SYSTEMS in one run. Unset expectation = legacy +# behaviour, still stamped. +# +# Identity comparison uses the resolved PROFILE's system_name/client. It must +# NOT use RfcDestination.SystemID -- that is the configured R3NAME and is +# EMPTY on a direct -Server/-Sysnr connection (the d7942b5 trap). For +# caller-supplied endpoints the guard attributes the SID via an exact +# endpoint match against the saved connection store, then falls back to the +# live logon identity in RfcDestination.SystemAttributes (best-effort; same +# read as _RfcDestIdentity in sap_rfc_read_source.ps1). +# +# A DELIBERATE cross-system connect (the second leg of /sap-compare, +# /sap-transport-sequencer, /sap-cc-* source reads) must clear the pair in +# its own process first (Set-SapGuiTargetExpectation -Clear) -- those skills +# set no expectation today, so they are unaffected until one is exported. # ============================================================================= # Module-scoped state so Disconnect-SapRfc can find what to remove. @@ -139,6 +169,8 @@ function Connect-SapRfc { $explicitMessageServer = -not (_Needs $MessageServer) $needAny = (_Needs $Server) -and (_Needs $MessageServer) # at least one endpoint if (-not $needAny) { $needAny = (_Needs $Client) -or (_Needs $User) -or (_Needs $Password) } + # Resolution provenance for the RFC_TARGET stamp + target guard below. + $prof = $null; $profVia = '' if ($needAny) { try { $libDir = $PSScriptRoot @@ -148,8 +180,8 @@ function Connect-SapRfc { if (Test-Path $sl) { . $sl } if (Test-Path $cl) { . $cl } } - $prof = $null - if (Get-Command Get-SapCurrentConnectionProfile -ErrorAction SilentlyContinue) { + $gcpCmd = Get-Command Get-SapCurrentConnectionProfile -ErrorAction SilentlyContinue + if ($gcpCmd) { # -PreferGuiActive: when no AI-session pin resolves, prefer the # profile matching the SAP GUI session this AI session is # actually driving over the saved DEFAULT profile. Without this, @@ -157,7 +189,13 @@ function Connect-SapRfc { # silently target the unrelated default system (wrong-system # write hazard). Explicit-param callers never reach this block; # pinned sessions resolve the pin first (GUI step is skipped). - $prof = Get-SapCurrentConnectionProfile -PreferGuiActive + if ($gcpCmd.Parameters.ContainsKey('ResolvedVia')) { + $prof = Get-SapCurrentConnectionProfile -PreferGuiActive -ResolvedVia ([ref]$profVia) + } else { + # Older sap_connection_lib.ps1 without the out-param + # (mixed plugin-cache versions): resolve without provenance. + $prof = Get-SapCurrentConnectionProfile -PreferGuiActive + } } if ($prof) { # Endpoint: prefer existing input shape (direct vs load-balanced); @@ -277,6 +315,74 @@ function Connect-SapRfc { return $null } + # ---- Cross-system target guard (RFC transport) -------------------------- + # Mirror of AssertSapGuiTarget (sap_attach_lib.vbs). See the header block + # "CROSS-SYSTEM TARGET GUARD" and gotcha 5 in + # contributing/parallel_safe_session_attach.md. Live incident 2026-08-06 + # (second cross-system contamination, RFC transport this time): a headless + # child session's fresh session id resolved a PINLESS AI session, this + # function fell through to the saved DEFAULT profile (S4H/400) and + # faithfully read the wrong system's source while the driver's pin, syntax + # check and verdict all pointed at S4D/100. Nothing on the RFC leg said + # which system it used -- the GUI leg has refused that drift since + # d7942b5; this closes the same hole here. + $expSys = "$env:SAPDEV_EXPECT_SYSTEM".Trim() + $expCli = "$env:SAPDEV_EXPECT_CLIENT".Trim() + $effClient = "$Client".Trim() + + # Attribute the target system. The resolved profile's system_name applies + # only when the ENDPOINT actually came from the profile fallback -- a + # caller that passed -Server / -MessageServer itself chose the target + # (e.g. the deliberate second legs of /sap-compare or + # /sap-transport-sequencer), and the pinned profile's system_name says + # nothing about where that endpoint points. Comparison deliberately uses + # profile identity, NOT $dest.SystemID -- that is the configured R3NAME + # and is EMPTY on a direct -Server/-Sysnr connection (the d7942b5 trap). + $endpointFromProfile = ($null -ne $prof) -and -not ($explicitServer -or $explicitMessageServer) + $rfcVia = 'explicit-params' + if ($endpointFromProfile) { $rfcVia = if ($profVia) { $profVia } else { 'profile' } } + $resolvedSid = '' + if ($endpointFromProfile) { $resolvedSid = "$($prof.system_name)".Trim() } + if (-not $resolvedSid -and $useMsg) { $resolvedSid = "$SystemID".Trim() } # R3NAME = declared SID + if (-not $resolvedSid -and ($expSys -or $expCli)) { + # Expectation declared but the caller supplied its own direct endpoint: + # attribute it via an exact endpoint match against the saved connection + # store. Covers the verify/lookup scripts that resolve the pinned + # profile themselves and pass -Server/-Sysnr explicitly -- their SID + # becomes checkable BEFORE logon. Best-effort; no match leaves the + # post-connect SystemAttributes leg below to decide. + try { + if (-not (Get-Command Read-SapConnectionStore -ErrorAction SilentlyContinue)) { + $sl2 = Join-Path $PSScriptRoot 'sap_settings_lib.ps1' + $cl2 = Join-Path $PSScriptRoot 'sap_connection_lib.ps1' + if (Test-Path $sl2) { . $sl2 } + if (Test-Path $cl2) { . $cl2 } + } + $epStore = $null + if (Get-Command Read-SapConnectionStore -ErrorAction SilentlyContinue) { $epStore = Read-SapConnectionStore } + if ($epStore -and $epStore.connections) { + $epSids = @($epStore.connections | Where-Object { + ("$($_.application_server)".Trim() -eq "$Server".Trim()) -and + ("$($_.system_number)".Trim() -eq "$Sysnr".Trim()) + } | ForEach-Object { "$($_.system_name)".Trim() } | Where-Object { $_ } | Select-Object -Unique) + if ($epSids.Count -eq 1) { $resolvedSid = $epSids[0] } + } + } catch { } + } + + if ($expSys -or $expCli) { + $tgtBad = $false + if ($expSys -and $resolvedSid -and ($resolvedSid -ne $expSys)) { $tgtBad = $true } + if ($expCli -and $effClient -and ($effClient -ne $expCli)) { $tgtBad = $true } + if ($tgtBad) { + $shownSid = if ($resolvedSid) { $resolvedSid } else { '?' } + Write-Host "ERROR: SAP RFC target mismatch. Expected $expSys/$expCli but Connect-SapRfc resolved $shownSid/$effClient (via $rfcVia)." + Write-Host " This run declared its SAP target via SAPDEV_EXPECT_SYSTEM/_CLIENT (Set-SapGuiTargetExpectation), so continuing would read/write TWO DIFFERENT SAP SYSTEMS in one run. Refusing before logon." + Write-Host " Fix: run /sap-login --switch $expSys to re-pin this AI session (headless child sessions: hand the driver's SAPDEV_AI_SESSION_ID through), or clear SAPDEV_EXPECT_SYSTEM/_CLIENT first if connecting to another system is genuinely intended (deliberate cross-system compare)." + return $null + } + } + if (-not (_Load-SapNco)) { return $null } # NCo writes a `dev_nco_rfc.log` trace file to the .NET process's current @@ -339,6 +445,46 @@ function Connect-SapRfc { $effGroupMsg = if ([string]::IsNullOrWhiteSpace($LogonGroup)) { 'PUBLIC (default)' } else { $LogonGroup } Write-Host "INFO: RFC connected to $SystemID via msrv=$MessageServer group=$effGroupMsg client=$Client (NCo 3.1, load-balanced)." } + + # ---- RFC_TARGET stamp + post-connect leg of the target guard -------- + # Best-effort live identity from the logon handshake + # (RfcDestination.SystemAttributes -- same read as _RfcDestIdentity in + # sap_rfc_read_source.ps1; degrades to blank, never throws). Fills the + # stamp for caller-supplied endpoints no saved profile matched, and + # closes the guard for that path. + $liveSid = '' + try { + $sa = $dest.SystemAttributes + if ($sa) { $liveSid = "$($sa.SystemID)".Trim() } + } catch { } + if ($resolvedSid -and $liveSid -and ($liveSid -ne $resolvedSid)) { + Write-Host "WARN: Connect-SapRfc: the resolved target claims system '$resolvedSid' but the live logon reports '$liveSid' -- stale saved profile? Re-run /sap-login to refresh it." + } + $stampSid = if ($resolvedSid) { $resolvedSid } else { $liveSid } + if (-not $stampSid) { $stampSid = '?' } + $endpointDesc = if ($useDirect) { "${Server}:${Sysnr}" } else { "/M/${MessageServer}/G/${effGroup}/S/${SystemID}" } + Write-Host "RFC_TARGET: system=$stampSid client=$effClient user=$User endpoint=$endpointDesc via=$rfcVia" + + if ($expSys -and -not $resolvedSid) { + # SID was not decidable before logon (caller-supplied endpoint, + # no store match). Enforce on the live identity; mirror + # AssertSapGuiTarget's unverified-target refusal when even the + # live read comes back blank -- an unverifiable target is exactly + # the case this guard exists for. + if (-not $liveSid) { + Write-Host "ERROR: SAP RFC target could not be identified (caller-supplied endpoint matches no saved profile and SystemAttributes.SystemID is blank) but SAPDEV_EXPECT_SYSTEM=$expSys was declared. Refusing to use an unverified connection." + try { [SAP.Middleware.Connector.RfcDestinationManager]::RemoveDestination($params) | Out-Null } catch { } + return $null + } + if ($liveSid -ne $expSys) { + Write-Host "ERROR: SAP RFC target mismatch. Expected $expSys/$expCli but the live logon landed on $liveSid/$effClient (endpoint $endpointDesc, via $rfcVia)." + Write-Host " This run declared its SAP target via SAPDEV_EXPECT_SYSTEM/_CLIENT (Set-SapGuiTargetExpectation), so continuing would read/write TWO DIFFERENT SAP SYSTEMS in one run." + Write-Host " Fix: run /sap-login --switch $expSys to re-pin this AI session (headless child sessions: hand the driver's SAPDEV_AI_SESSION_ID through), or clear SAPDEV_EXPECT_SYSTEM/_CLIENT first if connecting to another system is genuinely intended (deliberate cross-system compare)." + try { [SAP.Middleware.Connector.RfcDestinationManager]::RemoveDestination($params) | Out-Null } catch { } + return $null + } + } + $script:_SapRfc_Params = $params # Also expose at caller scope so legacy `RemoveDestination($g_rfcParams)` keeps working, # and re-publish the credential values as $g_sap* so consumers don't need their own