diff --git a/.gitignore b/.gitignore index 333c1e9..8c90613 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,10 @@ logs/ + +# Live credentials - never committed. See docs/DECISIONS.md for the M5 +# database connection contract; the actual secret lives only in these +# untracked files (and, on the deployment host, wherever it is provisioned +# from there - never hardcoded into a .wsc/.asp/test file). +db +db.local +db.connectionstring +*.secrets diff --git a/Framework/Application.wsc b/Framework/Application.wsc index afb3b51..393687f 100644 --- a/Framework/Application.wsc +++ b/Framework/Application.wsc @@ -13,6 +13,7 @@ + @@ -28,7 +29,7 @@ Option Explicit ' (COM/method failure -> 500) outcomes, and one place logs them. ctx is our ' own WscMvc.RequestContext object (not an ASP intrinsic), carrying only ' primitive request data. No ASP intrinsics are referenced here. -Sub Run(ctx, applicationName, viewsDir, statusLine, contentType, body, allowHeader) +Sub Run(ctx, applicationName, viewsDir, dbConnectionString, statusLine, contentType, body, allowHeader) Dim router, handlerKey statusLine = "" @@ -72,7 +73,7 @@ Sub Run(ctx, applicationName, viewsDir, statusLine, contentType, body, allowHead Case "Home.Hello" RunHomeHello ctx, viewsDir, statusLine, contentType, body, allowHeader Case "SelfTest.RunSelfTest" - RunSelfTest ctx, statusLine, contentType, body, allowHeader + RunSelfTest ctx, dbConnectionString, statusLine, contentType, body, allowHeader Case Else InternalServerError statusLine, contentType, body, allowHeader End Select @@ -133,7 +134,7 @@ Sub RunHomeHello(ctx, viewsDir, statusLine, contentType, body, allowHeader) allowHeader = "" End Sub -Sub RunSelfTest(ctx, statusLine, contentType, body, allowHeader) +Sub RunSelfTest(ctx, dbConnectionString, statusLine, contentType, body, allowHeader) Dim ctrl, selfTestBody Set ctrl = Nothing @@ -149,7 +150,7 @@ Sub RunSelfTest(ctx, statusLine, contentType, body, allowHeader) selfTestBody = "" On Error Resume Next - ctrl.RunSelfTest ctx.LogDir, selfTestBody + ctrl.RunSelfTest ctx.LogDir, dbConnectionString, selfTestBody If Err.Number <> 0 Then Err.Clear On Error Goto 0 diff --git a/Framework/Database.wsc b/Framework/Database.wsc new file mode 100644 index 0000000..713ba77 --- /dev/null +++ b/Framework/Database.wsc @@ -0,0 +1,253 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index f05cfb2..502ca76 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -37,9 +37,9 @@ Gate: route matrix and malformed URL tests pass. **MET** — see `docs/TEST-RESU Gate: output is deterministic and untrusted text does not become HTML. **MET** for the vertical slice shipped — see `docs/TEST-RESULTS.md`. `Views/Layout.html` (a layout convention) is not yet added since there is still only one view; add it when a second view needs shared chrome, not speculatively. ## M5 — Data -- [ ] ADODB contract and provider-specific integration test database. -- [ ] Parameterized operations, transaction ownership, cleanup on error. -Gate: real DB integration tests pass and input is not concatenated into SQL values. +- [x] ADODB contract and provider-specific integration test database. +- [x] Parameterized operations, transaction ownership, cleanup on error. +Gate: real DB integration tests pass and input is not concatenated into SQL values. **MET** — see `docs/TEST-RESULTS.md`. Includes one real defect found and fixed during testing (LocalDB is unreachable from the IIS `ApplicationPoolIdentity` worker process; pivoted to a network-reachable SQL Server with SQL auth) and one caught in the diagnostic code itself (a chained `On Error Resume Next` masked the real failure reason). Identifier allowlisting (SPEC SS9) is not yet exercised — no route takes a table/column name from input in this vertical slice; deferred until one does, not implemented speculatively. ## M6 — Hardening - [ ] Authentication/authorization design and security regression tests. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8230eeb..f2b1187 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1,4 +1,4 @@ -# WSC-MVC — Architecture (as implemented through M3) +# WSC-MVC — Architecture (as implemented through M5) ## Request flow @@ -8,7 +8,7 @@ GET / or GET /hello -> /Default.asp?route=/hello -> Server.CreateObject("WscMvc.RequestContext"); ctx.Initialize path, httpMethod, logDir -> Server.CreateObject("WscMvc.Application") - -> Application.Run(ctx, "production", statusLine, contentType, body, allowHeader) [Framework/Application.wsc] + -> Application.Run(ctx, "production", viewsDir, dbConnectionString, statusLine, contentType, body, allowHeader) [Framework/Application.wsc] -> CreateObject("WscMvc.Router") -> Router.Match(ctx, "production", ..., handlerKey) [Framework/Router.wsc] -> CreateObject("WscMvc.HomeController") @@ -63,10 +63,11 @@ curl http://100.127.62.31:8091/self-test | `Framework/Router.wsc` | `WscMvc.Router` | `{C92F9338-B478-4EAD-B865-892FFB1E1C51}` | | `Framework/Application.wsc` | `WscMvc.Application` | `{851C7763-1638-42FE-A166-BF3DD3A96A88}` | | `Framework/ViewRenderer.wsc` | `WscMvc.ViewRenderer` | `{4948DF84-5DC6-448A-9F1B-EB596C28842B}` | +| `Framework/Database.wsc` | `WscMvc.Database` | `{E3F7C1A2-9B4D-4E6F-8C3A-2D5B7A9E1F04}` | | `Controllers/HomeController.wsc` | `WscMvc.HomeController` | `{87488446-60BE-4068-8368-0B709BB68F3F}` | | `test-app/Controllers/SelfTestController.wsc` | `WscMvc.SelfTestController` | `{D2634944-4646-4C55-956E-4C05E7E10904}` | -CLSIDs are fixed at creation and must never be recycled for a different component (AGENTS.md). `Application`'s public `Run` signature changed during pre-release development (M2 added a leading `ctx` parameter; M3 added an `allowHeader` output parameter) under the same CLSID; acceptable because this is active pre-release (v0.1) development with exactly one caller shape (`Default.asp`, updated in lockstep) — not a claim that live interface changes are safe for a published/external client. +CLSIDs are fixed at creation and must never be recycled for a different component (AGENTS.md). `Application`'s public `Run` signature changed during pre-release development (M2 added a leading `ctx` parameter; M3 added an `allowHeader` output parameter; M4 added `viewsDir`; M5 added `dbConnectionString`) under the same CLSID; acceptable because this is active pre-release (v0.1) development with exactly one caller shape (`Default.asp`, updated in lockstep) — not a claim that live interface changes are safe for a published/external client. ## IIS sites (test host: win2025test, 100.127.62.31) @@ -98,6 +99,16 @@ Substitution is a single deterministic left-to-right scan; template content is n `Views/Home.html` is exactly `{{Message}}`; `HomeController` supplies `message = "Hello from WSC-MVC!"`, a string with no HTML-special characters, so `/hello`'s response body is byte-identical to the pre-M4 (M1-era) contract — the rendering pipeline was proven end-to-end without changing observable behavior. `Views/Layout.html` and a layout convention are deferred until a second view actually needs shared chrome (IMPLEMENTATION_PLAN M4 explicitly sequences "add layout after renderer works"). +## Data (M5) + +`Framework/Database.wsc` (`WscMvc.Database`) owns an explicit, request-scoped ADODB connection/transaction/command lifecycle — never cached in Session/Application (SPEC §10), never referencing ASP intrinsics. `Open`/`Close` are idempotent; `BeginTransaction`/`CommitTransaction`/`RollbackTransaction` give the caller explicit transaction ownership. `ExecuteNonQuery(sql, paramOrder, paramsDict, rowsAffected)` and `ExecuteScalar(sql, paramOrder, paramsDict, result)` use ADO's positional `?` placeholders: `paramOrder` is a plain comma-separated *string* naming the placeholder order (not an array — deliberately avoiding an untested VBScript-array-through-WSC-`` marshaling question), and `paramsDict` is a `Scripting.Dictionary` of name→value, the same object-parameter pattern already proven by `ViewRenderer.wsc`. ADODB parameter type/size are inferred from VBScript `VarType()`/`Len()` — a deliberate minimal v0.1 policy (not a general ORM type system, SPEC §3 non-goal). `Null` values pass through as a real SQL `NULL`. Every failure path (`Open`, command prep, execution) releases any `ADODB.Command`/`Recordset` it created and raises a generic, connection-string-free error — `Database.wsc` never leaks credentials or internals, matching SPEC §10. + +**Connection string wiring**: the live connection string (with real credentials) lives only in an untracked `db.connectionstring` file at the project root — `.gitignore`d, never hardcoded into any tracked `.wsc`/`.asp`/test file. Each `Default.asp` reads it via `Server.MapPath` exactly like `logDir`/`viewsDir` (production: `"../db.connectionstring"`; test-app: `"../../db.connectionstring"`, since `test-app/public/` sits one directory level deeper than production's `public/`) and threads it through `Application.Run`'s `dbConnectionString` parameter. `tools/Setup-TestDatabase.ps1` is the explicit, manually-run deployment step that creates the test database/schema (idempotent, same pattern as `tools/Setup-Site.ps1`) — never invoked from application code, per SPEC §10's "no broad filesystem/DB write privileges from a web request." + +**No new production route/view was added for this milestone.** SPEC §3 lists "database-backed example" as a v0.1 non-goal; M5 is proven the same way M1–M4 proved their components — direct WSH tests (`tests/Test-Components.vbs`) plus one diagnostic-only check added to the existing test-app `SelfTestController` (`database_connectivity_under_app_pool_identity`), which exists specifically to verify DB connectivity under the real IIS `ApplicationPoolIdentity` worker process, not just an interactive dev session. See `docs/DECISIONS.md` for why the first engine choice (LocalDB) failed that exact check and was replaced with a network-reachable SQL Server using SQL authentication (which has no dependency on the caller's Windows identity). + +No route currently selects a table/column name from user input, so SPEC §9's identifier-allowlist requirement isn't yet exercised — deferred until a route actually needs it, not implemented speculatively. + ## Deferred to later milestones (do not implement early) -Per SPEC §3 non-goals and IMPLEMENTATION_PLAN M5+: ADODB, auth. A more robust logging mechanism (if complete coverage under concurrent load is ever required) is deferred to M6 — see the concurrency finding in `docs/DECISIONS.md`. A layout convention (`Views/Layout.html`) and non-text-context encoding are deferred until a concrete view needs them — see the Views section above. +Per SPEC §3 non-goals and IMPLEMENTATION_PLAN M6+: auth, a least-privileged (non-`sa`) SQL login for the M5 database (see `docs/DECISIONS.md`), identifier allowlisting (no route needs it yet). A more robust logging mechanism (if complete coverage under concurrent load is ever required) is deferred to M6 — see the concurrency finding in `docs/DECISIONS.md`. A layout convention (`Views/Layout.html`) and non-text-context encoding are deferred until a concrete view needs them — see the Views section above. diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 1d8d6a3..c9592a3 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -127,3 +127,17 @@ Added `Framework/ViewRenderer.wsc` (`WscMvc.ViewRenderer`) and `Views/Home.html` No experimental surprises this milestone (unlike M0-M3, which each surfaced a real WSC/VBScript defect) — returning a newly-created object through a `Sub`'s `ByRef` out-parameter (`ctrl.Hello viewName, message`, both scalars in this case, but the same mechanism was also exercised for `Set data = CreateObject(...)` inside `RunHomeHello`) behaved exactly as ordinary VBScript `ByRef` semantics predict, and `Err.Raise` inside a WSC method, caught by the caller's existing `On Error Resume Next` + `Err.Number` pattern, worked identically to the naturally-thrown errors (`CreateObject` failures) already relied on elsewhere. Confirmed via the WSH suite's dedicated `ViewRenderer` contract tests (encoding, missing template, unresolved placeholder, invalid view name) before trusting it in `Application.wsc`. Confirmed `Views/` is unreachable over HTTP purely from being a sibling of `public/` (same guarantee already used for `Framework/`/`Controllers/`) — no new `web.config` rule was needed: `GET /Views/Home.html` → `404` on both sites. + +## M5 — Data: ADODB contract, and why LocalDB was abandoned for a real SQL Server (2026-09-19) + +**"database-backed example" in SPEC SS3 vs. M5's own gate**: SPEC SS3's non-goals list includes "database-backed example," which reads at first glance like it contradicts M5's gate ("real DB integration tests pass"). Read against SS9 (a full ADODB contract described as a real, later-phase deliverable), the non-goal is interpreted as "no new demo feature/route/page built to show the DB off" — not "don't implement the ADODB contract." M5 was implemented and proven the same way M1-M4 proved their components: direct WSH component tests (`tests/Test-Components.vbs`) plus one check added to the existing test-app `SelfTestController` (already a diagnostics-only surface, not a production feature) — no new production controller, route, or view was added. Flagged to Daniel before implementing; no objection raised. + +**Engine choice, attempt 1 (LocalDB) - failed for a real, evidenced reason, not a guess.** Daniel initially approved "SQL Server Express / LocalDB." LocalDB was tried first (already installed on this host, zero additional install) and its ADODB contract fully passed under the interactive dev account (`cscript`), via `Driver={ODBC Driver 17 for SQL Server}` - `MSOLEDBSQL` is not installed on this host and legacy `SQLOLEDB` cannot locate the LocalDB named-pipe instance at all, so ODBC Driver 17 is the only working provider found. However, SPEC SS10's "validate application pool identity... where applicable" discipline requires proving this under the *real* IIS worker identity, not just an interactive session - AGENTS.md is explicit that WSC/COM/host assumptions must be verified, not assumed. A dedicated `database_connectivity_under_app_pool_identity` check was added to `SelfTestController` (test-app's `WscMvcTests` site, `ApplicationPoolIdentity`, confirmed via `Get-WebConfigurationProperty` - no custom identity configured) and hit over real HTTP. It failed: `SQL Server Network Interfaces: ` (ADODB error -2147467259). This matches Microsoft's own documented guidance that LocalDB is a per-user, interactive-profile-dependent instance not supported for IIS-hosted use - now confirmed experimentally on this exact host/identity, not inferred from documentation alone. + +**Pivot to a real SQL Server, provided by Daniel mid-session.** Daniel supplied connection details (host/port/`sa` credentials) for an existing SQL Server 2022 instance reachable over the tailnet (`100.120.114.48:1433`). SQL authentication over plain TCP has no dependency on the calling process's Windows identity at all (unlike LocalDB's `Trusted_Connection`/named-pipe model), so this sidesteps the `ApplicationPoolIdentity` problem entirely rather than working around it. Verified: TCP reachability (`Test-NetConnection` succeeded), then an ADODB connection with SQL auth (interactive session, then - critically - re-verified via the same `SelfTestController` HTTP check under the real app-pool identity, which now reports `"pass":true`). `tools/Setup-TestDatabase.ps1` was generalized from a LocalDB-only script to accept `-ServerInstance`/`-SqlLogin`/`-SqlPassword` (the last a `SecureString`, converted to plain text only in the narrow window `sqlcmd.exe` - an external process with no SecureString-aware API - actually needs it as a CLI argument; flagged by the IDE's own PSScriptAnalyzer and fixed before use, not after). It remains idempotent (same create-if-missing pattern as `Setup-Site.ps1`) and is never invoked from application code - only as an explicit, manually-run deployment step, per SPEC SS10. + +**Credential handling.** The live password reached this session as a plaintext note (`db`, opened by Daniel in the IDE) and is never embedded in any tracked file. `.gitignore` now excludes `db`, `db.connectionstring`, `db.local`, and `*.secrets`, added and verified (`git check-ignore -v`) *before* the actual connection string file was written to disk - not after. `db.connectionstring` (project root, untracked) holds the one live ADO connection string; `Default.asp` (both sites) reads it via `Server.MapPath` exactly like `logDir`/`viewsDir` and threads it through `Application.Run`'s new `dbConnectionString` parameter - `Framework/Database.wsc` itself never touches the filesystem or ASP intrinsics for this. Test-app's `public/` is two directory levels below the project root (`root -> test-app -> public`, vs. production's one level, `root -> public`), so its `Default.asp` needed `"../../db.connectionstring"`, not `"../db.connectionstring"` - caught by checking the actual path depth rather than copy-pasting production's relative path. `tests/Test-Components.vbs` reads the same file directly and skips (NOT RUN, not FAIL) its entire Database contract section when the file is absent, so the rest of the suite still gives a clean verdict on a machine without this secret provisioned. The `sa` login is a real production-grade credential with full admin rights on the target server; using it directly from the app is accepted for this dev/test milestone but is a real least-privilege gap - flagged here for M6 hardening (a dedicated, minimally-privileged SQL login scoped to `WscMvcTest`), not silently treated as the final state. + +**A real bug caught in my own diagnostic code, not just the framework.** The first version of the `database_connectivity_under_app_pool_identity` self-test check chained `Open`/`ExecuteScalar`/`Close` under one `On Error Resume Next` block and checked `Err.Number` only once at the end - exactly the anti-pattern AGENTS.md's VBScript rules forbid ("no `On Error Resume Next` across an entire function; check `Err.Number` immediately"). Because `On Error Resume Next` lets execution continue to the next statement after an error, `ExecuteScalar`'s later "Database is not open" error silently overwrote `Open`'s real, more informative failure - masking the actual root cause (`SQL Server Network Interfaces`) behind a misleading message. Fixed by checking `Err.Number` after each call individually, matching the rest of the codebase's established discipline, before trusting the result to diagnose the LocalDB failure above. + +**`Framework/Database.wsc` (`WscMvc.Database`) contract summary**: explicit `Open`/`Close` (idempotent) and `BeginTransaction`/`CommitTransaction`/`RollbackTransaction`; `ExecuteNonQuery`/`ExecuteScalar` take `sql` (ADO positional `?` placeholders), `paramOrder` (a comma-separated *string*, not an array - deliberately avoiding an untested VBScript-array-through-WSC-``-method marshaling question for zero real benefit), and `paramsDict` (a `Scripting.Dictionary`, the same object-parameter pattern already proven by `ViewRenderer.wsc` in M4). Parameter ADODB type/size are inferred from VBScript `VarType()`/`Len()` - a deliberate minimal v0.1 policy (not a general ORM type system, per SPEC SS3). `Null` values pass through as a real SQL `NULL` via `Parameter.Value = Null`, verified round-tripping correctly, not assumed. Verified via `tests/Test-Components.vbs`: a parameterized INSERT/SELECT round-trip of a value containing a literal single quote (proving no string-concatenation injection risk and no manual escaping needed), explicit commit and rollback (each checked against a `COUNT(*)` read-back, not just "no error"), and safe failure on an unopened/closed connection. diff --git a/docs/TEST-RESULTS.md b/docs/TEST-RESULTS.md index 4e76d57..c7e6a3d 100644 --- a/docs/TEST-RESULTS.md +++ b/docs/TEST-RESULTS.md @@ -420,6 +420,72 @@ curl -o /dev/null -w "%{http_code}" http://localhost:8090/Views/Home.html -> 4 M4 gate status: **PASS** for the vertical slice implemented (safe separate template loading from a physically non-served directory; deterministic `{{Key}}` substitution; default HTML-text-context encoding verified against a real escaping case; missing-template and unresolved-placeholder failures verified path-free and safe; `/hello`'s existing exact-body contract verified unchanged end-to-end through IIS). `Views/Layout.html` and a layout convention are deferred (IMPLEMENTATION_PLAN M4 sequences "add layout after renderer works" as a second step) — not yet needed since there is still only one view. Attribute/URL/JS-context encoding remains explicitly out of scope until a view needs it (documented on `ViewRenderer.wsc` and in `docs/ARCHITECTURE.md`). +## M5 — Data: ADODB contract, real DB integration (2026-09-19) + +Host under test: `DESKTOP-80D128R`, same local IIS sites as M3/M4. Added `Framework/Database.wsc` (`WscMvc.Database`, `{E3F7C1A2-9B4D-4E6F-8C3A-2D5B7A9E1F04}`). Full narrative (engine-choice pivot, credential handling, a real bug caught in the diagnostic code itself) is in `docs/DECISIONS.md`; this section is the command-by-command evidence. + +**Attempt 1 — LocalDB (rejected with evidence).** `(localdb)\MSSQLLocalDB` was already present on this host. WSH-level contract tests (interactive account) **PASSED** via `Driver={ODBC Driver 17 for SQL Server}` (the only working provider — `MSOLEDBSQL` isn't installed, legacy `SQLOLEDB` can't find the named-pipe instance). A dedicated self-test check under the real IIS `ApplicationPoolIdentity` (`WscMvcTests` app pool, confirmed default/no custom identity via `Get-WebConfigurationProperty`) **FAILED**: + +``` +curl http://localhost:8091/self-test +{"...","name":"database_connectivity_under_app_pool_identity","pass":false,"detail":"Open: -2147467259 - [Microsoft][ODBC Driver 17 for SQL Server]SQL Server Network Interfaces: "} +``` + +This matches Microsoft's documented guidance that LocalDB isn't supported for IIS-hosted use — confirmed experimentally on this exact host/identity, not assumed from docs alone. + +**Attempt 2 — real SQL Server (works).** Daniel provided credentials for an existing SQL Server 2022 instance on the tailnet (`100.120.114.48:1433`, SQL auth). Connectivity verified in stages: + +``` +Test-NetConnection -ComputerName 100.120.114.48 -Port 1433 -> TcpTestSucceeded: True +cscript probe-sql.vbs -> who=sa version=Microsoft SQL Server 2022 (RTM-CU27)... / OK +``` + +`tools\Setup-TestDatabase.ps1 -ServerInstance '100.120.114.48,1433' -SqlLogin sa -SqlPassword ` (idempotent; run twice, second run a no-op) created database `WscMvcTest` and table `dbo.Widgets`. + +WSH component contract, connection string read from the untracked `db.connectionstring` (not hardcoded): + +``` +cscript //nologo tests\Test-Components.vbs +``` + +Result: **PASS**, Database-specific lines: + +``` +PASS: Database.ExecuteNonQuery(INSERT) rowsAffected +PASS: Database.ExecuteScalar round-trips quote-containing value +PASS: Database.ExecuteScalar Null parameter round-trips as SQL NULL +PASS: Database.RollbackTransaction leaves no row +PASS: Database.CommitTransaction persists the row +PASS: Database.ExecuteScalar on an unopened connection raises a safe error +PASS: Database.Close is idempotent +RESULT: ALL PASS +``` + +The INSERT/SELECT round-trip used the value `O'Brien` (a literal single quote) specifically to prove no string-concatenation SQL injection and no manual escaping requirement — a naive `"...VALUES ('" & name & "')"` would have broken on this exact input; the parameterized call did not. + +Real IIS/app-pool-identity re-check after the pivot: + +``` +curl http://localhost:8091/self-test +{"ok":true,"checks":[...,{"name":"database_connectivity_under_app_pool_identity","pass":true,"detail":""}]} +``` + +Full regression after all M5 wiring changes (both `Default.asp` files, `Application.wsc`'s `Run` signature, `SelfTestController.wsc`): + +``` +cscript //nologo tests\Test-Components.vbs -> ALL PASS (34 checks) +powershell -File tests\Test-Http.ps1 -BaseUrl http://localhost:8090 -TestBaseUrl http://localhost:8091 -> ALL PASS (14 checks) +curl http://localhost:8090/hello -> 200, unchanged body +curl -o /dev/null -w "%{http_code}" http://localhost:8090/db.connectionstring -> 404 +curl -o /dev/null -w "%{http_code}" http://localhost:8090/db -> 404 +curl -o /dev/null -w "%{http_code}" http://localhost:8091/db.connectionstring -> 404 +curl -o /dev/null -w "%{http_code}" http://localhost:8091/db -> 404 +``` + +Secret hygiene verified directly, not assumed: `git check-ignore -v db db.connectionstring` confirmed both patterns match, and `git status --short` never showed either file as trackable, from before either file was written to disk. + +M5 gate status: **PASS** — real DB integration tests pass against a real, explicitly-provisioned SQL Server (not mocked, not LocalDB-only); every value crosses the SQL boundary as a bound ADO parameter, never string concatenation. Deferred to M6: a least-privileged (non-`sa`) SQL login for the test database; identifier allowlisting (no route yet selects a table/column name from input, so there's nothing to allowlist against). + ## Remote deployment tooling — Windows Server 2025 live proof (2026-09-19) Host under test: `win2025test`, Windows Server 2025 Standard build 26100, 64-bit; production `WscMvc` on `:8090` and test `WscMvcTests` on `:8091`. diff --git a/public/Default.asp b/public/Default.asp index 0aa4e26..676c70c 100644 --- a/public/Default.asp +++ b/public/Default.asp @@ -1,7 +1,7 @@ <%@ Language="VBScript" %> <% Option Explicit %> <% -Dim route, httpMethod, logDir, viewsDir, ctx, app, applicationName, statusLine, contentType, body, allowHeader +Dim route, httpMethod, logDir, viewsDir, dbConnStringPath, dbConnStringFso, dbConnectionString, ctx, app, applicationName, statusLine, contentType, body, allowHeader route = Request.QueryString("route") httpMethod = Request.ServerVariables("REQUEST_METHOD") @@ -12,6 +12,21 @@ applicationName = "production" logDir = Server.MapPath("../logs") viewsDir = Server.MapPath("../Views") +' db.connectionstring holds the live DB credentials and is never committed +' (see .gitignore, docs/DECISIONS.md) - read as plain framework wiring here, +' same as logDir/viewsDir, never hardcoded into any tracked file. Optional: +' an empty string means no route on this app currently needs a database. +dbConnStringPath = Server.MapPath("../db.connectionstring") +dbConnectionString = "" +On Error Resume Next +Set dbConnStringFso = Server.CreateObject("Scripting.FileSystemObject") +If dbConnStringFso.FileExists(dbConnStringPath) Then + dbConnectionString = Trim(dbConnStringFso.OpenTextFile(dbConnStringPath, 1).ReadAll) +End If +Set dbConnStringFso = Nothing +Err.Clear +On Error Goto 0 + Set ctx = Nothing On Error Resume Next Set ctx = Server.CreateObject("WscMvc.RequestContext") @@ -58,7 +73,7 @@ body = "" allowHeader = "" On Error Resume Next -app.Run ctx, applicationName, viewsDir, statusLine, contentType, body, allowHeader +app.Run ctx, applicationName, viewsDir, dbConnectionString, statusLine, contentType, body, allowHeader If Err.Number <> 0 Then Err.Clear On Error Goto 0 diff --git a/test-app/Controllers/SelfTestController.wsc b/test-app/Controllers/SelfTestController.wsc index 0cc91a1..01939df 100644 --- a/test-app/Controllers/SelfTestController.wsc +++ b/test-app/Controllers/SelfTestController.wsc @@ -11,6 +11,7 @@ + @@ -28,7 +29,7 @@ Option Explicit ' route IS the diagnostics surface, so its whole job is to say what broke. ' This is a dev/test-milestone tool, not a production data endpoint - ' revisit whether it should be gated/removed during M6 hardening. -Sub RunSelfTest(logDir, body) +Sub RunSelfTest(logDir, dbConnectionString, body) Dim checks, allPass checks = "" allPass = True @@ -87,7 +88,7 @@ Sub RunSelfTest(logDir, body) Set ctxUnknown = CreateObject("WscMvc.RequestContext") ctxUnknown.Initialize "/hello", "GET", logDir unkStatus = "" : unkType = "" : unkBody = "" : unkAllow = "" - app.Run ctxUnknown, "tests", "", unkStatus, unkType, unkBody, unkAllow + app.Run ctxUnknown, "tests", "", "", unkStatus, unkType, unkBody, unkAllow If Err.Number <> 0 Then unkDetail = Err.Description Err.Clear @@ -147,6 +148,79 @@ Sub RunSelfTest(logDir, body) checks = AppendCheck(checks, "delete_self_test_allow_header", delOk, delDetail) allPass = allPass And delOk + ' --- Database connectivity under the real IIS app-pool identity (M5). + ' This is the genuinely open question SPEC SS10/SS15's "validate app pool + ' identity... where applicable" discipline calls for: WSH/direct component + ' tests only prove the contract under the interactive dev account. SQL + ' auth over TCP (not LocalDB's Windows-integrated named pipes) doesn't + ' depend on the caller's OS identity, but this is still verified for real + ' here rather than assumed, exactly because this route runs inside the + ' real worker process under ApplicationPoolIdentity - see + ' docs/DECISIONS.md for the LocalDB attempt that failed this exact check. + ' NOTE: this probes ADODB.Connection directly, bypassing WscMvc.Database, + ' solely to see the real underlying driver error - Database.Open + ' deliberately discards Err.Description (SPEC SS10: never leak connection + ' strings/internals to a client), which is correct for the production + ' error boundary but useless for diagnosing *why* connectivity fails. + ' SelfTestController is the one place intentionally exempted from that + ' rule (see file header comment). + Dim rawConn, dbOk, dbDetail, dbRs, dbSkipped + dbOk = False + dbDetail = "" + dbSkipped = False + + If Len(Trim(dbConnectionString)) = 0 Then + ' No db.connectionstring provisioned on this deployment - an + ' environment-provisioning fact, not a framework defect, so this + ' reports as passing-but-skipped rather than dragging down "ok" + ' (same NOT-RUN-not-FAIL discipline as tests/Test-Components.vbs). + dbSkipped = True + dbOk = True + dbDetail = "skipped: db.connectionstring not provisioned on this deployment" + End If + + If Not dbSkipped Then + On Error Resume Next + Set rawConn = CreateObject("ADODB.Connection") + If Err.Number <> 0 Then + dbDetail = "CreateObject: " & Err.Description + Err.Clear + End If + On Error Goto 0 + End If + + If Len(dbDetail) = 0 Then + On Error Resume Next + rawConn.Open dbConnectionString + If Err.Number <> 0 Then + dbDetail = "Open: " & Err.Number & " - " & Err.Description + Err.Clear + End If + On Error Goto 0 + End If + + If Len(dbDetail) = 0 Then + On Error Resume Next + Set dbRs = rawConn.Execute("SELECT 1") + If Err.Number <> 0 Then + dbDetail = "Execute: " & Err.Number & " - " & Err.Description + Err.Clear + Else + dbOk = True + dbRs.Close + End If + On Error Goto 0 + End If + + On Error Resume Next + If Not rawConn Is Nothing Then rawConn.Close + Err.Clear + On Error Goto 0 + + checks = AppendCheck(checks, "database_connectivity_under_app_pool_identity", dbOk, dbDetail) + allPass = allPass And dbOk + Set rawConn = Nothing + Set ctx1 = Nothing Set ctx2 = Nothing Set ctxUnknown = Nothing diff --git a/test-app/public/Default.asp b/test-app/public/Default.asp index dcc7d8a..0fd7dae 100644 --- a/test-app/public/Default.asp +++ b/test-app/public/Default.asp @@ -5,7 +5,7 @@ ' set in the shared framework. Production's bootstrap selects "production". ' Keeping this value inside each app prevents direct Default.asp?route=... ' requests from crossing the application boundary. -Dim route, httpMethod, logDir, viewsDir, ctx, app, applicationName, statusLine, contentType, body, allowHeader +Dim route, httpMethod, logDir, viewsDir, dbConnStringPath, dbConnStringFso, dbConnectionString, ctx, app, applicationName, statusLine, contentType, body, allowHeader route = Request.QueryString("route") httpMethod = Request.ServerVariables("REQUEST_METHOD") @@ -16,6 +16,25 @@ applicationName = "tests" logDir = Server.MapPath("../logs") viewsDir = Server.MapPath("../Views") +' db.connectionstring holds the live DB credentials and is never committed +' (see .gitignore, docs/DECISIONS.md) - read as plain framework wiring here, +' same as logDir/viewsDir, never hardcoded into any tracked file. Optional: +' an empty string means it isn't provisioned on this deployment. Unlike +' logs/ (each site has its own), there is exactly one shared credential file +' at the project root - test-app/public/ is two levels below the project +' root (root -> test-app -> public), one level deeper than production's +' public/ (root -> public), so it needs "../../" not "../" to reach it. +dbConnStringPath = Server.MapPath("../../db.connectionstring") +dbConnectionString = "" +On Error Resume Next +Set dbConnStringFso = Server.CreateObject("Scripting.FileSystemObject") +If dbConnStringFso.FileExists(dbConnStringPath) Then + dbConnectionString = Trim(dbConnStringFso.OpenTextFile(dbConnStringPath, 1).ReadAll) +End If +Set dbConnStringFso = Nothing +Err.Clear +On Error Goto 0 + Set ctx = Nothing On Error Resume Next Set ctx = Server.CreateObject("WscMvc.RequestContext") @@ -62,7 +81,7 @@ body = "" allowHeader = "" On Error Resume Next -app.Run ctx, applicationName, viewsDir, statusLine, contentType, body, allowHeader +app.Run ctx, applicationName, viewsDir, dbConnectionString, statusLine, contentType, body, allowHeader If Err.Number <> 0 Then Err.Clear On Error Goto 0 diff --git a/tests/Test-Components.vbs b/tests/Test-Components.vbs index ee9e93d..d8dbdf1 100644 --- a/tests/Test-Components.vbs +++ b/tests/Test-Components.vbs @@ -1,6 +1,7 @@ Option Explicit Dim fso, logDir, viewsDir, testViewsDir, pass +Dim dbConnStringPath, dbConnString, dbSectionAvailable pass = True Set fso = CreateObject("Scripting.FileSystemObject") @@ -13,6 +14,23 @@ End If ' rendering pipeline, not just the ViewRenderer component in isolation). viewsDir = fso.GetParentFolderName(WScript.ScriptFullName) & "\..\Views" +' The connection string (with live credentials) lives only in the untracked +' db.connectionstring file (see .gitignore, docs/DECISIONS.md) - never +' hardcoded here. Read up front so every RunApplication call (including the +' non-DB ones above the Database contract section below) sees a consistent +' value. If the file isn't present on this machine, dbSectionAvailable gates +' the Database contract tests as NOT RUN rather than FAIL further down. +dbConnStringPath = fso.GetParentFolderName(WScript.ScriptFullName) & "\..\db.connectionstring" +dbSectionAvailable = fso.FileExists(dbConnStringPath) +dbConnString = "" +If dbSectionAvailable Then + Dim dbConnStringStreamEarly + Set dbConnStringStreamEarly = fso.OpenTextFile(dbConnStringPath, 1) + dbConnString = Trim(dbConnStringStreamEarly.ReadAll) + dbConnStringStreamEarly.Close + Set dbConnStringStreamEarly = Nothing +End If + ' Disposable fixture directory for direct ViewRenderer contract tests below. testViewsDir = fso.GetParentFolderName(WScript.ScriptFullName) & "\test-views" If fso.FolderExists(testViewsDir) Then @@ -31,7 +49,7 @@ Sub RunApplication(app, path, httpMethod, applicationName, statusLine, contentTy Dim ctx Set ctx = NewContext(path, httpMethod) statusLine = "" : contentType = "" : body = "" : allowHeader = "" - app.Run ctx, applicationName, viewsDir, statusLine, contentType, body, allowHeader + app.Run ctx, applicationName, viewsDir, dbConnString, statusLine, contentType, body, allowHeader Set ctx = Nothing End Sub @@ -313,6 +331,218 @@ End If Set renderer = Nothing +' --- Database contract (M5) --- +' Targets the dedicated integration test database created by +' tools/Setup-TestDatabase.ps1 (never auto-created by application code - +' SPEC SS10: no broad filesystem/DB write privileges from a web request). +' dbConnString/dbSectionAvailable were already read at the top of this file +' (before the first RunApplication call). If db.connectionstring isn't +' present on this machine, the DB section is skipped (NOT RUN) rather than +' failed, since the real issue is an unprovisioned environment, not a +' framework defect. +Dim db +If Not dbSectionAvailable Then + WScript.Echo "NOT RUN: Database contract tests skipped - db.connectionstring not found at " & dbConnStringPath +End If + +If pass And dbSectionAvailable Then + On Error Resume Next + Set db = CreateObject("WscMvc.Database") + If Err.Number <> 0 Then + WScript.Echo "FAIL: could not create WscMvc.Database - " & Err.Description + pass = False + Err.Clear + End If + On Error Goto 0 +End If + +If pass And dbSectionAvailable Then + On Error Resume Next + db.Open dbConnString + If Err.Number <> 0 Then + WScript.Echo "FAIL: Database.Open raised error - " & Err.Description + pass = False + Err.Clear + End If + On Error Goto 0 +End If + +' Clean slate so this test is deterministic across repeated runs. +If pass And dbSectionAvailable Then + Dim rowsCleared + On Error Resume Next + db.ExecuteNonQuery "DELETE FROM dbo.Widgets", "", Nothing, rowsCleared + If Err.Number <> 0 Then + WScript.Echo "FAIL: Database.ExecuteNonQuery(DELETE) raised error - " & Err.Description + pass = False + Err.Clear + End If + On Error Goto 0 +End If + +' --- Parameterized INSERT with a value containing a single quote: proves no +' string-concatenation SQL injection (a naive concatenation would break the +' SQL syntax or need manual quote-escaping; the parameterized call needs +' neither). --- +If pass And dbSectionAvailable Then + Dim insertParams, rowsInserted + Set insertParams = CreateObject("Scripting.Dictionary") + insertParams.Add "Name", "O'Brien" + On Error Resume Next + db.ExecuteNonQuery "INSERT INTO dbo.Widgets (Name) VALUES (?)", "Name", insertParams, rowsInserted + If Err.Number <> 0 Then + WScript.Echo "FAIL: Database.ExecuteNonQuery(INSERT) raised error - " & Err.Description + pass = False + Err.Clear + End If + On Error Goto 0 + If pass Then + CheckEqual rowsInserted, 1, "Database.ExecuteNonQuery(INSERT) rowsAffected" + End If + Set insertParams = Nothing +End If + +' --- Parameterized SELECT round-trips the quote-containing value intact --- +If pass And dbSectionAvailable Then + Dim selectParams, foundName + Set selectParams = CreateObject("Scripting.Dictionary") + selectParams.Add "Name", "O'Brien" + On Error Resume Next + db.ExecuteScalar "SELECT Name FROM dbo.Widgets WHERE Name = ?", "Name", selectParams, foundName + If Err.Number <> 0 Then + WScript.Echo "FAIL: Database.ExecuteScalar(SELECT) raised error - " & Err.Description + pass = False + Err.Clear + End If + On Error Goto 0 + If pass Then + CheckEqual foundName, "O'Brien", "Database.ExecuteScalar round-trips quote-containing value" + End If + Set selectParams = Nothing +End If + +' --- Null parameter contract: a Null value must pass through as a real SQL +' NULL, not the string "Null" or an empty string. --- +If pass And dbSectionAvailable Then + Dim nullParams, nullResult + Set nullParams = CreateObject("Scripting.Dictionary") + nullParams.Add "v", Null + On Error Resume Next + db.ExecuteScalar "SELECT ? AS v", "v", nullParams, nullResult + If Err.Number <> 0 Then + WScript.Echo "FAIL: Database.ExecuteScalar(Null param) raised error - " & Err.Description + pass = False + Err.Clear + End If + On Error Goto 0 + If pass Then + If IsNull(nullResult) Then + WScript.Echo "PASS: Database.ExecuteScalar Null parameter round-trips as SQL NULL" + Else + WScript.Echo "FAIL: Database.ExecuteScalar Null parameter -> expected Null, got [" & nullResult & "]" + pass = False + End If + End If + Set nullParams = Nothing +End If + +' --- Transaction contract: rollback leaves no trace --- +If pass And dbSectionAvailable Then + Dim rollbackParams, rowsRolledBack, countAfterRollback + On Error Resume Next + db.BeginTransaction + Set rollbackParams = CreateObject("Scripting.Dictionary") + rollbackParams.Add "Name", "RolledBack" + db.ExecuteNonQuery "INSERT INTO dbo.Widgets (Name) VALUES (?)", "Name", rollbackParams, rowsRolledBack + db.RollbackTransaction + If Err.Number <> 0 Then + WScript.Echo "FAIL: Database rollback sequence raised error - " & Err.Description + pass = False + Err.Clear + End If + On Error Goto 0 + + If pass Then + Dim countParams + Set countParams = CreateObject("Scripting.Dictionary") + countParams.Add "Name", "RolledBack" + On Error Resume Next + db.ExecuteScalar "SELECT COUNT(*) FROM dbo.Widgets WHERE Name = ?", "Name", countParams, countAfterRollback + On Error Goto 0 + CheckEqual countAfterRollback, 0, "Database.RollbackTransaction leaves no row" + Set countParams = Nothing + End If + Set rollbackParams = Nothing +End If + +' --- Transaction contract: commit persists the change --- +If pass And dbSectionAvailable Then + Dim commitParams, rowsCommitted, countAfterCommit + On Error Resume Next + db.BeginTransaction + Set commitParams = CreateObject("Scripting.Dictionary") + commitParams.Add "Name", "Committed" + db.ExecuteNonQuery "INSERT INTO dbo.Widgets (Name) VALUES (?)", "Name", commitParams, rowsCommitted + db.CommitTransaction + If Err.Number <> 0 Then + WScript.Echo "FAIL: Database commit sequence raised error - " & Err.Description + pass = False + Err.Clear + End If + On Error Goto 0 + + If pass Then + On Error Resume Next + db.ExecuteScalar "SELECT COUNT(*) FROM dbo.Widgets WHERE Name = ?", "Name", commitParams, countAfterCommit + On Error Goto 0 + CheckEqual countAfterCommit, 1, "Database.CommitTransaction persists the row" + End If + Set commitParams = Nothing +End If + +' --- Failure path: operating on a closed/never-opened connection is a safe error --- +If pass And dbSectionAvailable Then + Dim db2, closedOk, closedDetail, dummyResult + closedOk = False + Set db2 = CreateObject("WscMvc.Database") + On Error Resume Next + db2.ExecuteScalar "SELECT 1", "", Nothing, dummyResult + If Err.Number <> 0 Then + closedOk = True + Err.Clear + End If + On Error Goto 0 + If closedOk Then + WScript.Echo "PASS: Database.ExecuteScalar on an unopened connection raises a safe error" + Else + WScript.Echo "FAIL: Database.ExecuteScalar on an unopened connection did not raise -> got [" & dummyResult & "]" + pass = False + End If + Set db2 = Nothing +End If + +' --- Close is idempotent --- +If pass And dbSectionAvailable Then + Dim closeOk + closeOk = True + On Error Resume Next + db.Close + db.Close + If Err.Number <> 0 Then + closeOk = False + Err.Clear + End If + On Error Goto 0 + If closeOk Then + WScript.Echo "PASS: Database.Close is idempotent" + Else + WScript.Echo "FAIL: calling Database.Close twice raised an error" + pass = False + End If +End If + +Set db = Nothing + ' --- Logging: best-effort log file was written with both outcomes --- If pass Then Dim logPath, logContent diff --git a/tools/Register-Components.ps1 b/tools/Register-Components.ps1 index 53594f9..0ad79c6 100644 --- a/tools/Register-Components.ps1 +++ b/tools/Register-Components.ps1 @@ -15,6 +15,7 @@ $components = @( (Join-Path $ProjectRoot 'Framework\RequestContext.wsc'), (Join-Path $ProjectRoot 'Framework\Router.wsc'), (Join-Path $ProjectRoot 'Framework\ViewRenderer.wsc'), + (Join-Path $ProjectRoot 'Framework\Database.wsc'), (Join-Path $ProjectRoot 'Framework\Application.wsc'), (Join-Path $ProjectRoot 'Controllers\HomeController.wsc'), (Join-Path $ProjectRoot 'test-app\Controllers\SelfTestController.wsc') diff --git a/tools/Setup-TestDatabase.ps1 b/tools/Setup-TestDatabase.ps1 new file mode 100644 index 0000000..f474c30 --- /dev/null +++ b/tools/Setup-TestDatabase.ps1 @@ -0,0 +1,54 @@ +[CmdletBinding()] +param( + [Parameter(Mandatory = $true)][string]$ServerInstance, + [Parameter(Mandatory = $true)][string]$SqlLogin, + [Parameter(Mandatory = $true)][SecureString]$SqlPassword, + [string]$DatabaseName = 'WscMvcTest' +) + +# Idempotent: creates the M5 integration test database and its one table if +# missing, does nothing if they already exist. This is a deliberate, explicit +# deployment step (SPEC SS10/SS9: "no broad filesystem/DB write privileges"; +# a web request must never create schema) - never called from Default.asp or +# any WSC, only from this tool. Credentials are supplied as parameters at +# invocation time, never embedded in this (tracked) script - see +# docs/DECISIONS.md for where the actual secret lives. SqlPassword is a +# SecureString and is only ever converted to plain text in the narrow window +# where sqlcmd.exe (an external process with no SecureString-aware API) +# actually needs it as a command-line argument. +$ErrorActionPreference = 'Stop' + +$plainPassword = [Runtime.InteropServices.Marshal]::PtrToStringUni( + [Runtime.InteropServices.Marshal]::SecureStringToGlobalAllocUnicode($SqlPassword) +) + +$createDbSql = @" +IF DB_ID(N'$DatabaseName') IS NULL +BEGIN + CREATE DATABASE [$DatabaseName]; +END +"@ + +$createTableSql = @" +IF OBJECT_ID(N'dbo.Widgets', N'U') IS NULL +BEGIN + CREATE TABLE dbo.Widgets ( + Id INT IDENTITY(1,1) PRIMARY KEY, + Name NVARCHAR(100) NOT NULL + ); +END +"@ + +try { + Write-Output "Ensuring database [$DatabaseName] exists on $ServerInstance ..." + sqlcmd -S $ServerInstance -U $SqlLogin -P $plainPassword -d master -Q $createDbSql -b + if ($LASTEXITCODE -ne 0) { throw "sqlcmd failed creating database (exit $LASTEXITCODE)" } + + Write-Output "Ensuring dbo.Widgets table exists ..." + sqlcmd -S $ServerInstance -U $SqlLogin -P $plainPassword -d $DatabaseName -Q $createTableSql -b + if ($LASTEXITCODE -ne 0) { throw "sqlcmd failed creating table (exit $LASTEXITCODE)" } +} finally { + $plainPassword = $null +} + +Write-Output "Test database ready: [$DatabaseName] on $ServerInstance (table dbo.Widgets)." diff --git a/tools/Unregister-Components.ps1 b/tools/Unregister-Components.ps1 index b623d29..37c67c5 100644 --- a/tools/Unregister-Components.ps1 +++ b/tools/Unregister-Components.ps1 @@ -19,6 +19,7 @@ $components = @( @{ Path = (Join-Path $ProjectRoot 'Controllers\HomeController.wsc'); ProgId = 'WscMvc.HomeController'; ClassId = '{87488446-60BE-4068-8368-0B709BB68F3F}' }, @{ Path = (Join-Path $ProjectRoot 'Framework\Application.wsc'); ProgId = 'WscMvc.Application'; ClassId = '{851C7763-1638-42FE-A166-BF3DD3A96A88}' }, @{ Path = (Join-Path $ProjectRoot 'Framework\ViewRenderer.wsc'); ProgId = 'WscMvc.ViewRenderer'; ClassId = '{4948DF84-5DC6-448A-9F1B-EB596C28842B}' }, + @{ Path = (Join-Path $ProjectRoot 'Framework\Database.wsc'); ProgId = 'WscMvc.Database'; ClassId = '{E3F7C1A2-9B4D-4E6F-8C3A-2D5B7A9E1F04}' }, @{ Path = (Join-Path $ProjectRoot 'Framework\Router.wsc'); ProgId = 'WscMvc.Router'; ClassId = '{C92F9338-B478-4EAD-B865-892FFB1E1C51}' }, @{ Path = (Join-Path $ProjectRoot 'Framework\RequestContext.wsc'); ProgId = 'WscMvc.RequestContext'; ClassId = '{1C36FA55-34DF-4974-94B9-D657389362B2}' } )