Bug #19173
closedWon't fix: Incoherent escaping between Technique Editor and Rudder Technique, and unexpected reports or behaviour in 6.X
Description
This issue won't be fixed in Rudder 6.x, as it would break existing installation of Rudder
However, it is fixed in Rudder 7.0, meaning that upgrades from 6.X to 7.0 may break values by escaping string escaped by hand
Using multiple "\" in a class parameter leads to inconsistent behaviour
Technique editor¶
Number of \ | Value | File edited | State of edition | Report | State of report |
---|---|---|---|---|---|
1 | /tmp/escapedfilebyfileonlyone\s | /tmp/escapedfilebyfileonlyone\s | OK | /tmp/escapedfilebyfileonlyone\s | OK |
2 | /tmp/escapedfilebyfiletwice\\s | /tmp/escapedfilebyfiletwice\s | NOK | /tmp/escapedfilebyfiletwice\s | NOK |
3 | /tmp/escapedfilebyfile\\\s | /tmp/escapedfilebyfile\\s | NOK | /tmp/escapedfilebyfile\\s | NOK |
and you get the idea for the rest
There is no escaping here, causing the \\ to be replaced by \
Content in file, or commands is also unescaped, so \\ are replaced by \
Rudder built-in techniques¶
Used with Technique Check generic file content
Number of \ | Value | File edited | State of edition | Report | State of report |
---|---|---|---|---|---|
1 | /tmp/file\sonce | /tmp/file\sonce | OK | /tmp/file\sonce | OK |
2 | /tmp/file\\stwice | /tmp/file\\stwice | OK | /tmp/file\\stwice | OK |
3 | /tmp/file\\\sthrice | 'file\\\sthrice' | OK | /tmp/escapedfilebyfile\\\s | OK |
There is escaping here, with \\ being replaced by \\\\, and so it ends up correct both in the file name and in the report
Updated by Nicolas CHARLES over 3 years ago
database entry looks surprising, as the \ are doubled
"Command execution","values":["sed -i 's#next if .*issuer_cn.*Let.*Encrypt.*#next if ($info->{\\x27issuer_o\\x27} !~ /Let\\x27s\\s\\\\\\s+Encrypt/i);#g' /tmp/test"],"unexpanded":["sed -i 's#next if .*issuer_cn.*Let.*Encrypt.*#next if ($info->{\\x27issuer_o\\x27} !~ /Let\\x27s\\s\\\\\\s+Encrypt/i);#g' /tmp/test"]}]}]},
report is not ok, as a \ is missing
282792118 | 2021-04-21 10:30:42.413451+02 | root | b94d4f41-10ea-4467-ada9-1acb62f73719 | be2f3386-0404-4a83-aff6-9ce76a92b070 | 0 | Command execution | sed -i 's#next if .*issuer_cn.*Let.*Encrypt.*#next if ($info->{\x27issuer_o\x27} !~ /Let\x27s\s\\s+Encrypt/i);#g' /tmp/test | 2021-04-21 10:30:38+02 | result_repaired | Command execution | Execute the command sed -i 's#next if .*issuer_cn.*Let.*Encrypt.*#next if ($info->{\x27issuer_o\x27} !~ /Let\x27s\s\\s+Encrypt/i);#g' /tmp/test was repaired
output from agent has the missing \
rudder info: Executing 'no timeout' ... 'sed -i 's#Let.*Encrypt.*#Lets\x27s\s\\sEncrypt/i);#g
but generated code is
command_execution("sed -i 's#Let.*Encrypt.*#Lets\\x27s\\s\\\sEncrypt/i);#g' /tmp/test")
Updated by Nicolas CHARLES over 3 years ago
- Subject changed from Unexpected report when using complex sed command to Unexpected report when using command execution with \\
- Description updated (diff)
Updated by François ARMAND over 3 years ago
- Severity set to Minor - inconvenience | misleading | easy workaround
- User visibility set to Operational - other Techniques | Rudder settings | Plugins
- Priority changed from 0 to 32
So, the problem is that bash (or shell) replace \\
into \
and so the value reported by cfengine is the one with only one \
. There is no simple way to change that appart from having a special case in expected report when we deal with command execution.
This does not breaks anything but impact compliance, so I put it in "misleading", but perhaps it more grave than what I understand now.
Updated by François ARMAND over 3 years ago
- Status changed from New to In progress
- Assignee set to François ARMAND
Updated by François ARMAND over 3 years ago
Work in progess here: https://github.com/fanf/rudder/commit/053e6517f8249781d73115f17086c2e5bbb94006
Updated by François ARMAND over 3 years ago
- Status changed from In progress to New
- Assignee deleted (
François ARMAND)
Updated by Nicolas CHARLES over 3 years ago
- Subject changed from Unexpected report when using command execution with \\ to Incoherent escaping between Technique Editor and Rudder Technique, and unexpected reports or behaviour
- Description updated (diff)
Updated by Nicolas CHARLES over 3 years ago
- change the value of class parameter in _method_reporting_context call before the generic method to escape it
- edit the 20/log_rudder.cf log rudder_mode method to replace the use of class_parameter with ${report_data.component_key}
This will fix the reporting, but the content will still be incorrect - people will need to properly escape their content
Updated by François ARMAND over 3 years ago
- Severity changed from Minor - inconvenience | misleading | easy workaround to Major - prevents use of part of Rudder | no simple workaround
- User visibility changed from Operational - other Techniques | Rudder settings | Plugins to Getting started - demo | first install | Technique editor and level 1 Techniques
- Priority changed from 32 to 70
Updated by François ARMAND over 3 years ago
- Severity changed from Major - prevents use of part of Rudder | no simple workaround to Critical - prevents main use of Rudder | no workaround | data loss | security
- Priority changed from 70 to 94
Setting it to critical, as it may lead to major outage (and executing commands is a main use case for rudder)
Updated by Vincent MEMBRÉ over 3 years ago
- Target version changed from 6.1.13 to 6.1.14
Updated by Vincent MEMBRÉ over 3 years ago
- Target version changed from 6.1.14 to 6.1.15
- Priority changed from 94 to 91
Updated by Vincent MEMBRÉ over 3 years ago
- Target version changed from 6.1.15 to 6.1.16
Updated by Félix DALLIDET over 3 years ago
- Priority changed from 91 to 89
I had a similar issue recently, in a 6.2.9, if I create a technique via the technique editor using the File lines present
method and want to ensure that the line \\"x
is present, the technique editor does accept the input, but the generation will break with the following error:
⇨ Policy update error for process '8' at 2021-08-27 09:50:16 ⇨ Cannot write nodes configuration ⇨ Unexpected: Error when executing hooks: Exit code=1 for hook: '/opt/rudder/etc/hooks.d/policy-generation-node-ready/10-cf-promise-check'. [stdout: error: There are syntax errors in policy files ][stderr:/var/rudder/cfengine-community/inputs.new/testing/1.0/technique.cf:13:109: error: syntax error "File lines present_${report_data.directive_id}_0" usebundle => file_lines_present("/tmp/my_file", "\\"x"), ...] (for node(s) root)
The json escape it like such:
{ "name":"lines", "value":"\\\\\"x" }
And in the cf file: file_lines_present("/tmp/my_file", "\\"x")
While in 7.0, it works perfectly. The same technique done via the technique editor does not break at generation time, and render the proper string in the target file.
Json generated by the technique editor was:
{ "name":"lines", "value":"\\\\\"x" }
And the cf file:"61986e26-f5d9-429b-8ffa-3cffb9befc72" usebundle => file_lines_present("/tmp/my_file", "\\\\\"x");
While the 7.0 took the exact same string as written in the json, the 6.2 seems to interpret the json while reading it somehow.
Updated by Nicolas CHARLES over 3 years ago
if technique is compiled with rudderc, then \\"x works as expected
but if it falls back to previous method, policy generation fails in 7.0
Updated by Vincent MEMBRÉ over 3 years ago
- Target version changed from 6.1.16 to 6.1.17
Updated by Nicolas CHARLES over 3 years ago
- Subject changed from Incoherent escaping between Technique Editor and Rudder Technique, and unexpected reports or behaviour to Won't fix: Incoherent escaping between Technique Editor and Rudder Technique, and unexpected reports or behaviour in 6.X
- Description updated (diff)
- Priority changed from 89 to 88
Updated by Vincent MEMBRÉ about 3 years ago
- Target version changed from 6.1.17 to 6.1.18
- Priority changed from 88 to 85
Updated by Vincent MEMBRÉ about 3 years ago
- Target version changed from 6.1.18 to 6.1.19
- Priority changed from 85 to 84
Updated by François ARMAND almost 3 years ago
- Status changed from New to Resolved
- Priority changed from 84 to 82
It's corrected in 7.0, and as explained won't be corrected in 6.x.