Skip to content

conf-yaml: support glob patterns in include directive (v3) - #15937

Open
ssam18 wants to merge 1 commit into
OISF:mainfrom
ssam18:feat-8427-include-glob-v3
Open

conf-yaml: support glob patterns in include directive (v3)#15937
ssam18 wants to merge 1 commit into
OISF:mainfrom
ssam18:feat-8427-include-glob-v3

Conversation

@ssam18

@ssam18 ssam18 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Continuation of #15574 (v3). See "Changes since #15574" below.

Contribution style:

Our Contribution agreements:

Changes (if applicable):

Link to ticket: https://redmine.openinfosecfoundation.org/issues/8427

Previous PR: #15574

Describe changes:

SCConfYamlHandleInclude now expands shell-style glob patterns (*, ?, [) via glob(3) when the include path contains them. Each match is loaded in lexicographic (sorted) order; literal paths bypass glob() and take the existing per-file path. A pattern matching zero files is logged as a warning, not an error, so drop-in conf.d/-style directories may be empty. The precedent for the glob(3) shape and HAVE_GLOB_H gating is rule-files: in src/detect-engine-loader.c.

Changes since #15574

Addresses @jasonish's review: "Document and accept that globbing doesn't work on Windows for now. Ideally with a warning message if the patterns are found."

  • Windows has no glob.h, so HAVE_GLOB_H is undefined and an include pattern fell through to be opened as a literal filename, failing with a confusing errno:

    Error: conf-yaml-loader: Failed to open configuration include file .../*-order.yaml: Invalid argument
    

    A pattern is now detected regardless of HAVE_GLOB_H and, on platforms without glob(3), logged and skipped instead:

    Warning: conf-yaml-loader: Glob patterns in include are not supported on this platform, skipping include .../*.yaml
    

    Literal includes are unaffected, including the existing hard error for a missing literal file.

  • Documented the platform limitation in doc/userguide/configuration/includes.rst and in the function's doxygen comment.

  • Rebased onto current main.

Skip vs. error. An unsupported pattern is a warning that continues, reading "accept that globbing doesn't work on Windows" as a documented platform limitation rather than a startup failure. The tradeoff is that a Windows user gets a partially-loaded config with only a warning. Happy to make it a hard error with an explicit message instead if you prefer that.

The suricata-verify test skips on builds without glob(3) by probing src/autoconf.h for HAVE_GLOB_H, so it no longer depends on the Windows runner incidentally failing the probe redirect.

Verification

Built with HAVE_GLOB_H undefined to exercise the Windows path directly:

case before after
pattern include, no glob(3) Error: Failed to open ... Invalid argument, exit 1 Warning: ... not supported on this platform, exit 0

On a normal build: all matched files load in lexicographic order, a no-match pattern still warns without failing, literal includes still work, a missing literal file still errors, and all 9 ConfYaml unit tests pass. suricata-verify: ===> config-includes-glob-order: OK, and SKIPPED on a build with HAVE_GLOB_H undefined.

The Windows CI failures on #15574 dated 2026-06-07 also predate the suricata-verify test rewrite of 2026-06-12, which removed the suricata.yaml that broke suricata-verify's setup step; the Ubuntu 24.04 (afpacket IPS tests in namespaces) failure there was a transient Codecov CLI GPG error, unrelated to the change.

Provide values to any of the below to override the defaults.

SV_BRANCH=OISF/suricata-verify#3250

SCConfYamlHandleInclude only accepted explicit filenames. The
rule-files directive has supported shell-style glob expansion via
glob(3) for years, and asymmetry with include has been blocking
clean drop-in conf.d/ style configuration directories.

Refactor SCConfYamlHandleInclude into a thin wrapper that resolves
the path against conf_dirname and, if the input contains glob
metacharacters (*, ?, [), expands it with glob(3) and includes
each match in lexicographic order. A pattern that matches no
files is logged as a warning and not treated as an error, so a
drop-in directory can be empty without breaking startup. Literal
paths skip glob() and call the existing per-file inclusion logic
directly, preserving current behaviour.

Glob expansion needs glob(3), which is not available everywhere.
Windows builds have no glob.h, so an include pattern used to fall
through and be opened as a literal filename, failing with a
confusing errno. On such platforms the pattern is now logged as a
warning and skipped instead. Literal includes are unaffected.

The behaviour is verified by the config-includes-glob-order
suricata-verify test, which checks that every file matching a
pattern is loaded and that matches are included in deterministic
lexicographic order.

Document the new behaviour, including the platform limitation, in
doc/userguide/configuration/includes.rst.

Feature: OISF#8427.
@ssam18

ssam18 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@jasonish this is v3 of #15574, addressing your review. On the first of the two options you listed:

Ideally figure out if we're just missing a package in msys2?

It is not a missing package. glob() is POSIX and mingw-w64 does not ship <glob.h> in either the MINGW64 or UCRT64 toolchains, which are the ones the failing Windows jobs build with. MSYS2's Cygwin-based msys environment does have it, but that is not the environment Suricata is built in here, so there is nothing to install. The remaining routes would be vendoring a third-party port (e.g. glob-dirent-win32) or reimplementing on FindFirstFile/FindNextFile, both of which look clearly out of scope for this PR — especially given your Rust based loader should fix it properly before 9.

So this takes your second option: document and accept, with a warning when a pattern is found. Details and the skip-vs-error tradeoff are in the description; I am happy to switch it to a hard error if you would rather it not start at all.

@catenacyber on your question in #15574:

What does the 3 mean in glob(3)?

It is the man page section, as @victorjulien said. Since the notation caused confusion in end-user documentation, I am happy to reword the user guide to "expanded using the C library's glob() function" — say the word and it goes into the next revision. I left it as is for now rather than opening another PR just for the wording.

@catenacyber

Copy link
Copy Markdown
Contributor

catenacyber on your question in #15574:

What does the 3 mean in glob(3)?

It is the man page section, as @victorjulien said. Since the notation caused confusion in end-user documentation, I am happy to reword the user guide to "expanded using the C library's glob() function" — say the word and it goes into the next revision. I left it as is for now rather than opening another PR just for the wording.

Ok for me as is for this part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants