Skip to content

Fix start-anchored validation and escaping in ddb lexer and codeartifact npm login - #10544

Open
Adityaj0 wants to merge 2 commits into
aws:v2from
Adityaj0:fix/lexer-escaping-and-validation
Open

Fix start-anchored validation and escaping in ddb lexer and codeartifact npm login#10544
Adityaj0 wants to merge 2 commits into
aws:v2from
Adityaj0:fix/lexer-escaping-and-validation

Conversation

@Adityaj0

Copy link
Copy Markdown

Fixes #10543.

Four defects, all of the same shape: a validation or unescaping step that only handles the start of a string.

1. ddb: base64 literals were validated with re.match

re.match anchors only at the beginning, so b"AAAA!!!!" passed validation. base64.b64decode discards out-of-alphabet characters by default, so the literal then decoded to a silently truncated value and was written to DynamoDB with no error. The comment above the check says this is precisely what the manual validation exists to prevent. Now uses fullmatch.

The check did work when the invalid characters came first (b"!!!!AAAA" was correctly rejected), which is likely why it went unnoticed.

# before                                    # after
data = b"AAAA!!!!"                          data = b"AAAA!!!!"
  -> Binary(b'\x00\x00\x00')                  -> LexerError: Invalid base64 string

2. ddb: escaped backslashes were never unescaped

_consume_until preserved \X verbatim and the callers only stripped \" / \', so "C:\\path" lexed to C:\\path rather than C:\path, with no way to express a single literal backslash.

Unescaping now happens while consuming the string. A backslash escaping neither the delimiter nor another backslash is still preserved verbatim, so sequences like \n are unchanged.

3. ddb: token end was a length rather than an offset

_consume_quoted_identifier / _consume_string_literal set 'end': token_len. exceptions.py computes token['end'] - token['start'] to size the ^^^ underline in error messages, so for any quoted token not at offset 0 this went negative and the underline was mis-sized. For bar = "foo" the literal reported start=6, end=4.

_next deliberately leaves _position on the final character at end of input, so a small _offset() helper covers the case where the closing quote is the last character. I left _next itself alone — changing it also shifts the caret position in several existing lexer error tests, which is a separate judgement call and didn't belong in this change.

4. codeartifact: npm scope name was validated with a start-anchored regex

NpmLogin.get_scope used '^(@[a-z0-9-~][a-z0-9-._~]*)', so validation stopped at the first character outside the allowed set and accepted everything after it. Namespaces such as foo bar, foo!!!, or one containing a newline were passed through to npm config set as a scope name. Now anchored with \A/\Z, matching how SwiftLogin.get_scope in the same file already validates its scope.

The existing test only covered an invalid leading character, which the start-anchored pattern did catch; added coverage for invalid characters elsewhere in the name.

Testing

  • 11 new test cases. I confirmed each one fails against the unpatched code and passes with the fix — the source changes were stashed and the new tests re-run to verify they were not vacuous.
  • tests/unit/customizations passes: 3265 passed, 1 skipped. (One unrelated pre-existing failure, test_which_with_existing_command, which looks for a python binary on PATH — this machine only has python3.)
  • ruff output on the touched files is unchanged from baseline.
  • Changelog entries added with scripts/new-change.

I did not run the full tests/functional suite locally, only the codeartifact functional tests.

Happy to split the codeartifact fix into its own PR if you'd prefer to keep the components separate — it's an independent commit.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Adityaj0 and others added 2 commits August 11, 2026 02:33
Three defects in the ``aws ddb`` expression lexer:

* Base64 literals were validated with ``re.match``, which only anchors
  at the start of the string.  A literal such as ``b"AAAA!!!!"`` passed
  validation, and because ``base64.b64decode`` discards characters
  outside the alphabet by default, it then decoded to a silently
  truncated value.  The surrounding comment states this is exactly what
  the manual validation is meant to prevent, so validate the whole
  string with ``fullmatch``.

* An escaped backslash inside a quoted string or identifier was
  preserved as two characters, so ``"C:\\path"`` lexed to ``C:\\path``
  rather than ``C:\path``.  Unescaping now happens while consuming the
  string; a backslash that escapes neither the delimiter nor another
  backslash is still preserved verbatim, as before.

* The ``end`` offset recorded for quoted tokens was a token *length*
  rather than an offset, so ``end - start`` went negative and the
  ``^^^`` underline in expression error messages was mis-sized.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
``NpmLogin.get_scope`` validated the scope with a regex anchored only at
the start, so validation stopped at the first character outside the
allowed set and everything after it was accepted.  Namespaces such as
``foo bar``, ``foo!!!`` or one containing a newline were passed through
to ``npm config set`` as a scope name.

Anchor the pattern at both ends with ``\A``/``\Z``, matching how
``SwiftLogin.get_scope`` already validates its scope.

The existing test only covered an invalid *leading* character, which the
start-anchored pattern did catch; add coverage for invalid characters
elsewhere in the name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Adityaj0
Adityaj0 requested a review from a team as a code owner August 11, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant