Skip to content

Commit e9aa73c

Browse files
iancooperclaude
andcommitted
fix: PR #4039 review item E4 — tighten Identifiers.AssertSafe to reject leading underscore
Identifiers.AssertSafe accepted `^[A-Za-z_][A-Za-z0-9_]*$`, allowing identifiers that start with `_`. Spanner rejects `_`-prefixed names as reserved (which is why SpannerBoxMigrationRunner's internal history table is `BrighterMigrationHistory`, not `_BrighterMigrationHistory`). A user-configured `OutBoxTableName = "_my_outbox"` would therefore pass the framework chokepoint and fail at Spanner's DDL with a less actionable "reserved identifier" error. Tighten the regex to `^[A-Za-z][A-Za-z0-9_]*$` so the strictest supported backend's rule applies to the framework-level identifier validation. Underscores remain legal in non-leading positions; only the leading character is now restricted to `[A-Za-z]`. No internal caller passes a `_`-prefixed name through AssertSafe — verified via grep of all AssertSafe call sites and all *MigrationCatalog constants — so this is a tightening, not a breaking change for downstream consumers using the standard configuration shape. TDD: flipped the existing `_underscore_leading` test case from the "safe inputs" Theory to the "unsafe inputs" Theory. RED before regex tightening (AssertSafe accepted `_underscore_leading`, no exception thrown), GREEN after. All 11 AssertSafe tests and 65 BoxProvisioning core tests pass on net9.0 and net10.0; 6 SQLite unsafe-name rejection tests pass on net9.0 (no regression in per-backend chokepoints). Updated the Identifiers <remarks> xmldoc and the exception-message regex hint so contributors and operators see the tightened rule. Review comment: #4039 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f2f9299 commit e9aa73c

2 files changed

Lines changed: 16 additions & 13 deletions

File tree

src/Paramore.Brighter.BoxProvisioning/Identifiers.cs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,19 @@ namespace Paramore.Brighter.BoxProvisioning;
3333
/// allowed set is an injection vector for any host that builds the table name from external input.
3434
/// </summary>
3535
/// <remarks>
36-
/// The allowed regex <c>^[A-Za-z_][A-Za-z0-9_]*$</c> matches the bare-identifier rules common to
36+
/// The allowed regex <c>^[A-Za-z][A-Za-z0-9_]*$</c> matches the bare-identifier rules common to
3737
/// every supported backend (MSSQL, PostgreSQL, MySQL, SQLite, Spanner) and is a strict subset of
3838
/// what each backend permits inside quoted/backticked identifiers. The over-restriction is
39-
/// deliberate: the helper rejects unusual but legal identifiers (e.g. names containing spaces or
40-
/// non-ASCII characters that would otherwise need quoting) before they reach a code path where a
41-
/// future contributor might forget to escape them.
39+
/// deliberate: the helper rejects unusual but legal identifiers (e.g. names containing spaces,
40+
/// non-ASCII characters, or a leading underscore — Spanner rejects <c>_</c>-prefixed names as
41+
/// reserved while every other backend accepts them, so the regex picks the strictest backend's
42+
/// rule to keep portable configuration safe) before they reach a code path where a future
43+
/// contributor might forget to escape them.
4244
/// </remarks>
4345
public static class Identifiers
4446
{
4547
private static readonly Regex s_safeIdentifier = new(
46-
"^[A-Za-z_][A-Za-z0-9_]*$",
48+
"^[A-Za-z][A-Za-z0-9_]*$",
4749
RegexOptions.Compiled | RegexOptions.CultureInvariant);
4850

4951
/// <summary>
@@ -53,7 +55,8 @@ public static class Identifiers
5355
/// <param name="parameterName">The name of the caller's parameter — included in the exception
5456
/// message so contributors can see which call-site rejected the value.</param>
5557
/// <exception cref="ConfigurationException">Thrown when <paramref name="identifier"/> is null,
56-
/// empty, or contains characters outside <c>[A-Za-z0-9_]</c> or starts with a digit.</exception>
58+
/// empty, or contains characters outside <c>[A-Za-z0-9_]</c>, starts with a digit, or starts
59+
/// with an underscore.</exception>
5760
public static void AssertSafe(string identifier, string parameterName)
5861
{
5962
// Split missing-configuration (null) from malformed-identifier so operators see the real
@@ -69,6 +72,6 @@ public static void AssertSafe(string identifier, string parameterName)
6972

7073
throw new ConfigurationException(
7174
$"Unsafe SQL identifier supplied for '{parameterName}': '{identifier}'. " +
72-
$"Identifiers must match the regex ^[A-Za-z_][A-Za-z0-9_]*$.");
75+
$"Identifiers must match the regex ^[A-Za-z][A-Za-z0-9_]*$.");
7376
}
7477
}

tests/Paramore.Brighter.BoxProvisioning.Tests/When_assert_safe_identifier_is_called_with_known_unsafe_inputs_it_should_throw.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,12 @@ namespace Paramore.Brighter.BoxProvisioning.Tests;
4242
public class AssertSafeIdentifierTests
4343
{
4444
[Theory]
45-
[InlineData("O'Brien")] // single quote — breaks information_schema probe at MySqlOutboxMigrationCatalog.cs:165
46-
[InlineData("Outbox; DROP")] // semicolon — statement terminator
47-
[InlineData("my-outbox")] // hyphen — invalid in bare identifiers
48-
[InlineData("1Outbox")] // leading digit — invalid as bare identifier across all backends
49-
[InlineData("")] // empty
45+
[InlineData("O'Brien")] // single quote — breaks information_schema probe at MySqlOutboxMigrationCatalog.cs:165
46+
[InlineData("Outbox; DROP")] // semicolon — statement terminator
47+
[InlineData("my-outbox")] // hyphen — invalid in bare identifiers
48+
[InlineData("1Outbox")] // leading digit — invalid as bare identifier across all backends
49+
[InlineData("")] // empty
50+
[InlineData("_underscore_leading")] // leading underscore — Spanner rejects `_`-prefixed identifiers as reserved (PR #4039 review item E4)
5051
public void When_assert_safe_identifier_is_called_with_known_unsafe_inputs_it_should_throw(string unsafeIdentifier)
5152
{
5253
//Arrange
@@ -65,7 +66,6 @@ public void When_assert_safe_identifier_is_called_with_known_unsafe_inputs_it_sh
6566
[Theory]
6667
[InlineData("Outbox")]
6768
[InlineData("my_outbox_v2")]
68-
[InlineData("_underscore_leading")]
6969
[InlineData("Outbox123")]
7070
public void When_assert_safe_identifier_is_called_with_safe_inputs_it_should_not_throw(string safeIdentifier)
7171
{

0 commit comments

Comments
 (0)