Skip to content

fix: reject leading-zero semver values in local evaluation #206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dmarticus merged 5 commits into from
May 22, 2026
Merged

fix: reject leading-zero semver values in local evaluation #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60a4f1b
Reject semver values with leading zeros in local evaluation
dmarticus May 20, 2026
9e241c6
fix: address review feedback on semver leading-zero rejection
dmarticus May 20, 2026
a6b0aae
chore: add changeset for semver leading-zero fix
dmarticus May 21, 2026
36f2a9f
fix: address haacked review feedback on semver leading-zero rejection
dmarticus May 22, 2026
4d1d37c
Merge branch 'main' into dmarticus/strict-semver-leading-zeros
dmarticus May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/strict-semver-leading-zeros.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"PostHog": patch
---

Reject semver values with leading zeros in local flag evaluation. Per semver 2.0.0 §2, numeric identifiers must not include leading zeros — values like `1.07.3` are not valid semver and should not match targeting conditions. Both override values and flag values are now validated; invalid inputs cause `SemanticVersion.TryParse` to return false so the condition does not match.
39 changes: 26 additions & 13 deletions src/PostHog/Library/SemanticVersion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Diagnostics.CodeAnalysis;
using System.Globalization;

namespace PostHog.Library;

Expand Down Expand Up @@ -47,6 +48,7 @@ public SemanticVersion(int major, int minor, int patch)
/// 5. Default missing components to 0 (e.g., "1.2" → (1, 2, 0), "1" → (1, 0, 0))
/// 6. Ignore extra components beyond the third (e.g., "1.2.3.4" → (1, 2, 3))
/// 7. Return false for truly invalid input (empty string, non-numeric parts, leading dot)
/// 8. Reject components with leading zeros per semver 2.0.0 §2 (e.g., "01.02.03")
/// </remarks>
public static bool TryParse(string? value, [NotNullWhen(returnValue: true)] out SemanticVersion? version)
{
Expand Down Expand Up @@ -110,33 +112,44 @@ public static bool TryParse(string? value, [NotNullWhen(returnValue: true)] out
var parts = trimmed.Split('.');

// Parse major (required)
if (!int.TryParse(parts[0], out var major))
if (!TryParseSemverNumericIdentifier(parts[0], out var major))
{
return false;
}

// Parse minor (optional, defaults to 0)
var minor = 0;
if (parts.Length > 1 && !string.IsNullOrEmpty(parts[1]) && !int.TryParse(parts[1], out minor))
if (parts.Length > 1 && !string.IsNullOrEmpty(parts[1]) && !TryParseSemverNumericIdentifier(parts[1], out minor))
{
return false;
}

// Parse patch (optional, defaults to 0)
var patch = 0;
if (parts.Length > 2 && !string.IsNullOrEmpty(parts[2]) && !int.TryParse(parts[2], out patch))
if (parts.Length > 2 && !string.IsNullOrEmpty(parts[2]) && !TryParseSemverNumericIdentifier(parts[2], out patch))
{
return false;
}

// Reject negative version components
if (major < 0 || minor < 0 || patch < 0)
version = new SemanticVersion(major, minor, patch);
return true;
}

static bool TryParseSemverNumericIdentifier(string part, out int value)
{
value = 0;
if (string.IsNullOrEmpty(part))
{
return false;
}

version = new SemanticVersion(major, minor, patch);
return true;
// Semver 2.0.0 §2: literal "0" is allowed; "01", "00", "001" are not.
if (part.Length > 1 && part[0] == '0')
{
return false;
}
// NumberStyles.None + InvariantCulture rejects signs, whitespace, and non-ASCII digits.

@haacked haacked May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: The deletion of the explicit if (major < 0 || minor < 0 || patch < 0) check is safe only because NumberStyles.None rejects sign characters during parse. That coupling is load-bearing and invisible. The existing comment names the behavior; spelling out the dependency keeps the next person from "tightening" to NumberStyles.Integer and silently re-opening negative-component acceptance.

Suggested change
// NumberStyles.None + InvariantCulture rejects signs, whitespace, and non-ASCII digits.
// NumberStyles.None + InvariantCulture rejects signs, whitespace, and non-ASCII digits.
// The sign rejection is what lets TryParse drop the explicit negative-component check above.

// The sign rejection is what lets TryParse drop the explicit negative-component check.
return int.TryParse(part, NumberStyles.None, CultureInfo.InvariantCulture, out value);
}

/// <summary>
Expand Down Expand Up @@ -252,7 +265,7 @@ public static bool TryParseWildcard(
{
// "X" pattern - treat as "X.*"
// Bare wildcards like "*" and non-numeric values are invalid
if (!int.TryParse(parts[0], out var major))
if (!TryParseSemverNumericIdentifier(parts[0], out var major))
{
return false;
}
Expand All @@ -264,7 +277,7 @@ public static bool TryParseWildcard(
else if (parts.Length == 2)
{
// "X.Y" or "X.*" pattern
if (!int.TryParse(parts[0], out var major))
if (!TryParseSemverNumericIdentifier(parts[0], out var major))
{
return false;
}
Expand All @@ -277,7 +290,7 @@ public static bool TryParseWildcard(
return true;
}

if (!int.TryParse(parts[1], out var minor))
if (!TryParseSemverNumericIdentifier(parts[1], out var minor))
{
return false;
}
Expand All @@ -290,12 +303,12 @@ public static bool TryParseWildcard(
else if (parts.Length >= 3)
{
// "X.Y.Z" or "X.Y.*" pattern
if (!int.TryParse(parts[0], out var major))
if (!TryParseSemverNumericIdentifier(parts[0], out var major))
{
return false;
}

if (!int.TryParse(parts[1], out var minor))
if (!TryParseSemverNumericIdentifier(parts[1], out var minor))
{
return false;
}
Expand Down
42 changes: 35 additions & 7 deletions tests/UnitTests/Features/LocalEvaluatorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2289,7 +2289,6 @@ public void HandlesSemverWildcardMinorOperator(string overrideValue, ComparisonO
[InlineData("1.2.3-alpha", ComparisonOperator.SemverEquals, "1.2.3", true)] // Pre-release stripped
[InlineData("1.2.3+build", ComparisonOperator.SemverEquals, "1.2.3", true)] // Build metadata stripped
[InlineData(" 1.2.3 ", ComparisonOperator.SemverEquals, "1.2.3", true)] // Whitespace stripped
[InlineData("01.02.03", ComparisonOperator.SemverEquals, "1.2.3", true)] // Leading zeros
[InlineData("1.2", ComparisonOperator.SemverEquals, "1.2.0", true)] // Partial version
[InlineData("1", ComparisonOperator.SemverEquals, "1.0.0", true)] // Partial version
[InlineData("1.2.3.4", ComparisonOperator.SemverEquals, "1.2.3", true)] // Extra parts ignored
Expand Down Expand Up @@ -2329,6 +2328,14 @@ public void HandlesSpecialVersionFormats(string overrideValue, ComparisonOperato
[InlineData(".1.2.3", ComparisonOperator.SemverEquals)]
[InlineData("not-a-version", ComparisonOperator.SemverGreaterThan)]
[InlineData("", ComparisonOperator.SemverGreaterThan)]
// Leading-zero override values are invalid per semver 2.0.0 §2
[InlineData("01.02.03", ComparisonOperator.SemverEquals)]
[InlineData("1.07.3", ComparisonOperator.SemverEquals)]
[InlineData("01.02.03", ComparisonOperator.SemverNotEquals)]
[InlineData("1.07.3", ComparisonOperator.SemverGreaterThan)]
[InlineData("01.2.3", ComparisonOperator.SemverGreaterThanOrEquals)]
[InlineData("1.2.03", ComparisonOperator.SemverLessThan)]
[InlineData("1.07.3", ComparisonOperator.SemverLessThanOrEquals)]
public void ThrowsInconclusiveMatchExceptionForInvalidOverrideVersion(string overrideValue, ComparisonOperator comparison)
{
var flags = CreateFlags(
Expand Down Expand Up @@ -2358,11 +2365,28 @@ public void ThrowsInconclusiveMatchExceptionForInvalidOverrideVersion(string ove

[Theory]
// Invalid version in filter value - should throw InconclusiveMatchException
[InlineData("not-a-version")]
[InlineData("")]
[InlineData("abc.def.ghi")]
[InlineData(".1.2.3")]
public void ThrowsInconclusiveMatchExceptionForInvalidFilterVersion(string filterValue)
[InlineData("not-a-version", ComparisonOperator.SemverEquals)]
[InlineData("", ComparisonOperator.SemverEquals)]
[InlineData("abc.def.ghi", ComparisonOperator.SemverEquals)]
[InlineData(".1.2.3", ComparisonOperator.SemverEquals)]

@haacked haacked May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: The signature change to this test opens the door to running each invalid value under every operator, but the four pre-existing rows ("not-a-version", "", "abc.def.ghi", ".1.2.3") only get SemverEquals. The override-version twin test above already pairs "not-a-version" and "" with SemverGreaterThan. Mirror that here so the non-equals operators stay covered for non-leading-zero malformed input:

[InlineData("not-a-version", ComparisonOperator.SemverGreaterThan)]
[InlineData("", ComparisonOperator.SemverGreaterThan)]
[InlineData("abc.def.ghi", ComparisonOperator.SemverTilde)]
[InlineData(".1.2.3", ComparisonOperator.SemverCaret)]

// Mirror the override-version test: non-leading-zero malformed inputs must also fail
// under non-equals operators, not just SemverEquals.
[InlineData("not-a-version", ComparisonOperator.SemverGreaterThan)]
[InlineData("", ComparisonOperator.SemverGreaterThan)]
[InlineData("abc.def.ghi", ComparisonOperator.SemverTilde)]
[InlineData(".1.2.3", ComparisonOperator.SemverCaret)]
// Leading-zero filter values are invalid per semver 2.0.0 §2, across all operators
[InlineData("01.02.03", ComparisonOperator.SemverEquals)]
[InlineData("1.07.3", ComparisonOperator.SemverEquals)]
[InlineData("01.2.3", ComparisonOperator.SemverEquals)]
[InlineData("1.2.03", ComparisonOperator.SemverEquals)]
[InlineData("1.07.3", ComparisonOperator.SemverGreaterThan)]
[InlineData("01.2.3", ComparisonOperator.SemverGreaterThanOrEquals)]
[InlineData("1.2.03", ComparisonOperator.SemverLessThan)]
[InlineData("01.02.03", ComparisonOperator.SemverLessThanOrEquals)]
[InlineData("1.07.3", ComparisonOperator.SemverTilde)]
[InlineData("01.2.3", ComparisonOperator.SemverCaret)]
public void ThrowsInconclusiveMatchExceptionForInvalidFilterVersion(string filterValue, ComparisonOperator comparison)
{
var flags = CreateFlags(
key: "version",
Expand All @@ -2372,7 +2396,7 @@ public void ThrowsInconclusiveMatchExceptionForInvalidFilterVersion(string filte
Type = FilterType.Person,
Key = "app_version",
Value = new PropertyFilterValue(filterValue),
Operator = ComparisonOperator.SemverEquals
Operator = comparison
}
]
);
Expand All @@ -2394,6 +2418,10 @@ public void ThrowsInconclusiveMatchExceptionForInvalidFilterVersion(string filte
[InlineData("*")]
[InlineData("1.2.3")] // Not a wildcard pattern
[InlineData("abc.*")]
// Leading-zero wildcard patterns are invalid per semver 2.0.0 §2
[InlineData("01.*")]
[InlineData("1.02.*")]
[InlineData("01.2.*")]
public void ThrowsInconclusiveMatchExceptionForInvalidWildcardPattern(string filterValue)
{
var flags = CreateFlags(
Expand Down
31 changes: 27 additions & 4 deletions tests/UnitTests/Library/SemanticVersionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,12 @@ public void IgnoresExtraComponents(string input, int expectedMajor, int expected
}

[Theory]
// Leading zeros are parsed as integers
[InlineData("01.02.03", 1, 2, 3)]
[InlineData("001.002.003", 1, 2, 3)]
public void ParsesLeadingZeros(string input, int expectedMajor, int expectedMinor, int expectedPatch)
// Literal "0" components are valid per semver 2.0.0
[InlineData("0.0.1", 0, 0, 1)]
[InlineData("0.1.0", 0, 1, 0)]
[InlineData("1.0.0", 1, 0, 0)]
[InlineData("1.2.0", 1, 2, 0)]
public void ParsesLiteralZeroComponents(string input, int expectedMajor, int expectedMinor, int expectedPatch)
{
var result = SemanticVersion.TryParse(input, out var version);

Expand Down Expand Up @@ -139,6 +141,19 @@ public void ParsesLeadingZeros(string input, int expectedMajor, int expectedMino
[InlineData("1.-2.3")] // Negative minor
[InlineData("-1.2.3")] // Negative major
[InlineData("1.2.-3")] // Negative patch
[InlineData("01.02.03")] // Leading zeros (all components)

@haacked haacked May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: TryParseSemverNumeric relies on NumberStyles.None + InvariantCulture to reject signs, embedded whitespace, and non-ASCII digits, plus on int.TryParse returning false on overflow. The new leading-zero rows exercise the explicit part[0] == '0' guard; the rest of the helper's contract is only covered by the pre-existing -1.2.3/1.-2.3 sign cases. If someone later swaps in int.Parse(part) or relaxes the styles, every present test still passes. Three rows close the gap:

[InlineData("1. 2.3")]              // Embedded whitespace in minor (NumberStyles.None)
[InlineData("١.2.3")]              // Arabic-Indic digit in major (InvariantCulture)
[InlineData("99999999999999.0.0")]  // Overflows int.MaxValue

[InlineData("001.002.003")] // Multiple leading zeros
[InlineData("1.07.3")] // Leading zero in minor
[InlineData("1.2.03")] // Leading zero in patch
[InlineData("01.2.3")] // Leading zero in major
[InlineData("00.0.0")] // Leading zero on a zero major
[InlineData("v01.2.3")] // Leading zero with v-prefix

@haacked haacked May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: The new ReturnsFalseForInvalidInput rows cover bare leading-zero triples and "v01.2.3", but nothing pins the contract that the leading-zero check survives the upstream pre-release, build-metadata, and outer-whitespace strips. If a future change reorders TryParse so the leading-zero check runs before those strips, today's tests still pass while "01.2.3-alpha" silently goes back to being accepted as 1.2.3. A few rows lock the composition in:

[InlineData("01.2.3-alpha")]      // Leading zero + pre-release
[InlineData("01.2.3+build")]      // Leading zero + build metadata
[InlineData("  01.2.3  ")]        // Leading zero + outer whitespace

[InlineData("01.2.3-alpha")] // Leading zero + pre-release
[InlineData("01.2.3+build")] // Leading zero + build metadata
[InlineData(" 01.2.3 ")] // Leading zero + outer whitespace
[InlineData("1. 2.3")] // Embedded whitespace in minor (NumberStyles.None rejects)
[InlineData("١.2.3")] // Arabic-Indic digit in major (InvariantCulture rejects)
[InlineData("99999999999999.0.0")] // Overflows int.MaxValue
public void ReturnsFalseForInvalidInput(string? input)
{
var result = SemanticVersion.TryParse(input, out var version);
Expand Down Expand Up @@ -470,6 +485,14 @@ public void ParsesImplicitMinorWildcard(string pattern, string expectedLower, st
[InlineData("1.2.3.*")] // Too specific
[InlineData(".1.*")] // Leading dot
[InlineData("abc.*")] // Non-numeric
[InlineData("01.*")] // Leading zero in major
[InlineData("1.02.*")] // Leading zero in minor
[InlineData("01.2.*")] // Leading zero in major (X.Y.*)
[InlineData("01")] // Leading zero in implicit major
[InlineData("01.2")] // Leading zero in implicit X.Y
[InlineData("1.02")] // Leading zero in implicit minor

@haacked haacked May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: ReturnsFalseForInvalidPatterns covers leading zeros in major/minor and the implicit X/X.Y forms but is missing two cases that ReturnsFalseForInvalidInput does cover for TryParse: a v-prefixed leading zero and a multiple-leading-zero variant. Both paths go through TryParseSemverNumeric, so behavior should already be correct, but the symmetry protects against someone later short-circuiting the wildcard path or moving the strip-v logic:

[InlineData("v01.*")]    // Leading zero with v-prefix
[InlineData("001.*")]    // Multiple leading zeros

[InlineData("v01.*")] // Leading zero with v-prefix
[InlineData("001.*")] // Multiple leading zeros
public void ReturnsFalseForInvalidPatterns(string? pattern)
{
var result = SemanticVersion.TryParseWildcard(pattern, out var lower, out var upper);
Expand Down
Loading