Conversation
… only) Vendors CodeMirror 5.65.21 and adds an opt-in "Rule editor" checkbox next to the custom-rules box on the Suricata coverage page: syntax highlighting via a small Suricata mode, off by default, with the plain textarea remaining the default/fallback experience. No new container, endpoint, or dependency in this change - CodeMirror.fromTextArea()/toTextArea() make the toggle reversible mid-edit with a byte-identical submitted value, and the preference persists per-browser via localStorage.
Adds a new "linter" sidecar container, built by default alongside the
Suricata agents by splitting Dockerfile_suricata into base/linter/agent
stages so it shares their Suricata compile. It serves /health and
/keywords, generating the keyword list (plus ~28 legacy sticky-buffer
spellings current engines still accept but no longer advertise) from its
own Suricata engine at startup rather than from any third-party database,
keeping it Apache-2.0-clean and version-exact.
The controller proxies /dalton/controller_api/rule_keywords to it, caching
the last successful response in-process so a linter outage degrades
completion to stale rather than absent, and failing open (HTTP 200,
{"available": false}) on any other problem - missing config, connection
refused, timeout, non-200, unparseable body, or valid-JSON-non-object.
Job submission never touches this endpoint.
The rule editor added in the previous commit now offers keyword completion
(Ctrl-Space) and hover documentation sourced from this endpoint.
Adds /check to the linter, backed by the Suricata Language Server (SLS, GPL-3.0, suricata-language-server==2.1.2). SLS runs only inside the linter container and is reached by the (Apache-2.0) controller as an arm's-length subprocess over HTTP - not linked into or distributed as part of the Dalton codebase; disclosed in README.rst alongside the existing CyberChef section. SLS is designed for a developer editing a file on their own machine and reads directives out of the rule buffer on that assumption - '## SLS suricata-options:' goes straight onto the Suricata argv, 'pcap-file:' reads a file off disk, 'replace:' rewrites the buffer via re.sub. guard.py rejects any buffer containing these directives (or a lua:/luajit: keyword that would load a script from disk) before SLS ever sees the text, matching more loosely than SLS's own directive regex so nothing can slip through. Checked against a live Suricata 8.0.6 engine to confirm 'luaxform:' (a real keyword) is never mistaken for the 'lua:' guard. The controller proxies POST /dalton/controller_api/check_rules with the same fail-open contract as the existing /rule_keywords proxy. The rule editor's CodeMirror lint gutter uses the addon's documented-but-easy-to-miss pattern: getAnnotations delivers results synchronously from a variable kept in closure scope, and cm.performLint() is called once a check resolves - stashing and calling the updateLinting callback directly gets silently discarded by the addon's own async-abort-on-change logic. A second "Engine analysis" checkbox (on by default, independently persisted) adds Suricata's own performance/coverage guidance, filtering the noisy once-per-rule "Rule type is ..." classification client-side while keeping the two other severity-4 sources, which are actionable.
The gutter markers were the only way to see what a check found. That means one problem at a time, only if you know to hover, and nothing at all on a touch screen -- and it gives no answer to "how many problems do I have?". Worse, when the checker was unavailable the diagnostics silently cleared and nothing was said, so an outage looked exactly like "your rules are clean". That is the one conclusion a broken checker must never let someone draw. It now says so, and says "No problems found." when there genuinely are none. So there is a results list under the editor, with a severity, a clickable line number that jumps the cursor, and the message. Alongside it a "Check rules" button: without one, someone who ticks "Rule editor" gets highlighting and may never learn that checking exists, since nothing on screen suggests it. The engine analysis toggle moves down next to that button. It modifies what a check produces and you reach for it after reading output, so it belongs with the results rather than above the box. Also: - Guard against overlapping checks. Rapid edits could previously start a second request before the first returned. - Raise the debounce from 0.5s to 1.5s now that a check is one request rather than one per pause. - Sort diagnostics by line, then severity within a line, so an error leads the warning on the same line instead of arriving in engine order. - Cap the results height; a hundred rules with engine analysis on would push the submit button off the page. - Add the stylesheet these need.
fetchKeywords() and runCheck() both fire from enableEditor() and both write to #ruleEditorStatus; whichever resolved second used to win, occasionally letting "keywords from Suricata X" clobber a just-shown check summary (or vice versa) depending on response timing. runCheck() always starts synchronously right after fetchKeywords() kicks off its own fetch, so mark a check as authoritative at that point - before either promise resolves - rather than waiting for it to complete. fetchKeywords() checks that flag before every status write and steps aside once a check is underway, since the check result is the more current, more complete answer (it reports the engine version too). Verified by polling the status element's text over time: it now goes straight from "" to "Checking..." to the final result with no flicker back to the keyword message, on both a fresh page load and a live edit.
A second race in the same area as the status-line one, with a different trigger. The status line and results list were written unconditionally when a check resolved; only the lint-marker update checked that the editor still existed. So a check still in flight when someone unticked "Rule editor" landed after disableEditor() had cleared the panel and refilled it, and those findings were still there the next time the editor was switched on, describing text that may have changed since. A generation token rather than a null check on cm, because a disable followed by an enable inside one request's lifetime leaves an editor that exists but is not the one that asked. Releasing the in-flight guard stays unconditional: a superseded response still has to clear it or the next check never runs.
Completion was reachable only through Ctrl-Space, which on most Linux desktops is bound to the input-method switcher and never reaches the browser. The hint function itself was fine -- it just had nothing to call it, so the feature looked broken. The editor now offers completion as you type, which is how anyone would expect to meet it. Ctrl-Space still works as a manual trigger. Two things that surfaced while confirming the trigger: - It completed in the rule header too, offering option keywords like http.uri where only an action, protocol, address or port is legal. Restricted to between the header's parens. - With an empty prefix it dropped the entire keyword list, several hundred entries, into the popup. It now waits for something to match on. And the typing guard needed to look at the line's length, not just the number of lines: change.text is an array of lines, so a single-line paste passed the old check and dropped a completion popup over the rule that had just been pasted.
alert tcp any any -> any any (msg:"fk"; http.uri; content:"adsf"; fast_pattern:only; http.user_agent; content:"foo"; sid:232;) showed the 'fast_pattern:only' note twice. The duplication is Suricata's, not ours or the language server's: engine analysis runs a fast-pattern pass and a rule pass, both append to the same record, and its rules.json comes out with two identical entries in one rule's notes array. Deduplicate on line, severity and message when rendering. Identical notes at one spot collapse; the same text on another line, or different text at the same spot, still shows.
From two code reviews of this branch. The reviewers could not defeat the "## SLS" or lua guards across 1,884 crafted variants, but found a third file-touching path neither guard covered. **dataset: is not a "## SLS" directive**, so the guard never looked at it, but SLS acts on it all the same: _rules_buffer_prepare_dataset() takes the load/save/state target straight out of the rule and joins it onto its temp directory unsanitised. os.path.join does not contain a traversal -- "../x" escapes the directory and an absolute path replaces it outright -- so an unchecked target was a create-or-truncate primitive at any path the linter user can write. The read_only rootfs is what stopped it; that is defence worth having but not worth relying on. Targets must now be plain filenames. **Oversized buffers were refused two hops too late.** The 64KB limit lived only in the linter, while MAX_CONTENT_LENGTH is a gigabyte because it is sized for pcap uploads, so a huge "rules" string was parsed, re-serialised and re-encoded in the controller first. Refused now on content-length and on decoded size. **A check asked for while one was in flight was dropped with nothing to re-arm it**, so toggling engine analysis or pressing the button at the wrong moment left the panel stale until the next edit. Coalesced into one follow-up run. **Nothing bounded the browser's own wait**, so a stall on that leg would pin the in-flight flag and kill checking until reload, with the status stuck on "Checking...". AbortController, eight seconds. Also: the proxy returns only the two fields the page uses rather than the linter's whole response body; a lone surrogate is refused rather than raising out of the request; a non-string "rules" is a 400 rather than a 500; and the promise chain ends in finally, so a throw inside the error handler can no longer leak the in-flight guard.
gunicorn ran with default settings, so a wedged check held a worker for 30s -- long after the controller gave up at 6s -- and the accept queue was unbounded. Sets a 15s timeout, a graceful timeout and a backlog. The hover tooltip's documentation link could never be clicked. The tooltip is appended to document.body, so it is not a descendant of the editor wrapper, and moving the pointer onto it fired the wrapper's mouseleave and removed the tooltip out from under the cursor. Checks relatedTarget first. Hover work is also debounced. Building it does coordsChar plus getTokenAt, which re-tokenises the line, and then rebuilds the tooltip DOM -- all at pointer rate while someone simply rests the cursor on a keyword. Dropped the form-submit listener: CodeMirror.fromTextArea already registers one on the form and wraps form.submit, so this duplicated it. It was also the one unguarded getElementById in init(), which runs first in $(document).ready -- had #submitjob ever been absent it would have thrown and taken the page's ruleset, sensor and pcap wiring down with it.
app.py was importing suricatals.langserver.LangServer directly, which links GPL-3.0 code into the same process as this Apache-2.0 codebase - contradicting the README's arm's-length-subprocess licensing claim. Switch to invoking SLS's own --batch-file CLI as a separate OS process instead, and update the README/comments to describe the actual boundary. Also replace stale/guessed performance comments with numbers measured against the rebuilt linter container.
CodeMirror's lint addon ships gutter icons for "error" and "warning" only, so LSP Info and Hint were mapped onto "warning" and drew a warning triangle beside messages that are not warnings -- which is most of what engine analysis emits. Add a "note" severity with its own icon and fold Info and Hint into it.
It sorted by line, with severity only breaking ties within one line, so an error in the fortieth rule sat below thirty-nine notes. The panel is a fixed height that scrolls, so "1 error" in the status line could point at a row that was not on screen. Sort on a rank, not the raw severity: Information and Hint render identically as notes, and ordering them against each other would break line order for a distinction the reader cannot see.
The linter returned an empty diagnostics list when SLS crashed, which the editor rendered as "No problems found" for rules that were never checked. Treat a nonzero exit with no diagnostics, a timeout, or non-JSON output as a failure (HTTP 502) so the controller reports "unavailable" instead. SLS was never given the throwaway directory as its cwd, so the isolation the comment promised did not exist: it resolves dataset load targets with a cwd-relative os.path.exists(). Run it there, with TMPDIR pointed there too so its own sls_* scratch directory is removed with it, and in its own process group so a timeout kills the Suricata it forked rather than leaving that orphaned on the container's one CPU. Drop --max-lines, which SLS only consults on the LSP didChange path, never in batch mode; the guard's line limit is the only one. README and dalton.conf said removing the two URL keys disables the feature, but the controller fell back to the linter container's address. Make an absent or empty URL mean "off", as documented, and say so in both places. Fold the duplicated GET and POST fail-open helpers into one, and serve the keyword payload from memory for ten minutes before asking the linter again, since page loads were pulling the full list through the same two workers that serve syntax checks. In the editor, completion decided "inside the options" by comparing the last ( and ) on the line, so a ) inside a pcre or content string switched it off for the rest of the rule, and continuation lines never completed at all. Ask the mode's tokenizer state instead, and treat an unterminated string as a string so completion stays quiet inside a half-typed value. The keyword/check status race flag was set before the empty-buffer early return, which blanked the keyword status line whenever the editor was enabled over an empty box; set it only when a check request goes out. The guard tests' __main__ block sat above the last test class, so running the file directly skipped the dataset-traversal tests. Move it to the end.
A major bump rather than a minor one because the default compose stack gains a service: docker compose up now builds and starts the linter container, which runs the GPL-3.0 Suricata Language Server as a separate process. Nothing breaks for existing deployments (the feature is off by default and the controller fails open), but the deployment topology changes, and that deserves a version operators will read the notes for. Also brings the VERSION file, which had lagged at 3.6.0, back in line with pyproject.toml.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional CodeMirror-based editor for the custom-rules box on the Suricata coverage page, with syntax highlighting, keyword completion and hover docs, and real syntax checking against a running Suricata engine. Off by default; the plain textarea stays the default experience.
Syntax checking is backed by a new
lintersidecar container that runs the Suricata Language Server (SLS). Because this adds a service to the default compose stack, this is released as 5.0.0.What's in it
app/static/js/suricata-editor.js, vendored CodeMirror 5.65.21, MIT): opt-in via a "Rule editor" checkbox, preference remembered in localStorage. Completion triggers while typing (Ctrl-Space is often eaten by input-method switchers on Linux). Diagnostics render as gutter markers plus a results list below the editor, sorted worst-first, with line links. An "Engine analysis" toggle adds Suricata's own performance/coverage guidance.dalton-agent/linter/): a small Flask app built from a newlintertarget inDockerfile_suricata, sharing the agent's Suricata compile./keywordsis generated fromsuricata --list-keywordsat startup (plus a verified table of legacy sticky-buffer spellings)./checkwrites the buffer to a throwaway directory and invokes SLS's--batch-fileCLI as a separate process.app/dalton.py): two proxy endpoints,rule_keywordsandcheck_rules, behind the existingcheck_userauth. Both fail open: any problem reaching the linter reports "unavailable" rather than an error, and job submission never touches this path. Keyword data is cached in-process. Request size is capped before parsing.dalton.conf):rule_check_url,rule_check_timeout,keywords_url. An absent or empty URL disables the feature.Licensing
SLS is GPL-3.0. It is confined to the linter container and invoked only as a separate OS process via its own CLI, never imported or linked. The linter's Flask app and the controller are Apache-2.0 as before; the controller only ever talks to the linter over HTTP. Details in the README section and
dalton-agent/linter/requirements.txt.Security notes
SLS is designed for a developer editing files on their own machine and acts on directives in the buffer (
## SLS suricata-options:goes straight to Suricata's argv,## SLS pcap-file:reads a file,dataset:targets are copied by unsanitised path).dalton-agent/linter/guard.pyrejects all of these before SLS sees the text, matching more loosely than SLS's own regexes. The container also runs read-only, non-root, with all capabilities dropped,no-new-privileges, a 1 CPU / 1 GB cap, and a 256 MB tmpfs. SLS runs in its own process group with the throwaway directory as both cwd and TMPDIR, so a timed-out run is killed as a tree and leaves nothing behind.Upgrade notes
docker compose upafter this change builds and starts one more container (dalton_linter). It is optional: remove the service fromdocker-compose.ymland leave the two URLs empty indalton.confto run without it. Notedalton.confis baked into the controller image, so changing it means a rebuild.Testing
make lint,make test(68 tests, including 27 for the guard and 18 for the fail-open proxies), andmake hadolintall pass.)inside a pcre string, on continuation lines, suppressed in the header / inside strings / after the closing paren, legacy keyword display, keyword status on an empty editor, and a full check round trip.