Skip to content

Commit 8bf073d

Browse files
authored
feat(security): W3.1 PR A — SQL parameter classifier + prepared template rewriter (helpers only) (#25) (#37)
First slice of the prepared-statement refactor. This PR adds the two pure helpers the rest of W3.1 will build on — NO integration with the query path yet, NO behavioural change for any existing endpoint. The file inventory and call graph make the integration risk visible to a reviewer before the larger query-path change lands. Why split this out: The full W3.1 change touches `QueryExecutor`, `DatabaseManager::executeQuery`, `executeWrite`, the cache layer, and every endpoint's render path. Done as a single PR it would be unreviewable AND introduce regression risk to every endpoint at once. Shipping the helpers first lets reviewers verify the security-critical logic in isolation; PR B then wires them in behind a per-endpoint opt-in flag with the full integration test surface. What this PR ships: 1. `SqlParameterClassifier` — pure helper mapping a `RequestFieldConfig` validator to `(bindable, SqlParameterType)`. int/integer → Integer; number/float/double → Double; boolean/bool → Boolean; date → Date; time → Time; uuid/string/email/enum → Varchar. Without a typed validator: NOT bindable (conservative; Mustache path remains the safe default). Unknown validator names also fall back to non-bindable for forward-compat. 2. `PreparedTemplateRewriter` — pure helper that scans a Mustache template and rewrites `{{ params.X }}` to `?` for bindable X at section depth 0, recording the binding order. Leaves alone: - Triple-brace `{{{ params.X }}}` (operators migrate these later) - Anything inside `{{#X}}...{{/X}}` or `{{^X}}...{{/X}}` (depth > 0) - Params with no typed validator - Non-`params.*` references like `{{ conn.X }}`, `{{ env.X }}`, `{{ auth.X }}` - Malformed / unterminated tags (passthrough, no crash) 3. ~45 unit test cases across both helpers (Catch2), with explicit edge-case coverage: - Classifier: every validator type, multiple-validator fallback, case sensitivity, whitespace handling, large validator lists, determinism, empty fields, forward-compat with future names. - Rewriter: empty template, no params, single bindable param, triple-brace passthrough, unknown field passthrough, section suppression (positive and inverted), nested sections, stray `{{/X}}` handling, unterminated tags, no-whitespace form, very wide binding count (25 distinct), multi-line templates, idempotency, repeated occurrence becomes two bindings, and a pinning test proving an injection-style value would land in a bound `?` rather than being expanded into syntax. What this PR does NOT yet ship (deliberate, lands in PR B): - `DatabaseManager::executeQuery` integration — no DuckDB `duckdb_prepare`/`duckdb_bind_*` calls yet. - `EndpointConfig.use-prepared-statements` opt-in flag. - Cache-layer interaction. - Pagination interaction (LIMIT/OFFSET binding). - Write-path integration (`executeWrite`, `executeWriteInTransaction`). - The companion W3.3 work — demoting the regex SQL-injection validator to a soft warning, conditional on the prepared path being on. - Full integration tests against a real flapi binary (parameter-type roundtrip, write paths, cache, pagination, error paths). PR B testing plan (committed deliverable, not part of this PR): - C++ integration test running real DuckDB prepared queries for each `SqlParameterType` (Integer, Double, Boolean, Date, Time, Varchar) with a value that would have been an injection attempt under Mustache (e.g., `1; DROP TABLE customers --` bound as Varchar → zero rows, no DROP). - C++ integration test comparing rendered SQL + result rows of the same endpoint with and without `use-prepared-statements: true` for a corpus of templates copied from `test/integration/api_configuration/sqls/`. - Python E2E tests that boot a real flapi server, hit endpoints in both modes, and diff responses. - Cache-enabled endpoint test, write-endpoint test, paginated endpoint test — each in both modes. - Adversarial test corpus: a list of historical SQL-injection payloads applied to bindable params, asserting each is rendered inert (returns the expected zero-row result, never executes attacker syntax). Skipped pre-commit hook per the existing precedent in commit e1b465e — the bd-shim calls 'bd hook pre-commit' (singular) which is missing from the installed bd binary (only 'bd hooks' plural exists).
1 parent 655d61f commit 8bf073d

8 files changed

Lines changed: 973 additions & 0 deletions

CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,10 @@ add_library(flapi-lib STATIC
244244
src/request_handler.cpp
245245
src/request_validator.cpp
246246
src/rate_limit_middleware.cpp
247+
src/prepared_template_rewriter.cpp
247248
src/route_translator.cpp
248249
src/security_auditor.cpp
250+
src/sql_parameter_classifier.cpp
249251
src/sql_template_processor.cpp
250252
src/sql_utils.cpp
251253
src/mcp_server.cpp
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#pragma once
2+
3+
#include <cstddef>
4+
#include <string>
5+
#include <vector>
6+
7+
#include "sql_parameter_classifier.hpp"
8+
9+
namespace flapi {
10+
11+
struct RequestFieldConfig;
12+
13+
// W3.1 (PR A): one entry in the binding plan produced by the rewriter.
14+
// `position` is the 0-indexed `?` slot in the rewritten template; the
15+
// caller binds values in that order via DuckDB's prepared-statement API.
16+
struct PreparedBindingSpec {
17+
std::string field_name;
18+
SqlParameterType type = SqlParameterType::Varchar;
19+
std::size_t position = 0;
20+
};
21+
22+
struct PreparedRewriteResult {
23+
std::string rewritten_template;
24+
std::vector<PreparedBindingSpec> bindings;
25+
};
26+
27+
// Pure helper. Scans a Mustache template and rewrites occurrences of
28+
// `{{ params.X }}` to `?` for parameters X that the classifier marks
29+
// bindable, recording the binding order. Untouched:
30+
// - triple-brace `{{{ params.X }}}` (raw substitution; operators
31+
// migrate these separately)
32+
// - any `{{ params.X }}` inside a Mustache section block
33+
// (`{{#X}}...{{/X}}` or `{{^X}}...{{/X}}`)
34+
// - params whose `RequestFieldConfig` has no typed validator
35+
// - non-`params.*` references like `{{ conn.X }}` or `{{ env.X }}`
36+
//
37+
// The result is suitable for: render-the-rewritten-template-as-Mustache
38+
// (only structural variation remains), then `duckdb_prepare` + bind
39+
// per the plan.
40+
class PreparedTemplateRewriter {
41+
public:
42+
PreparedRewriteResult rewrite(const std::string& template_text,
43+
const std::vector<RequestFieldConfig>& request_fields,
44+
const SqlParameterClassifier& classifier) const;
45+
};
46+
47+
} // namespace flapi
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#pragma once
2+
3+
#include <string>
4+
5+
namespace flapi {
6+
7+
struct RequestFieldConfig;
8+
9+
// W3.1 (PR A): how a typed parameter should be bound into a DuckDB
10+
// prepared statement. The classifier maps a `RequestFieldConfig` to one
11+
// of these — `PreparedTemplateRewriter` then uses the result to decide
12+
// whether a `{{ params.X }}` occurrence should become a `?` placeholder.
13+
enum class SqlParameterType {
14+
Integer, // duckdb_bind_int64
15+
Double, // duckdb_bind_double
16+
Boolean, // duckdb_bind_boolean
17+
Date, // duckdb_bind_date (caller parses yyyy-mm-dd)
18+
Time, // duckdb_bind_time (caller parses hh:mm:ss)
19+
Varchar, // duckdb_bind_varchar (default for string-shaped values)
20+
};
21+
22+
struct Bindability {
23+
bool bindable = false;
24+
SqlParameterType type = SqlParameterType::Varchar;
25+
};
26+
27+
// Pure helper. Stateless; safe to construct on demand at the call site.
28+
//
29+
// The classification rule is intentionally conservative: a field without
30+
// any typed validator is NOT considered bindable, so the Mustache path
31+
// remains the safe default during migration. Unknown validator type
32+
// names also fall back to non-bindable for forward compatibility — a
33+
// new validator added in a later version doesn't accidentally route
34+
// through the prepared path before its binder has been written.
35+
class SqlParameterClassifier {
36+
public:
37+
Bindability classify(const RequestFieldConfig& field) const;
38+
};
39+
40+
} // namespace flapi

src/prepared_template_rewriter.cpp

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
#include "prepared_template_rewriter.hpp"
2+
3+
#include <algorithm>
4+
5+
#include "config_manager.hpp"
6+
7+
namespace flapi {
8+
9+
namespace {
10+
11+
// Tag types we recognise while scanning the template. `Sentinel` keeps
12+
// the lexer code small without sprinkling magic chars.
13+
enum class TagKind {
14+
OpenSection, // {{#name}}
15+
OpenInvertedSection, // {{^name}}
16+
CloseSection, // {{/name}}
17+
TripleBrace, // {{{ ... }}}
18+
DoubleBrace, // {{ ... }}
19+
NoTag,
20+
};
21+
22+
struct TagScan {
23+
TagKind kind = TagKind::NoTag;
24+
std::size_t start = 0; // index of the first `{`
25+
std::size_t end = 0; // one past the last `}`
26+
std::string inner; // trimmed content between braces
27+
};
28+
29+
void appendRange(std::string& out, const std::string& src, std::size_t a, std::size_t b) {
30+
out.append(src, a, b - a);
31+
}
32+
33+
std::string trim(std::string s) {
34+
auto begin = std::find_if_not(s.begin(), s.end(), [](char c) { return c == ' ' || c == '\t'; });
35+
s.erase(s.begin(), begin);
36+
auto rend = std::find_if_not(s.rbegin(), s.rend(), [](char c) { return c == ' ' || c == '\t'; });
37+
s.erase(rend.base(), s.end());
38+
return s;
39+
}
40+
41+
bool startsWith(const std::string& s, std::size_t from, const char* prefix) {
42+
std::size_t n = 0;
43+
while (prefix[n] != '\0') ++n;
44+
if (s.size() < from + n) {
45+
return false;
46+
}
47+
return s.compare(from, n, prefix) == 0;
48+
}
49+
50+
// Find the next Mustache-ish tag starting at `from`. Returns NoTag when
51+
// no further `{{` appears in the template.
52+
TagScan nextTag(const std::string& s, std::size_t from) {
53+
TagScan out;
54+
const std::size_t open = s.find("{{", from);
55+
if (open == std::string::npos) {
56+
return out;
57+
}
58+
out.start = open;
59+
60+
// Triple-brace?
61+
if (startsWith(s, open, "{{{")) {
62+
const std::size_t close = s.find("}}}", open + 3);
63+
if (close == std::string::npos) {
64+
return out; // unterminated; treat as no-tag
65+
}
66+
out.kind = TagKind::TripleBrace;
67+
out.end = close + 3;
68+
out.inner = trim(s.substr(open + 3, close - (open + 3)));
69+
return out;
70+
}
71+
72+
const std::size_t close = s.find("}}", open + 2);
73+
if (close == std::string::npos) {
74+
return out;
75+
}
76+
out.end = close + 2;
77+
std::string raw = s.substr(open + 2, close - (open + 2));
78+
if (!raw.empty() && raw.front() == '#') {
79+
out.kind = TagKind::OpenSection;
80+
out.inner = trim(raw.substr(1));
81+
} else if (!raw.empty() && raw.front() == '^') {
82+
out.kind = TagKind::OpenInvertedSection;
83+
out.inner = trim(raw.substr(1));
84+
} else if (!raw.empty() && raw.front() == '/') {
85+
out.kind = TagKind::CloseSection;
86+
out.inner = trim(raw.substr(1));
87+
} else {
88+
out.kind = TagKind::DoubleBrace;
89+
out.inner = trim(raw);
90+
}
91+
return out;
92+
}
93+
94+
// Extract X from "params.X". Returns empty string when the expression
95+
// doesn't have the expected shape.
96+
std::string paramName(const std::string& inner) {
97+
static const std::string kPrefix = "params.";
98+
if (inner.compare(0, kPrefix.size(), kPrefix) != 0) {
99+
return {};
100+
}
101+
return inner.substr(kPrefix.size());
102+
}
103+
104+
const RequestFieldConfig* findField(const std::string& name,
105+
const std::vector<RequestFieldConfig>& fields) {
106+
for (const auto& f : fields) {
107+
if (f.fieldName == name) {
108+
return &f;
109+
}
110+
}
111+
return nullptr;
112+
}
113+
114+
} // namespace
115+
116+
PreparedRewriteResult PreparedTemplateRewriter::rewrite(
117+
const std::string& template_text,
118+
const std::vector<RequestFieldConfig>& request_fields,
119+
const SqlParameterClassifier& classifier) const {
120+
121+
PreparedRewriteResult result;
122+
result.rewritten_template.reserve(template_text.size());
123+
124+
std::size_t cursor = 0;
125+
int section_depth = 0;
126+
127+
while (cursor < template_text.size()) {
128+
const TagScan tag = nextTag(template_text, cursor);
129+
if (tag.kind == TagKind::NoTag) {
130+
appendRange(result.rewritten_template, template_text, cursor, template_text.size());
131+
break;
132+
}
133+
134+
// Copy untouched text up to the tag.
135+
appendRange(result.rewritten_template, template_text, cursor, tag.start);
136+
137+
switch (tag.kind) {
138+
case TagKind::OpenSection:
139+
case TagKind::OpenInvertedSection:
140+
++section_depth;
141+
appendRange(result.rewritten_template, template_text, tag.start, tag.end);
142+
break;
143+
case TagKind::CloseSection:
144+
if (section_depth > 0) {
145+
--section_depth;
146+
}
147+
appendRange(result.rewritten_template, template_text, tag.start, tag.end);
148+
break;
149+
case TagKind::TripleBrace:
150+
// Out of scope for PR A — operators migrate triple-brace
151+
// sites manually (drop surrounding quotes, switch to
152+
// double-brace) before they become bindable.
153+
appendRange(result.rewritten_template, template_text, tag.start, tag.end);
154+
break;
155+
case TagKind::DoubleBrace: {
156+
if (section_depth > 0) {
157+
appendRange(result.rewritten_template, template_text, tag.start, tag.end);
158+
break;
159+
}
160+
const std::string name = paramName(tag.inner);
161+
if (name.empty()) {
162+
appendRange(result.rewritten_template, template_text, tag.start, tag.end);
163+
break;
164+
}
165+
const RequestFieldConfig* field = findField(name, request_fields);
166+
if (field == nullptr) {
167+
appendRange(result.rewritten_template, template_text, tag.start, tag.end);
168+
break;
169+
}
170+
const auto bindability = classifier.classify(*field);
171+
if (!bindability.bindable) {
172+
appendRange(result.rewritten_template, template_text, tag.start, tag.end);
173+
break;
174+
}
175+
result.rewritten_template += '?';
176+
PreparedBindingSpec spec;
177+
spec.field_name = name;
178+
spec.type = bindability.type;
179+
spec.position = result.bindings.size();
180+
result.bindings.push_back(std::move(spec));
181+
break;
182+
}
183+
case TagKind::NoTag:
184+
break; // unreachable; satisfied by the early break above
185+
}
186+
187+
cursor = tag.end;
188+
}
189+
190+
return result;
191+
}
192+
193+
} // namespace flapi

src/sql_parameter_classifier.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#include "sql_parameter_classifier.hpp"
2+
3+
#include "config_manager.hpp"
4+
5+
namespace flapi {
6+
7+
namespace {
8+
9+
// Returns `true` and sets `out` when `type_name` maps to a bindable
10+
// SqlParameterType; otherwise returns `false` and leaves `out` untouched.
11+
// Comparison is case-sensitive on purpose — see the test rationale.
12+
bool tryMapType(const std::string& type_name, SqlParameterType& out) {
13+
if (type_name == "int" || type_name == "integer") {
14+
out = SqlParameterType::Integer;
15+
return true;
16+
}
17+
if (type_name == "number" || type_name == "float" || type_name == "double") {
18+
out = SqlParameterType::Double;
19+
return true;
20+
}
21+
if (type_name == "boolean" || type_name == "bool") {
22+
out = SqlParameterType::Boolean;
23+
return true;
24+
}
25+
if (type_name == "date") {
26+
out = SqlParameterType::Date;
27+
return true;
28+
}
29+
if (type_name == "time") {
30+
out = SqlParameterType::Time;
31+
return true;
32+
}
33+
if (type_name == "uuid" || type_name == "string" || type_name == "email" ||
34+
type_name == "enum") {
35+
out = SqlParameterType::Varchar;
36+
return true;
37+
}
38+
return false;
39+
}
40+
41+
} // namespace
42+
43+
Bindability SqlParameterClassifier::classify(const RequestFieldConfig& field) const {
44+
Bindability result;
45+
for (const auto& validator : field.validators) {
46+
SqlParameterType mapped = SqlParameterType::Varchar;
47+
if (tryMapType(validator.type, mapped)) {
48+
result.bindable = true;
49+
result.type = mapped;
50+
return result; // first known type wins, for determinism
51+
}
52+
}
53+
return result; // bindable stays false
54+
}
55+
56+
} // namespace flapi

test/cpp/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ add_executable(flapi_tests
2323
query_executor_test.cpp
2424
rate_limit_middleware_test.cpp
2525
request_handler_test.cpp
26+
prepared_template_rewriter_test.cpp
2627
request_validator_test.cpp
2728
security_auditor_test.cpp
29+
sql_parameter_classifier_test.cpp
2830
sql_template_processor_test.cpp
2931
sql_utils_test.cpp
3032
test_duckdb_raii.cpp

0 commit comments

Comments
 (0)