starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Custom script does not correctly escape log output for HTML display #5876

New Issue
Closed
opened 2025-12-29 19:33:44 +01:00 by adam · 8 comments
adam commented 2025-12-29 19:33:44 +01:00
Owner
Copy Link

Originally created by @candlerb on GitHub (Jan 4, 2022).

NetBox version

v3.1.4

Python version

3.8

Steps to Reproduce

Write a custom script which uses log_xxx and outputs < characters. e.g.

from extras.scripts import Script, StringVar

class Foo(Script):
    abc = StringVar(required=False)
    name = "Foo"

    def run(self, data, commit):
        self.log_info(f'Type of abc is {type(data["abc"])}')

Expected Behavior

Script to log and display:

Type of abc is <class 'str'>

Observed Behavior

image

Oddly, something is going on. if I change the script to

        self.log_info(f'Type of abc is <b>grr</b>')

then I get "Type of abc is grr" (without the bold). Maybe there's some dubious regexp-type sanitisation instead of proper HTML escaping going on.

Originally created by @candlerb on GitHub (Jan 4, 2022). ### NetBox version v3.1.4 ### Python version 3.8 ### Steps to Reproduce Write a custom script which uses `log_xxx` and outputs `<` characters. e.g. ``` from extras.scripts import Script, StringVar class Foo(Script): abc = StringVar(required=False) name = "Foo" def run(self, data, commit): self.log_info(f'Type of abc is {type(data["abc"])}') ``` ### Expected Behavior Script to log and display: ``` Type of abc is <class 'str'> ``` ### Observed Behavior ![image](https://user-images.githubusercontent.com/44789/148116937-98a05be3-2db3-4c90-8ca8-46863e9bd990.png) Oddly, *something* is going on. if I change the script to ``` self.log_info(f'Type of abc is <b>grr</b>') ``` then I get "Type of abc is grr" (without the bold). Maybe there's some dubious regexp-type sanitisation instead of proper HTML escaping going on.
adam added the type: bugstatus: needs ownerpending closure labels 2025-12-29 19:33:44 +01:00
adam closed this issue 2025-12-29 19:33:44 +01:00
adam commented 2025-12-29 19:33:45 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Jan 5, 2022):

The HTML tags (or anything that looks like one) are being stripped by the render_markdown filter to protect against XSS. I'm not sure there's any action to take here as this should be considered expected behavior.

If you want to print angle brackets, you can wrap the line (or value) using django.utils.html.escape:

self.log_info(escape(f'Type of abc is {type(data["abc"])}'))
@jeremystretch commented on GitHub (Jan 5, 2022): The HTML tags (or anything that looks like one) are being stripped by the `render_markdown` filter to protect against XSS. I'm not sure there's any action to take here as this should be considered expected behavior. If you want to print angle brackets, you can wrap the line (or value) using `django.utils.html.escape`: ``` self.log_info(escape(f'Type of abc is {type(data["abc"])}')) ```
adam commented 2025-12-29 19:33:45 +01:00
Author
Owner
Copy Link

@candlerb commented on GitHub (Jan 5, 2022):

this should be considered expected behavior

I see it's documented that "Markdown rendering is supported for log messages", although I can't see that it says that html tags are stripped. I can't think of any other system which supports markdown in log messages, or what the intended use is - maybe for **bold** text? ISTM there's plenty of risk of special characters in log data being mistaken for markdown-ese, and I think it would be much safer not to do markdown processing.

However, even if there's a markdown pipeline, another option would be to escape html tags prior to markdown rendering? Obviously this protects against XSS in the same way.

Otherwise, I suppose I can can emit backticks around each log message (ugh).

@candlerb commented on GitHub (Jan 5, 2022): > this should be considered expected behavior I see it's [documented](https://netbox.readthedocs.io/en/stable/customization/custom-scripts/#logging) that "Markdown rendering is supported for log messages", although I can't see that it says that html tags are stripped. I can't think of any other system which supports markdown in log messages, or what the intended use is - maybe for `**bold**` text? ISTM there's plenty of risk of special characters in log data being mistaken for markdown-ese, and I think it would be much safer not to do markdown processing. However, even if there's a markdown pipeline, another option would be to escape html tags _prior_ to markdown rendering? Obviously this protects against XSS in the same way. Otherwise, I suppose I can can emit backticks around each log message (ugh).
adam commented 2025-12-29 19:33:45 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Jan 5, 2022):

