---
phase: 65.1-phase-65-code-review-remediation
plan: 07
type: execute
wave: 3
depends_on:
  - 65.1-02b
  - 65.1-01
files_modified:
  - cpp/src/shim.cpp
  - cpp/src/shim.hpp
  - src/parse.rs
  - src/ddl/alter_helpers_ffi.rs
  - test/sql/phase651_yaml_filesystem_access_gating.test
autonomous: true
requirements:
  - REM-CR-01
  - REM-IN-04

must_haves:
  truths:
    - "`sv_create_from_yaml_bind` reads the YAML file via `FileSystem::GetFileSystem(context).OpenFile(...)` rather than `Connection::Query(\"SELECT content FROM read_text(...)\")`"
    - "No `path_escaped` quote-doubling block remains in `sv_create_from_yaml_bind`"
    - "`__sv_compute_create_from_yaml` helper TF signature drops the `INTEGER kind` parameter — now 3-arg `(file_path VARCHAR, view_name VARCHAR, comment VARCHAR)`"
    - "Rust FFI `sv_compute_create_from_yaml_rust` matches the slimmed signature"
    - "`src/parse.rs::rewrite_yaml_file_create` emits a 3-arg call to `__sv_compute_create_from_yaml`"
    - "sqllogictest `phase651_yaml_filesystem_access_gating.test` asserts `enable_external_access=false` rejects `CREATE SEMANTIC VIEW v FROM YAML FILE '<path>'` while INLINE FROM YAML still succeeds"
    - "`FROM YAML FILE failed:` error prefix preserved (pinned by phase53_yaml_file.test)"
  artifacts:
    - path: cpp/src/shim.cpp
      provides: "FileSystem-direct read path in sv_create_from_yaml_bind + 3-arg helper TF body + registration block"
      contains: "FileSystem::GetFileSystem"
    - path: cpp/src/shim.hpp
      provides: "Updated sv_compute_create_from_yaml_rust extern (no kind param)"
    - path: src/parse.rs
      provides: "rewrite_yaml_file_create emits 3-arg helper TF call"
    - path: src/ddl/alter_helpers_ffi.rs
      provides: "sv_compute_create_from_yaml_rust signature without kind param"
    - path: test/sql/phase651_yaml_filesystem_access_gating.test
      provides: "Populated sqllogictest pinning enable_external_access gating under FileSystem-direct path"
      contains: "enable_external_access"
  key_links:
    - from: cpp/src/shim.cpp::sv_create_from_yaml_bind
      to: cpp/include/duckdb.hpp::FileSystem::OpenFile
      via: "direct FileSystem call replacing read_text SQL indirection"
      pattern: "OpenFile.*FILE_FLAGS_READ"
---

<objective>
Two paired changes scoped to the YAML-FILE helper TF surface:

1. CR-01 (D-01..D-03) — replace `Connection::Query("SELECT content FROM read_text('<path>')")` in `sv_create_from_yaml_bind` (`cpp/src/shim.cpp:732-775`) with direct `FileSystem::GetFileSystem(context).OpenFile(path, FileFlags::FILE_FLAGS_READ)` → `GetFileSize` → `Read`. Eliminates the SQL surface (and the `path_escaped` quote-doubling). `LocalFileSystem` continues to honour `enable_external_access` per RESEARCH Pitfall 2.

2. IN-04 (D-24) — remove the `kind: u8` parameter from `__sv_compute_create_from_yaml` and its Rust handler. The kind is redundantly encoded by the outer parser_override INSERT shape (`OR IGNORE` / `OR REPLACE` / plain).

Plus: populate `test/sql/phase651_yaml_filesystem_access_gating.test` (Plan 01 stub) with `enable_external_access=false` rejection cases.

Per CONTEXT D-02: do NOT take Prepare/bind workaround. Per CONTEXT D-03: helper TF stays named `__sv_compute_create_from_yaml`.

