Summary
The getFromDB method in internal/plugins/db/fsdb/patterns.go (line ~121) joins the user-supplied name parameter directly into file paths via filepath.Join without explicit path traversal sanitization. While the risk is low, adding a guard would be a good defensive hardening measure.
Details
In getFromDB, the pattern name flows into two filepath.Join calls:
customPatternPath := filepath.Join(o.CustomPatternsDir, name, o.SystemPatternFile)
// ...
patternPath := filepath.Join(o.Dir, name, o.SystemPatternFile)
A malicious or accidental name value like ../../etc could theoretically traverse outside the intended patterns directory. In practice, the risk is very low because:
filepath.Join normalizes paths (removes redundant separators, resolves . elements)
- The resulting path must contain a valid
system.md file to succeed
- Fabric is a local CLI tool, so the user already has filesystem access
However, as defense-in-depth, a simple guard would prevent any future risk if this code path is ever exposed through a network-facing interface.
Suggested Fix
Add a path traversal check at the top of getFromDB, for example:
if strings.Contains(name, "..") {
return nil, fmt.Errorf("invalid pattern name: %q", name)
}
Or use filepath.Rel to verify the resolved path stays within the intended directory.
Context
This was identified during the review of PR #2093. It is a pre-existing issue, not introduced by that PR. Filed separately as recommended in the review.
Priority: Low
Category: Security hardening
Summary
The
getFromDBmethod ininternal/plugins/db/fsdb/patterns.go(line ~121) joins the user-suppliednameparameter directly into file paths viafilepath.Joinwithout explicit path traversal sanitization. While the risk is low, adding a guard would be a good defensive hardening measure.Details
In
getFromDB, the pattern name flows into twofilepath.Joincalls:A malicious or accidental
namevalue like../../etccould theoretically traverse outside the intended patterns directory. In practice, the risk is very low because:filepath.Joinnormalizes paths (removes redundant separators, resolves.elements)system.mdfile to succeedHowever, as defense-in-depth, a simple guard would prevent any future risk if this code path is ever exposed through a network-facing interface.
Suggested Fix
Add a path traversal check at the top of
getFromDB, for example:Or use
filepath.Relto verify the resolved path stays within the intended directory.Context
This was identified during the review of PR #2093. It is a pre-existing issue, not introduced by that PR. Filed separately as recommended in the review.
Priority: Low
Category: Security hardening