dohertj2/scadalink-design
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(inbound-api): resolve InboundAPI-007 — remove unimplemented Database.Connection() script API from design doc (conflicts with script trust model)

Browse Source
This commit is contained in:
Joseph Doherty
2026-05-16 23:49:25 -04:00
parent 3d0c1c6963
commit a2f6c1b9b2
2 changed files with 26 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
code-reviews/InboundAPI/findings.md
View File

@@ -8,7 +8,7 @@
| Last reviewed | 2026-05-16 |
| Reviewer | claude-agent |
| Commit reviewed | `9c60592` |
| Open findings | 2 |
| Open findings | 1 |
## Summary
@@ -294,9 +294,9 @@ and `FilterCapsMaxRequestBodySizeFeature` added.
| | |
|--|--|
| Severity | Medium — verified real drift; left Open pending a design decision (see Resolution) |
| Severity | Medium |
| Category | Design-document adherence |
| Status | Open |
| Status | Resolved |
| Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:188-203` |
**Description**
@@ -317,27 +317,20 @@ API.
**Resolution**
_Unresolved — left Open; needs a design decision the resolving agent cannot make._
Re-triage 2026-05-16: confirmed against the current source — the drift is **real**.
`InboundScriptContext` (`InboundScriptExecutor.cs:188-203`) exposes only `Parameters`,
`Route`, and `CancellationToken`; there is no `Database` member, so a method script
following the documented `Database.Connection("name")` API fails to compile.
This finding cannot be closed by the InboundAPI module agent for two reasons:
1. **Scope** — the alternative resolution (deleting the "Database Access" section)
edits `docs/requirements/Component-InboundAPI.md`, which is outside the editable
scope (`src/ScadaLink.InboundAPI`, `tests/`, this file only).
2. **It is a genuine design decision.** Implementing `Database.Connection()` is not a
mechanical fix: it hands inbound API scripts a *raw* MS SQL client. The ScadaLink
script trust model (CLAUDE.md, Akka.NET conventions) forbids scripts from `System.IO`
and raw network access, and `ForbiddenApiChecker` (added for InboundAPI-005) now
statically blocks `System.Net`/`System.IO`. A raw `SqlConnection` is in clear
tension with that trust model, and the set of connection names a script may open,
read-only vs. read-write access, and connection lifetime/pooling all require a
design call. **Surface to the design owner:** decide whether `Database.Connection()`
is in scope — if yes, write a design note covering the trust-model carve-out and
then implement a `Database` member backed by a connection-factory service; if no,
delete the "Database Access" section from `Component-InboundAPI.md`.
Resolved 2026-05-16 (commit `<pending>`). The drift was confirmed real:
`InboundScriptContext` (`InboundScriptExecutor.cs:188-203`) exposes only
`Parameters`, `Route`, and `CancellationToken` — there is no `Database` member,
so a method script following the documented `Database.Connection("name")` API
would fail to compile. Resolution direction: the design doc is stale, not the
code. Implementing `Database.Connection()` would hand inbound API scripts a
*raw* MS SQL client, in direct tension with the ScadaLink script trust model
(scripts are forbidden `System.IO`, raw network, etc.; `ForbiddenApiChecker`
statically enforces this). Rather than carve a hole in the trust model, the
"Database Access" section was removed from `docs/requirements/Component-InboundAPI.md`
and replaced with an explicit "No direct database access" note explaining that
scripts interact only through the curated `Route`/`Parameters` surfaces, and
that any future data access must go behind a dedicated scoped helper added as an
explicit design change. Code and doc now agree; no code or test change required.
### InboundAPI-008 — Inbound API endpoint not restricted to the active central node