Wave 2 because it shares `cpp/src/shim.cpp` with Plan 02 (WR-02 ABI rework).

Implements D-01, D-02, D-03 (CR-01) + D-24 (IN-04).
</objective>

<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>

<context>
@.planning/PROJECT.md
@.planning/STATE.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-CONTEXT.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md
@cpp/src/shim.cpp
@cpp/src/shim.hpp
@src/parse.rs
@src/ddl/alter_helpers_ffi.rs
@test/sql/phase53_yaml_file.test
@test/sql/phase651_yaml_filesystem_access_gating.test
</context>

<interfaces>
CR-01 target shape (full in PATTERNS.md lines 52-95, RESEARCH.md lines 261-307):

Replace the `path_escaped` block + `Connection probe(*context.db); probe.Query("SELECT content FROM read_text(...)")` + result extraction in `sv_create_from_yaml_bind` with:

```
auto &fs = FileSystem::GetFileSystem(context);
unique_ptr<FileHandle> handle;
try {
    handle = fs.OpenFile(bd->file_path, FileFlags::FILE_FLAGS_READ);
} catch (const std::exception &e) {
    throw BinderException("FROM YAML FILE failed: " + std::string(e.what()));
}
int64_t size = fs.GetFileSize(*handle);
if (size < 0) {
    throw BinderException("FROM YAML FILE failed: GetFileSize returned -1 for '" + bd->file_path + "'");
}
std::string yaml_content(static_cast<size_t>(size), '\0');
int64_t got = fs.Read(*handle, yaml_content.data(), size);
if (got != size) {
    throw BinderException("FROM YAML FILE failed: short read (" + std::to_string(got) + " of " + std::to_string(size) + ") for '" + bd->file_path + "'");
}
```

The `FROM YAML FILE failed:` prefix is REQUIRED — pinned by `test/sql/phase53_yaml_file.test`.

IN-04 sites (RESEARCH.md lines 726-744):
- `cpp/src/shim.cpp:137-143` — FFI extern declaration: drop `uint8_t kind,`
- `cpp/src/shim.cpp:~713` `CreateFromYamlBindData` struct: drop `kind` field
- `cpp/src/shim.cpp:~729` bind callback: drop `bd->kind = input.inputs[2].GetValue<int32_t>();`
- `cpp/src/shim.cpp:~793` call to `sv_compute_create_from_yaml_rust`: drop `static_cast<uint8_t>(bd->kind)` arg
- `cpp/src/shim.cpp:~2519-2524` TF registration `arg_types[]`: drop trailing `INTEGER`; `arg_count` becomes 3
- `cpp/src/shim.hpp` matching extern: drop `uint8_t kind,`
- `src/ddl/alter_helpers_ffi.rs:~127` Rust function signature: drop `_kind: u8` parameter + any body usage
- `src/parse.rs::rewrite_yaml_file_create`: emit 3-arg call (drop the kind argument)
</interfaces>

<tasks>

<task type="auto">
  <name>Task 1: CR-01 FileSystem-direct read in sv_create_from_yaml_bind</name>
  <read_first>
    - cpp/src/shim.cpp lines 700-820 (full sv_create_from_yaml_bind + adjacent bind data struct)
    - cpp/include/duckdb.hpp lines 8205-8217 (FileSystem API signatures)
    - cpp/include/duckdb.hpp lines 7720-7858 (FileOpenFlags constants — pick `FileFlags::FILE_FLAGS_READ`)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 52-95 (target shape + error-wording preservation rule)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md lines 261-307
    - test/sql/phase53_yaml_file.test (verify which `FROM YAML FILE failed:` substrings are pinned — DO NOT break them)
  </read_first>
  <files>cpp/src/shim.cpp</files>
  <action>
1. Locate `sv_create_from_yaml_bind` (lines ~732-816). Delete the `path_escaped` quote-doubling loop entirely (the `size_t pos = 0; while ...` block building `path_escaped`).

