Osquerybeat: MFT File Table (Windows) - #50641
Conversation
🤖 GitHub commentsJust comment with:
|
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
TL;DRAll 5 failing Windows jobs are hitting the same compile-time code bug in Remediation
Investigation detailsRoot CauseThe
This mismatch causes Go compilation to fail before tests start. Evidence
Verification
Follow-upIf helpful, I can map each failing call site to the exact replacement expression ( Note 🔒 Integrity filter blocked 4 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | noneWhat is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
…y, and test coverage - Propagate path.Match ErrBadPattern through childrenMatching/FindByDirectory so a malformed filename glob returns an error rather than silently empty rows - Increase findAndResolveInode depth limit from 64 to 128 to accommodate legitimate directory trees up to Windows MAX_PATH depth - Fix preferredFileName to use explicit namespace priority switch (Win32 > Win32+DOS > POSIX > DOS) rather than relying on iteration order; fixes stale DOS timestamps for directories - Set parent_inode=5 for root-level files instead of emitting zero - Fix childrenMatching to log and continue on individual GetMFT failures rather than aborting the entire directory listing - Fix YAML spec examples section (was structurally malformed YAML); add note documenting osquery zero-encoding convention for integer 0 - Add unit tests for preferredFileName namespace priority (no live volume required; constructs FILE_NAME fixtures from raw bytes) - Add explodePath edge-case tests: empty path, root-only path, ADS suffix - Fix GetPartitions -> getPartitions in test (function is unexported); remove stale go-ntfs/parser import from parser_test.go - Regenerate docs/tables/elastic_ntfs_file.md and pkg/tables/generated/ntfs/elastic_ntfs_file/elastic_ntfs_file.go via mage generate
reflect.Ptr was deprecated in Go 1.18 in favour of reflect.Pointer. Fixes two govet inline warnings in GetValueByOsqueryTag.
|
Pinging @elastic/sec-windows-platform (Team:Security-Windows Platform) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds a new osquery table Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@x-pack/osquerybeat/ext/osquery-extension/pkg/ntfs/file.go`:
- Around line 185-197: NewFileNode currently allows a nil volumeInfo which can
cause downstream nil-pointer dereferences in methods like BuildFullPath and
Materialize; update NewFileNode (constructor for fileNode) to validate
volumeInfo and return a non-nil error (e.g., "invalid volumeInfo provided") when
volumeInfo is nil, similar to the existing mftEntry check, so callers cannot
create a fileNode with a nil volume reference.
In `@x-pack/osquerybeat/ext/osquery-extension/pkg/ntfs/parser.go`:
- Around line 72-75: The current loop swallows predicate errors by logging
(log.Errorf("predicate failed for %q: %v", name, err)) and continuing, which
hides malformed glob errors; change the behavior in the predicate evaluation
(the block referencing name, err, log.Errorf and continue) to return the error
to the caller instead of continuing so directory queries fail fast on invalid
globs, and make the same change for the second occurrence around the predicate
handling at lines noted (the block around the second log.Errorf/continue).
- Around line 33-36: The check in explodePath that validates the drive-letter
prefix is case-sensitive and rejects valid paths like "c:\..."; update the
prefix comparison to be case-insensitive by normalizing both sides (e.g.,
strings.ToLower or strings.ToUpper) or using strings.EqualFold when comparing
the drive-letter. Specifically, replace the existing !strings.HasPrefix(p,
v.DriveLetter) check with a case-insensitive variant such as
!strings.HasPrefix(strings.ToLower(p), strings.ToLower(v.DriveLetter)) (or
compare the first character using strings.EqualFold on p[:len(v.DriveLetter)]
and v.DriveLetter) so paths with different letter casing pass validation.
In `@x-pack/osquerybeat/ext/osquery-extension/specs/elastic_ntfs_file.yaml`:
- Line 1: The file elastic_ntfs_file.yaml contains CRLF line endings which cause
yamllint failures; convert its line endings to LF (Unix-style) to unblock CI by
normalizing the file (e.g., run a dos2unix conversion or update the editor/git
settings so the committed file uses \n), then re-commit the updated
elastic_ntfs_file.yaml; also ensure repository/IDE .gitattributes or
core.autocrlf is configured to prevent future CRLF commits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: fb68cf86-48b7-408d-97ce-621809399b94
⛔ Files ignored due to path filters (2)
x-pack/osquerybeat/ext/osquery-extension/pkg/tables/generated/ntfs/elastic_ntfs_file/elastic_ntfs_file.gois excluded by!**/generated/**x-pack/osquerybeat/ext/osquery-extension/pkg/tables/generated/registry_windows.gois excluded by!**/generated/**
📒 Files selected for processing (10)
changelog/fragments/1779917047-add-mft-file-table-to-osquerybeat.yamlx-pack/osquerybeat/ext/osquery-extension/README.mdx-pack/osquerybeat/ext/osquery-extension/docs/tables/elastic_ntfs_file.mdx-pack/osquerybeat/ext/osquery-extension/pkg/filters/filters.gox-pack/osquerybeat/ext/osquery-extension/pkg/ntfs/file.gox-pack/osquerybeat/ext/osquery-extension/pkg/ntfs/file_test.gox-pack/osquerybeat/ext/osquery-extension/pkg/ntfs/parser.gox-pack/osquerybeat/ext/osquery-extension/pkg/ntfs/parser_test.gox-pack/osquerybeat/ext/osquery-extension/pkg/ntfs/volume.gox-pack/osquerybeat/ext/osquery-extension/specs/elastic_ntfs_file.yaml
… into mft_stack/file_data
Proposed commit message
Adds a new elastic_ntfs_file table to the osquery Elastic extension. The table reads file and directory metadata directly from the NTFS $MFT (Master File Table) on Windows, bypassing the filesystem API. This makes it
useful for forensic and threat-hunting queries where standard API access is unreliable — for example, locating files with suspicious timestamps, detecting MFT record reuse via sequence numbers, or enumerating files on
a potentially compromised volume.
● The table supports three query modes:
pathinode,drivedirectory,filename GLOBChecklist
stresstest.shscript to run them under stress conditions and race detector to verify their stability../changelog/fragmentsusing the changelog tool.Disruptive User Impact
How to test this PR locally
Related issues
Use cases
Screenshots
Logs