Skip to content

[cvl]: Escape Lua metacharacters in delete-dependency and must/when filters - #249

Open
gupta-08 wants to merge 2 commits into
sonic-net:masterfrom
gupta-08:cvl-lua-filter-escaping
Open

gupta-08 wants to merge 2 commits into
sonic-net:masterfrom
gupta-08:cvl-lua-filter-escaping

Conversation

@gupta-08

@gupta-08 gupta-08 commented Aug 31, 2026

Copy link
Copy Markdown

Description of PR

Summary:

CVL interpolates Redis keys and YANG values into generated Lua and into
libyang3 list-key path predicates. Values that contain ', \, or %
can break those strings, so delete-dependency lookup, must/when evaluation,
and list-node creation fail or match the wrong entries.

This change:

  • Escapes \ and ' before interpolating keys into delete-dependency Lua
    filters in GetDepDataForDelete().
  • Uses plain string.find (plain=true) so % is not treated as a Lua
    pattern character in leaf-list matching.
  • Escapes the same characters when building must/when Lua predicates in the
    xpath engine patch.
  • Quotes libyang3 list-key predicates the way libyang does: single quotes
    unless the value itself contains ', then double quotes
    (formatLyPredicateValue() used by AddListNode()).

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
  • Test case improvement

Back port request

  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605

Tested branch

  • master
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • N/A

Test result

Added / updated CVL unit tests:

  • TestGetDepDataForDeleteSpecialKeyChars — keys containing ', \, %
  • TestGetDepDataForDeleteLeafListPatternChars — leaf-list values with Lua
    pattern characters
  • TestValidateEditConfig_Must_Predicate_Escape_SingleQuote
  • TestValidateEditConfig_Must_Predicate_Escape_Backslash
  • TestValidateEditConfig_When_Predicate_Escape_SingleQuote
  • TestValidateEditConfig_When_Predicate_Escape_Backslash

Approach

What is the motivation for this PR?

GetDepDataForDelete() embeds Redis key values into a Lua script using
fmt.Sprintf and string.find. Unescaped quotes break the Lua string, and
unescaped % is treated as a pattern. must/when predicates had the same
issue when config values were written into Lua predicate strings.

AddListNode() always wrapped list-key values in single quotes
([key='value']). Libyang3 uses double quotes when the value contains ',
so keys with a single quote fail path parsing.

That can cause:

  • failed or incomplete dependent-delete discovery
  • incorrect must/when validation for values that contain ', \, or %
  • failed list-node create when a key value contains '

How did you do it?

In cvl/cvl_api.go (GetDepDataForDelete()):

  • Escape \ and ' in the key before building the Lua filter script.
  • Call string.find(..., 1, true) so the search is a plain string match.

In patches/xpath.patch:

  • Escape \ and ' when assigning SFilter.Predicate from config values
    (currentQuery, constantQuery, and childQuery evaluation).

In cvl/internal/yparser/ly_path.go and cvl/internal/yparser/yparser.go:

  • Add formatLyPredicateValue() and use it in AddListNode() so list-key
    predicates match libyang quoting ('value' or "value").

No change to Redis data; only generated Lua and libyang path strings are
sanitized.

How did you verify/test it?

CVL Go unit tests covering:

  • delete-dependency filters with special characters in keys and leaf-lists
  • must/when predicates whose values contain ' or \

Existing TestGetDepDataForDelete coverage is unchanged.

Any platform specific information?

No. Generic CVL / Lua / libyang3 path behavior.

Supported testbed topology if it's a new test case?

N/A

Documentation

N/A

…ilters

Escape quotes/backslashes in Redis Lua filter keys and must/when predicates
so special characters cannot break Lua scripts.

Signed-off-by: Aditi Gupta <rasheegupta.11@gmail.com>
@linux-foundation-easycla

linux-foundation-easycla Bot commented Aug 31, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: gupta-08 / name: Aditi Gupta (a78cb8a)

@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Libyang uses single quotes unless the value itself contains a single
quote. Always wrapping keys in single quotes breaks path parsing for
those values.

Signed-off-by: Aditi Gupta <rasheegupta.11@gmail.com>
@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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.

2 participants