2. Delete the `Connection probe(*context.db); std::string read_sql = "SELECT content FROM read_text(...)"; auto result = probe.Query(read_sql);` lines and the `result->HasError()` check + the `yaml_content` extraction from the result row (column 0 of row 0).

3. Replace with the `<interfaces>` target shape. Use `FileSystem::GetFileSystem(context)`, `OpenFile(bd->file_path, FileFlags::FILE_FLAGS_READ)`, `GetFileSize`, `Read`. Pass NO `FileOpener` (let it default to nullptr) — `LocalFileSystem` falls back to the global `DBConfigOptions::enable_external_access` check when no opener is provided (RESEARCH Pitfall 2).

4. Preserve the EXACT error-message prefix `"FROM YAML FILE failed: "` on every throw (4 paths: OpenFile exception, negative GetFileSize, short read, plus any downstream parse error that stays unchanged).

5. Update the docstring/comment block at the top of `sv_create_from_yaml_bind` to describe the FileSystem-direct read shape. Cite Phase 65.1 D-01..D-03.

6. Ensure `unique_ptr<FileHandle>` and `FileFlags` are reachable — both live in the `duckdb` namespace. Should already be in scope via the `using namespace duckdb;` or qualified usage elsewhere in shim.cpp; verify before adding any new includes.

7. DO NOT modify any other part of `sv_create_from_yaml_bind` — the bind data setup before this block AND the YAML parsing after stay unchanged. Surgical replacement of the read mechanism only.
  </action>
  <verify>
    <automated>just build &gt; /tmp/claude/p07_t1_build.log 2>&amp;1; RC=$?; tail -60 /tmp/claude/p07_t1_build.log; test $RC -eq 0 &amp;&amp; just test-sql &gt; /tmp/claude/p07_t1_sql.log 2>&amp;1; RC2=$?; tail -40 /tmp/claude/p07_t1_sql.log; test $RC2 -eq 0</automated>
  </verify>
  <acceptance_criteria>
    - `grep -n "FileSystem::GetFileSystem" cpp/src/shim.cpp` returns at least one match in `sv_create_from_yaml_bind`
    - `grep -n "path_escaped" cpp/src/shim.cpp` returns zero matches inside the YAML-file bind function
    - `grep -n "SELECT content FROM read_text" cpp/src/shim.cpp` returns zero matches anywhere in the file
    - All `throw BinderException(...)` paths in the function carry the `"FROM YAML FILE failed: "` prefix
    - `just build` exits 0
    - `just test-sql` exits 0 — phase53_yaml_file.test still green
  </acceptance_criteria>
  <done>CR-01 closed: FileSystem-direct read replaces SQL-injection surface; error wording preserved.</done>
</task>

<task type="auto">
  <name>Task 2: IN-04 remove kind parameter from helper TF + FFI + bind + registration</name>
  <read_first>
    - cpp/src/shim.cpp around lines 137-143 (FFI extern), 700-820 (bind callback), 2519-2524 (TF registration)
    - cpp/src/shim.hpp (matching extern declaration for sv_compute_create_from_yaml_rust)
    - src/ddl/alter_helpers_ffi.rs around line 120-140 (Rust handler with `_kind: u8`)
    - src/parse.rs (grep for `rewrite_yaml_file_create` — emits the helper TF call)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-CONTEXT.md (D-24)
  </read_first>
  <files>cpp/src/shim.cpp, cpp/src/shim.hpp, src/ddl/alter_helpers_ffi.rs, src/parse.rs</files>
  <action>
Five paired edits — all must land or the build breaks:

