Skip to content

fix(c8y): strip control characters from the operation results - #4412

Open
reubenmiller wants to merge 6 commits into
thin-edge:mainfrom
reubenmiller:fix/c8y-strip-control-chars-from-operation-results
Open

reubenmiller wants to merge 6 commits into
thin-edge:mainfrom
reubenmiller:fix/c8y-strip-control-chars-from-operation-results

Conversation

@reubenmiller

@reubenmiller reubenmiller commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Proposed changes

A c8y_Command operation whose output contains control characters, e.g. the terminal color escape sequences printed by many CLI tools, stays in the EXECUTING state forever. Cumulocity rejects the 506 status update with 41,506,Message [...] contains invalid characters on c8y/s/e, which the mapper doesn't act on, while the command has already been cleared on the device. A minimal reproduction is the command printf 'a\033b'.

This PR:

  • strips the control characters, along with the terminal escape sequences they introduce, from the operation results and failure reasons sent to Cumulocity. This is done by the SmartREST serializer, for every operation reporting a result or a failure reason, including the SmartREST custom operations run by the mapper. CSV results are stripped field by field, once parsed, so delimiters and quotes are kept. sanitize_for_smartrest no longer removes control characters, and is renamed truncate_for_smartrest. The mapper no longer truncates the output of a SmartREST custom operation before the stripping, so the escape sequences don't count toward the size limit
  • sets NO_COLOR=1 and CLICOLOR=0 for the commands run by tedge-shell-plugin, so the tools following either convention don't print colors in the first place

The system tests were pushed first, on their own, and failed on the CI as expected: the operations stayed in the EXECUTING state.

The new system tests cover the c8y_Command operation:

  • a colored output, which is reported without the escape sequences
  • the minimal reproduction printf 'a\033b'
  • other control characters, which are removed while tabs and new lines are kept
  • a colored failure reason
  • the NO_COLOR and CLICOLOR variables set for the command

The SmartREST custom operations with a CSV result are covered by a mapper unit test, with an escape character followed by a delimiter.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
  • I ran just format as mentioned in CODING_GUIDELINES
  • I used just check as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

a\033b is reported as a, since ESC b is a valid two-character escape sequence. An unterminated control string, e.g. a window title missing its terminator, is removed up to the end of its line, so a stray escape character cannot remove the following lines.

🤖 Generated with Claude Code

@reubenmiller
reubenmiller deployed to Test Pull Request November 10, 2026 06:24 — with GitHub Actions Active
@reubenmiller reubenmiller added the theme:troubleshooting Theme: Troubleshooting and remote control label Nov 10, 2026
@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.47826% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/extensions/c8y_mapper_ext/src/converter.rs 42.85% 3 Missing and 1 partial ⚠️
crates/core/c8y_api/src/smartrest/message.rs 97.67% 0 Missing and 1 partial ⚠️
...core/c8y_api/src/smartrest/smartrest_serializer.rs 97.22% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
1064 0 8 1064 100 3h15m22.261493s

reubenmiller and others added 2 commits November 10, 2026 21:39
Cumulocity rejects an operation status update containing control
characters, e.g. the terminal color escape sequences, leaving the
operation in the EXECUTING state.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: reubenmiller <[email protected]>
Cumulocity rejects an operation status update containing control
characters, which left a c8y_Command operation printing colors in the
EXECUTING state.

- Remove the control characters, along with the terminal escape
  sequences they introduce, from the operation results and failure
  reasons sent to Cumulocity.
- Set NO_COLOR=1 and CLICOLOR=0 for the commands run by
  tedge-shell-plugin, so the tools following either convention don't
  print colors in the first place.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: reubenmiller <[email protected]>
@reubenmiller
reubenmiller force-pushed the fix/c8y-strip-control-chars-from-operation-results branch from 8a0303c to 14ef115 Compare November 10, 2026 11:39
@reubenmiller
reubenmiller deployed to Test Pull Request November 10, 2026 11:39 — with GitHub Actions Active
@reubenmiller
reubenmiller marked this pull request as ready for review November 10, 2026 11:43

Removes the terminal escape sequences from the failure reason
${operation}= Cumulocity.Execute Shell Command
... printf '\\033[31merror\\033[0m\\n' >&2; exit 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[AI] suggestion: Open an issue for a failure reason that loses its error text to an escape-only last line.

tedge-shell-plugin sets the reason to Command returned exit code <N>: <last line>, after trim_end() on the whole output. So printf 'foo\nbar\n\n'; exit 1 gives the reason ...: bar.
In this test, \033[0m comes before the last newline. When \033[0m comes after, as in printf '\033[31merror\n\033[0m' >&2; exit 1, the last line is only \033[0m. fail_operation strips the \033[0m line, and the reason ends with exit code 1: . The PR does not cause the loss: on main, Cumulocity rejected the unstripped ESC, and the operation stayed EXECUTING.

@rina23q rina23q left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved

This branch was successfully deployed

2 active deployments
Test Auto — 5813cd54 Deployed Nov 11, 2026 by reubenmiller via Test x86_64 #8888
Test Pull Request — 5813cd54 Deployed Nov 11, 2026 by reubenmiller via Approve #8888
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme:troubleshooting Theme: Troubleshooting and remote control

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants