Fix PDS member STOR/RETR: wrong dataset name, STC-identity allocation, S130 ABEND

[mgrossmann]

Closes #61.

Fixes three interlocking bugs in the MVS store/retrieve path (src/ftpd#mvs.c), all surfaced by uploading a file into a PDS.

What was wrong

cd into a PDS, then put iefbr14.jcl:

• Bug 1 — wrong dataset name. resolve_dsn()'s PDS-member branch was guarded by !strchr(arg, '.'). A client filename like iefbr14.jcl contains a dot, so it fell through to the relative-prefix branch (CWD carries a trailing dot in PDS context), yielding IBMUSER.CNTL.IEFBR14.JCL instead of IBMUSER.CNTL(IEFBR14).
• Bug 2 — allocation under the wrong identity. check_dataset_access() and fopen() both run under the logged-in user's ACEE, but __dsalcf() (SVC 99) ran between them with no ACEE switch — so allocation executed under the STC identity (FTPD). This is why TK5 RAKF denials showed FTPD,FTPD instead of the logged-in user.
• Bug 3 — S130 ABEND. With allocation done (or deferred) under the STC, a later OPEN under the user's ACEE hit the RAKF check on a failed/half-allocated dataset and abended S130, taking the worker down.

The fix

1. resolve_dsn() member derivation (~line 163). In PDS context an unquoted argument is a member reference and a dot is a filename extension, not a qualifier separator. The branch now strips leading path + extension, uppercases, and truncates to 8 chars. Shared by RETR/STOR/DELE, so all three get correct member handling. The quoted-absolute and relative-prefix (non-PDS) branches are unchanged.
2. STOR allocation under the user ACEE (~line 1510). __dsalcf()/__dsfree() are now wrapped in racf_set_acee(sess->acee), matching the fopen() idiom, with the ACEE always restored — including on the failure path. __dsfree() runs only when allocation succeeded.
3. S130 guard. Running __dsalcf() under the user's identity makes it fail cleanly for an unauthorized user; the existing early return then prevents ever reaching fopen()/OPEN. Audited STOR/RETR/DELE — each replies and returns immediately on access denial, allocation failure, and fopen() == NULL. No failure path reaches OPEN or an I/O loop with a NULL/invalid FILE *.

Blast radius

resolve_dsn()'s in_pds branch is only reached by RETR/STOR/DELE (and MKD/RMD/RNFR/RNTO). LIST/NLST does not hit it — in PDS context it uses sess->pds_name directly and treats the argument as a member filter, so listing behavior is unchanged. The only behavioral delta is a dotted argument inside a PDS, which now correctly resolves to PDS(MEMBER).

Testing

• Host build passes clean (make modules, -Wall -Werror).
• The member-derivation logic (Fix 1) was verified in isolation on the host across the reported bug case and edge cases (dotless name, leading path, >8-char truncation, multi-dot, leading-dot rejection, empty). It is not covered by an in-repo [[test]] because resolve_dsn() is static and the file has no host-buildable test seam — #include-ing the TU would drag in the full MVS runtime (racf, __locate, __dsalcf, dynalloc). Extracting the function for one assertion would be scope creep on a fix PR, so it's left for a follow-up if a test seam is added.

On-MVS matrix to run before merge (needs live target)

cd 'IBMUSER.CNTL'          (PDS context)
put iefbr14.jcl            → 250, creates IBMUSER.CNTL(IEFBR14), RAKF shows IBMUSER not FTPD
get IEFBR14                → downloads the member
get iefbr14.jcl            → downloads IBMUSER.CNTL(IEFBR14) (extension stripped)
dir                        → listing shows member IEFBR14
delete IEFBR14             → removes the member

cd 'IBMUSER.'              (prefix mode, not a PDS)
put test.data             → creates sequential dataset IBMUSER.TEST.DATA (unchanged)

# Negative / crash-regression:
cd 'SYS1.PARMLIB'          (a PDS the user may not have ALTER on)
put foo.jcl               → 550 Access denied, NO S130 ABEND, STC stays up

Confirm on both MVS/CE (IBMUSER) and TK5 (HERC01) that the RAKF requestor/user in any denial is the logged-in user, not FTPD, and that a denial never crashes the worker.

---

Follow-up commit: sanitize quoted DSN(member) member names (FileZilla)

A second bug in the same PDS member path, found after the first fix. The first commit handled the relative form (STOR iefbr14.jcl in PDS context — CLI ftp). But FileZilla sends the member explicitly in parentheses:

STOR 'IBMUSER.CNTL(iefbr14.jcl)'

That argument takes resolve_dsn()'s quoted-absolute branch verbatim, and split_member() copied the parenthesized text with only an 8-char truncation → member physically named IEFBR14. (trailing dot), which no later DELE could match.

Fix: centralized member-name cleanup in one helper — sanitize_member() (strip extension from the first ., uppercase, truncate to FTPD_MAX_MBR_LEN) — applied at both extraction points: resolve_dsn()'s PDS branch (replacing the inline loop) and split_member() (the actual bug). Now STOR 'IBMUSER.CNTL(iefbr14.jcl)' creates member IEFBR14, matching the CLI result.

Deliberate deviation from the proposed patch: the follow-up suggested removing the !strchr(arg, '(') guard from resolve_dsn()'s PDS branch. I kept it. FileZilla's arg is quoted so it never reaches that branch; removing the guard would only change the bare DSN(member) form and make it worse (sanitize_member would derive member IBMUSER from IBMUSER.CNTL(...), silently writing to a valid-but-wrong member instead of the current clean 550). (-containing args stay routed to split_member(), which matches the prose intent.

Leftover cleanup: a member already created with a trailing dot (e.g. IEFBR14.) is unreachable via FTP after this fix (DELE input is sanitized too), so it needs manual IEHPROGM/ISPF cleanup on the target. Inherent, not a regression.

Verification: host build clean; sanitize_member() + the split_member() path verified in isolation across the follow-up matrix (IEFBR14.JCL → IEFBR14, MyProg.cbl → MYPROG, IEFBR14. → IEFBR14, idempotent, .foo/empty rejected).

Additional on-MVS checks:

quote STOR 'IBMUSER.CNTL(iefbr14.jcl)'   → member IEFBR14 (not IEFBR14.)
quote DELE 'IBMUSER.CNTL(iefbr14.jcl)'   → removes member IEFBR14
quote STOR 'IBMUSER.CNTL(MyProg.cbl)'    → member MYPROG
cd 'IBMUSER.CNTL'; put iefbr14.jcl        → member IEFBR14 (CLI regression, still works)

Tracked under #61 (follow-up comment there).

---

Follow-up commit 2: LIST path — run __listpd() under the user ACEE + authorize first (fixes S913)

The first commit fixed RETR/STOR/DELE/MKD/RMD, but the LIST path had the same identity/authorization gap. __listpd() OPENs the PDS directory with BPAM under whatever ACEE is active, and ftpd_mvs_list() never switched to sess->acee — so directory listing was authorized as the STC (FTPD), not the user. On a PDS the user is denied but FTPD is not, the unprotected OPEN escalated a RAKF denial to ABEND S913 and killed the worker (ls of SYS1.SECURE.CNTL on TK5).

Fix (PDS-member branch of ftpd_mvs_list()):

• check_dataset_access(sess, prefix, RACF_ATTR_READ) before __listpd(), returning immediately on denial (S130-style guard) so the OPEN is never reached for a PDS the user can't read.
• __listpd() wrapped in racf_set_acee(sess->acee)/restore, so the OPEN-time RACHECK evaluates against the logged-in user.

Both halves are required — the pre-check alone wouldn't stop an S913 for a PDS where the user is allowed but FTPD is denied.

Scope: __listpd() is the only dataset OPEN in the LIST path (formatters render already-fetched entries; __listds()/__locate()/__dscbdv() are catalog/VTOC ops with no OPEN). The dataset-listing branch's __listds() is left unwrapped — a prefix like SYS1. isn't a single RACF DATASET resource — with a code comment flagging it to confirm on TK5. CWD is unchanged by design (ftpd_mvs_is_pds() is __locate+__dscbdv, no OPEN), so cd of a protected PDS still returns 250 and the denial surfaces on ls, matching z/OS.

Deploy order: this LIST fix must be deployed before the original STOR/RETR matrix can be run — the S913 on cd/ls of a protected PDS was blocking testers from even reaching the STOR test.

Additional on-MVS checks (TK5 HERC01 + MVS/CE IBMUSER):

# Regression check — MUST still succeed, authorized AS THE USER (run first):
cd 'SYS1.MACLIB'; ls     → member listing renders (user has READ), NOT a 550

# Security fix — previously leaked a listing under FTPD; now a clean deny:
cd 'SYS1.SECURE.CNTL'    → 250 (PDS context, no I/O), no crash
ls                       → 550 Access denied, NO S913, STC stays up; denial shows the user, not FTPD

The SYS1.MACLIB positive control is the key regression check: LIST now calls check_dataset_access() where it never did before.