1. `cpp/src/shim.hpp` — `sv_compute_create_from_yaml_rust` extern: drop `uint8_t kind,`.
2. `cpp/src/shim.cpp:137-143` — matching FFI extern at file scope: drop `uint8_t kind,`.
3. `cpp/src/shim.cpp` bind callback:
   - Remove `kind` field from `CreateFromYamlBindData` struct (~line 713)
   - Remove `bd->kind = input.inputs[2].GetValue<int32_t>();` line (~729)
   - Remove `static_cast<uint8_t>(bd->kind)` argument from the `sv_compute_create_from_yaml_rust(...)` call (~793)
4. `cpp/src/shim.cpp:~2519-2524` TF registration — `arg_types[]` currently has 4 entries `{VARCHAR, VARCHAR, VARCHAR, INTEGER}`; drop the trailing `INTEGER`; `arg_count` becomes 3. Helper TF becomes `(file_path VARCHAR, view_name VARCHAR, comment VARCHAR)`.
5. `src/ddl/alter_helpers_ffi.rs:~127` — drop `_kind: u8` from function signature; remove any `let _ = _kind;` body line.
6. `src/parse.rs::rewrite_yaml_file_create` — locate via grep. It builds a SQL string like `SELECT * FROM __sv_compute_create_from_yaml('<file>', '<name>', '<comment>', <kind>)`. Drop the `<kind>` argument so the emitted call is 3-arg. Trace `kind` back through the function and remove the parameter/local if unused after that edit.

Edit ordering: do C side first (shim.hpp + shim.cpp), then Rust side (alter_helpers_ffi.rs + parse.rs). The intermediate build will fail on Rust extern mismatch — that's expected; the Rust edit clears it. Use CLAUDE.md Rule 1 (redirect to file then tail).
  </action>
  <verify>
    <automated>cargo build &gt; /tmp/claude/p07_t2_build.log 2>&amp;1; RC=$?; tail -60 /tmp/claude/p07_t2_build.log; test $RC -eq 0 &amp;&amp; just build &gt; /tmp/claude/p07_t2_jbuild.log 2>&amp;1; RC2=$?; tail -40 /tmp/claude/p07_t2_jbuild.log; test $RC2 -eq 0 &amp;&amp; just test-sql &gt; /tmp/claude/p07_t2_sql.log 2>&amp;1; RC3=$?; tail -50 /tmp/claude/p07_t2_sql.log; test $RC3 -eq 0</automated>
  </verify>
  <acceptance_criteria>
    - `grep -n "uint8_t kind" cpp/src/shim.cpp cpp/src/shim.hpp` returns zero matches in sv_compute_create_from_yaml_rust extern, in the bind struct, and in the call site
    - `grep -n "_kind: u8" src/ddl/alter_helpers_ffi.rs` returns zero matches
    - `rewrite_yaml_file_create` emits a 3-arg call (no fourth arg after the comment string)
    - TF registration `arg_types[]` has exactly 3 entries (all VARCHAR)
    - `just build` and `just test-sql` exit 0 — existing FROM YAML FILE tests still pass with 3-arg call shape
  </acceptance_criteria>
  <done>IN-04 closed: helper TF slimmed to 3 args; FFI surface reduced; tests green.</done>
</task>

<task type="auto">
  <name>Task 3: Populate phase651_yaml_filesystem_access_gating.test sqllogictest</name>
  <read_first>
    - test/sql/phase651_yaml_filesystem_access_gating.test (Plan 01 stub)
    - test/sql/phase53_yaml_file.test (template for FROM YAML FILE assertion shape including any `statement error` block-form patterns)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md (CR-01 enable_external_access section + Pitfall 2 — confirm LocalFileSystem denial wording)
    - CLAUDE.md (block-form `statement error` requirement; inline regex unsupported)
  </read_first>
  <files>test/sql/phase651_yaml_filesystem_access_gating.test</files>
  <action>
Replace the Plan 01 stub body. The populated test must:

1. Keep the SQL-comment header naming Plan 07 and decisions D-01..D-03.
2. Setup statements:
   - `CREATE TABLE t1 (id INTEGER);`
   - `SET enable_external_access = false;`