I think there are plenty of valid uses for markdown however. Lets say your script creates a bunch of interfaces and you want to drop a link to them in the logs. Perfectly valid. What about a script that pulls data from some external interface you want to link to? I currently use markdown in scripts to print out block data in a pretty fashion (code block).

There are lots of valid uses and we can't discout them because another script doesn't escape data that should be escaped.

@DanSheps commented on GitHub (Jan 5, 2022): I think there are plenty of valid uses for markdown however. Lets say your script creates a bunch of interfaces and you want to drop a link to them in the logs. Perfectly valid. What about a script that pulls data from some external interface you want to link to? I currently use markdown in scripts to print out block data in a pretty fashion (code block). There are lots of valid uses and we can't discout them because another script doesn't escape data that should be escaped.
adam commented 2025-12-29 19:33:45 +01:00
Author
Owner
Copy Link

@candlerb commented on GitHub (Jan 5, 2022):

But you're disallowing HTML tags. So how are you doing links: I guess using Markdown links, [foo](/bar)? OK, that will work. But then I wouldn't call these output page snippets "logs", as they are more than that.

I find the current behaviour surprising as I've never known "logs" work this way in any other application, nor have I ever had to protect log messages against simple symbols like asterisk and underscore. I'm not saying it's not useful, now that I know it's there. Perhaps a note in the documentation would make this clearer.

Or it could be opt-in: self.log_success("some message", markdown=True)

@candlerb commented on GitHub (Jan 5, 2022): But you're disallowing HTML tags. So how are you doing links: I guess using Markdown links, `[foo](/bar)`? OK, that will work. But then I wouldn't call these output page snippets "logs", as they are more than that. I find the current behaviour surprising as I've never known "logs" work this way in any other application, nor have I ever had to protect log messages against simple symbols like asterisk and underscore. I'm not saying it's not useful, now that I know it's there. Perhaps a note in the documentation would make this clearer. Or it could be opt-in: `self.log_success("some message", markdown=True)`
adam commented 2025-12-29 19:33:45 +01:00
Author
Owner
Copy Link

@hagbarddenstore commented on GitHub (Jan 5, 2022):

I think there are plenty of valid uses for markdown however. Lets say your script creates a bunch of interfaces and you want to drop a link to them in the logs. Perfectly valid. What about a script that pulls data from some external interface you want to link to? I currently use markdown in scripts to print out block data in a pretty fashion (code block).

There are lots of valid uses and we can't discout them because another script doesn't escape data that should be escaped.

Wouldn't that use case better fit the output instead?

I'm with @candlerb here, the parsing markdown from the logs is unexpected.

@hagbarddenstore commented on GitHub (Jan 5, 2022): > I think there are plenty of valid uses for markdown however. Lets say your script creates a bunch of interfaces and you want to drop a link to them in the logs. Perfectly valid. What about a script that pulls data from some external interface you want to link to? I currently use markdown in scripts to print out block data in a pretty fashion (code block). > > There are lots of valid uses and we can't discout them because another script doesn't escape data that should be escaped. Wouldn't that use case better fit the output instead? I'm with @candlerb here, the parsing markdown from the *logs* is unexpected.
adam commented 2025-12-29 19:33:45 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Jan 10, 2022):

I've marked this as needs owner. Please note that any proposed change which breaks Markdown support will not be accepted.

@jeremystretch commented on GitHub (Jan 10, 2022): I've marked this as `needs owner`. Please note that any proposed change which breaks Markdown support will not be accepted.
adam commented 2025-12-29 19:33:45 +01:00
Author
Owner
Copy Link

@github-actions[bot] commented on GitHub (Mar 12, 2022):

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

@github-actions[bot] commented on GitHub (Mar 12, 2022): This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our [contributing guide](https://github.com/netbox-community/netbox/blob/develop/CONTRIBUTING.md).
adam commented 2025-12-29 19:33:45 +01:00
Author
Owner
Copy Link

@github-actions[bot] commented on GitHub (Apr 11, 2022):

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.

@github-actions[bot] commented on GitHub (Apr 11, 2022): This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label pending closure status: needs owner type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#5876
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?