🐛 Fix marker parser misinterpreting number-ish strings as numeric literals

pkg/crd/schema.go

@@ -424,6 +424,7 @@ func mapToSchema(ctx *schemaContext, mapType *ast.MapType) *apiextensionsv1.JSON
 	}
 
 	return &apiextensionsv1.JSONSchemaProps{
+		//nolint:goconst // this is a constant, but it's more readable to have it here
 		Type: "object",
 		AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{
 			Schema: valSchema,

pkg/markers/parse.go

@@ -265,7 +265,7 @@ func guessType(scanner *sc.Scanner, raw string, allowSlice bool) *Argument {
 		maybeItem := guessType(scanner, raw, false)
 
 		subRaw := raw[scanner.Pos().Offset:]
-		subScanner := parserScanner(subRaw, scanner.Error)
+		subScanner := parserScanner(subRaw, func(*sc.Scanner, string) {})
 
 		var tok rune
 		for {
@@ -291,7 +291,8 @@ func guessType(scanner *sc.Scanner, raw string, allowSlice bool) *Argument {
 	// (so we don't consume our scanner tokens until we actually
 	// go to use this -- Go doesn't like scanners that can be rewound).
 	subRaw := raw[scanner.Pos().Offset:]
-	subScanner := parserScanner(subRaw, scanner.Error)
+	var hadScanError bool
+	subScanner := parserScanner(subRaw, func(*sc.Scanner, string) { hadScanError = true })
 
 	// skip whitespace
 	hint := peekNoSpace(subScanner)
@@ -316,7 +317,7 @@ func guessType(scanner *sc.Scanner, raw string, allowSlice bool) *Argument {
 			// might be a map or slice, parse the string and check for colon
 			// (blech, basically arbitrary look-ahead due to raw strings).
 			var keyVal string // just ignore this
-			(&Argument{Type: StringType}).parseString(subScanner, raw, reflect.Indirect(reflect.ValueOf(&keyVal)))
+			(&Argument{Type: StringType}).parseString(subScanner, raw, reflect.Indirect(reflect.ValueOf(&keyVal)), true)
 
 			if token := subScanner.Scan(); token == ':' || hint == '}' {
 				// it's got a string followed by a colon -- it's a map
@@ -361,11 +362,22 @@ func guessType(scanner *sc.Scanner, raw string, allowSlice bool) *Argument {
 			nextTok = subScanner.Scan()
 		}
 
-		if nextTok == sc.Int {
-			return &Argument{Type: IntType}
-		}
-		if nextTok == sc.Float {
-			return &Argument{Type: NumberType}
+		// Only treat as a numeric type if:
+		// 1. The scanner did not emit errors while tokenising (e.g. invalid
+		//    octal "087bdd" produces sc.Int but also triggers an error).
+		// 2. The token is immediately followed by a delimiter (comma,
+		//    semicolon, EOF) or whitespace-then-delimiter, meaning there is
+		//    no trailing non-numeric content.  Inputs like "123/config",
+		//    "100Mi", "2024-01-15" start with a valid numeric token but
+		//    contain additional characters and must be treated as strings.
+		if !hadScanError && (nextTok == sc.Int || nextTok == sc.Float) {
+			followTok := peekNoSpace(subScanner)
+			if followTok == ',' || followTok == ';' || followTok == sc.EOF || followTok == '}' {
+				if nextTok == sc.Int {
+					return &Argument{Type: IntType}
+				}
+				return &Argument{Type: NumberType}
+			}
 		}
 	}
 
@@ -374,7 +386,8 @@ func guessType(scanner *sc.Scanner, raw string, allowSlice bool) *Argument {
 }
 
 // parseString parses either of the two accepted string forms (quoted, or bare tokens).
-func (a *Argument) parseString(scanner *sc.Scanner, raw string, out reflect.Value) {
+// When inMap is true, ':' is treated as a delimiter (map key separator).
+func (a *Argument) parseString(scanner *sc.Scanner, raw string, out reflect.Value, inMap bool) {
 	// we need to temporarily disable the scanner's int/float parsing, since we want to
 	// prevent number parsing errors.
 	oldMode := scanner.Mode
@@ -399,10 +412,16 @@ func (a *Argument) parseString(scanner *sc.Scanner, raw string, out reflect.Valu
 	}
 
 	// the "hard" case -- bare tokens not including ',' (the argument
-	// separator), ';' (the slice separator), ':' (the map separator), or '}'
-	// (delimitted slice ender)
+	// separator), ';' (the slice separator), or '}' (delimitted slice ender).
+	// ':' (the map separator) is only a delimiter when parsing inside a map.
+	isDelim := func(hint rune) bool {
+		if hint == ',' || hint == ';' || hint == '}' || hint == sc.EOF {
+			return true
+		}
+		return inMap && hint == ':'
+	}
 	startPos := scanner.Position.Offset
-	for hint := peekNoSpace(scanner); hint != ',' && hint != ';' && hint != ':' && hint != '}' && hint != sc.EOF; hint = peekNoSpace(scanner) {
+	for hint := peekNoSpace(scanner); !isDelim(hint); hint = peekNoSpace(scanner) {
 		// skip this token
 		scanner.Scan()
 	}
@@ -470,7 +489,7 @@ func (a *Argument) parseMap(scanner *sc.Scanner, raw string, out reflect.Value)
 	}
 
 	for hint := peekNoSpace(scanner); hint != '}' && hint != sc.EOF; hint = peekNoSpace(scanner) {
-		a.parseString(scanner, raw, key)
+		a.parseString(scanner, raw, key, true)
 		if !expect(scanner, ':', "colon") {
 			return
 		}
@@ -566,7 +585,7 @@ func (a *Argument) parse(scanner *sc.Scanner, raw string, out reflect.Value, inS
 		// strings are a bit weird -- the "easy" case is quoted strings (tokenized as strings),
 		// the "hard" case (present for backwards compat) is a bare sequence of tokens that aren't
 		// a comma.
-		a.parseString(scanner, raw, out)
+		a.parseString(scanner, raw, out, false)
 	case BoolType:
 		if !expect(scanner, sc.Ident, "true or false") {
 			return

pkg/markers/parse_test.go

@@ -219,6 +219,16 @@ var _ = Describe("Parsing", func() {
 				},
 			}
 			It("should support non-uniform, nested maps (and always guess as such)", argParseTestCase{arg: anyArg, raw: `{text: "abc", len: 3, "as bytes": 97;98;99, props: {encoding: ascii, nullsafe: true, tags: {"triple", "in a row"}}}`, output: complexMap}.Run)
+
+			It("should parse number-ish strings with invalid octal prefix as string", argParseTestCase{arg: anyArg, raw: `087bdd`, output: "087bdd"}.Run)
+			It("should parse path-like strings as string", argParseTestCase{arg: anyArg, raw: `/tmp/tmp.4paHkdEcpK`, output: "/tmp/tmp.4paHkdEcpK"}.Run)
+			It("should parse numeric path as string", argParseTestCase{arg: anyArg, raw: `123/config`, output: "123/config"}.Run)
+			It("should parse invalid octal number as string", argParseTestCase{arg: anyArg, raw: `0987`, output: "0987"}.Run)
+			It("should parse resource quantity as string", argParseTestCase{arg: anyArg, raw: `100Mi`, output: "100Mi"}.Run)
+			It("should parse date-like string as string", argParseTestCase{arg: anyArg, raw: `2024-01-15`, output: "2024-01-15"}.Run)
+			It("should parse SHA prefix as string", argParseTestCase{arg: anyArg, raw: `sha256:08a7b`, output: "sha256:08a7b"}.Run)
+			It("should parse UUID-like string as string", argParseTestCase{arg: anyArg, raw: `00000000-1234-5678`, output: "00000000-1234-5678"}.Run)
+			It("should parse alphanumeric starting with number as string", argParseTestCase{arg: anyArg, raw: `123abc`, output: "123abc"}.Run)
 		})
 	})
 })

pkg/version/version_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package version //nolint:revive
+package version
 
 import (
 	. "github.com/onsi/ginkgo"