3. `statement error` block (block form, not inline — CLAUDE.md):
   ```
   statement error
   CREATE SEMANTIC VIEW v_blocked FROM YAML FILE '/tmp/nonexistent_phase651.yaml'
   ----
   FROM YAML FILE failed:
   ```
   Substring matching only on the stable `FROM YAML FILE failed:` prefix preserved by Task 1.

4. POSITIVE CONTROL: `SET enable_external_access = true;` then attempt the same DDL against a path created in the same test via `COPY (SELECT 'metric: count(*)' AS s) TO '<tmp>/v_ok.yaml' (FORMAT 'csv', HEADER false, QUOTE '')` — if this is too convoluted for sqllogictest, skip and add `# NOTE: positive control deferred to Python integration test (none required for CR-01 gate)`.

5. Add a THIRD case: with `enable_external_access = false` still in effect, `CREATE SEMANTIC VIEW v_inline FROM YAML 'metric: count(*)'` (the INLINE form) — MUST succeed because no file I/O. Confirms gating is FILE-variant only.

If a real YAML fixture file is needed, reference any used by `phase53_yaml_file.test`.
  </action>
  <verify>
    <automated>just test-sql &gt; /tmp/claude/p07_t3_sql.log 2>&amp;1; RC=$?; tail -50 /tmp/claude/p07_t3_sql.log; test $RC -eq 0 &amp;&amp; grep -q "enable_external_access" test/sql/phase651_yaml_filesystem_access_gating.test &amp;&amp; grep -q "FROM YAML FILE failed:" test/sql/phase651_yaml_filesystem_access_gating.test</automated>
  </verify>
  <acceptance_criteria>
    - `test/sql/phase651_yaml_filesystem_access_gating.test` contains at least one `statement error` block matching `FROM YAML FILE failed:`
    - Test sets `enable_external_access = false` before the assertion
    - INLINE `FROM YAML 'metric: count(*)'` case succeeds even under the false gating
    - `just test-sql` exits 0
  </acceptance_criteria>
  <done>CR-01 access-gating regression test in place; FileSystem-direct path proven to honour enable_external_access.</done>
</task>

</tasks>

<threat_model>
## Trust Boundaries

| Boundary | Description |
|----------|-------------|
| User-supplied YAML file path | DDL accepts arbitrary path; must respect enable_external_access gating |

## STRIDE Threat Register

| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-65.1-07-01 | Tampering | SQL injection via interpolated path in `read_text('<path>')` | mitigate | Replace SQL surface with FileSystem::OpenFile (D-01) — no string-interpolation into SQL remains |
| T-65.1-07-02 | Elevation of privilege | Bypassing enable_external_access by switching to FileSystem-direct | mitigate | Sqllogictest pins false-gating rejection; LocalFileSystem honours the global DBConfigOption (RESEARCH Pitfall 2) |
| T-65.1-07-03 | Information disclosure | Path leaked verbatim in error message could surface filesystem layout | accept | Error messages already contain the path on parse-failure; this change preserves the same disclosure shape, no regression |
</threat_model>

<verification>
  <automated>just test-all &gt; /tmp/claude/p07_all.log 2>&amp;1; RC=$?; tail -100 /tmp/claude/p07_all.log; test $RC -eq 0</automated>
</verification>

<success_criteria>
- `sv_create_from_yaml_bind` uses FileSystem API; no SQL `read_text` indirection
- Helper TF is 3-arg `(file_path, view_name, comment)`
- Rust FFI matches the slimmed signature
- `rewrite_yaml_file_create` emits 3-arg call
- `phase651_yaml_filesystem_access_gating.test` populated with `enable_external_access=false` rejection
- `just test-all` green
</success_criteria>

<output>
Create `.planning/phases/65.1-phase-65-code-review-remediation/65.1-07-SUMMARY.md` when done.
</output>
