file.list works for directories, shows timestamp information as epoch for create,access,modified,changed#2376
file.list works for directories, shows timestamp information as epoch for create,access,modified,changed#2376AmeliaYeah wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Thanks for the PR @AmeliaYeah and sorry for the delay.
We're all on a short break post CCDC so may be slow to respond.
This looks like a great feature!
Instead of adding a new function though would moving this capability to the existing file list function make sense for your use case?
I like the idea of update file list to work on dirs, and also switching to epoch.
Yeah the more I think of it the more I feel this might be better, I'll update file list to show the directory and also show timestamp information as epoch |
|
Reviewing Summary: Good forensic enhancement, keeps legacy Detailed file:line comments posted below as inline review comments + this summary. |
jabbate19
left a comment
There was a problem hiding this comment.
File:line review for timestamp + dir-self changes – see inline comments for exact locations.
| // If I implement `handle_list` roughly: | ||
| // show information for the file | ||
| // if it's a directory, show information of the directory being listed from | ||
| final_res.push(create_dict_from_file(&path_buf)?); |
There was a problem hiding this comment.
Breaking change – dir self now included (55-68) at list_impl.rs:55-68:
Old:
if path_buf.is_dir() { push children } else { push self }New always pushes self then children if dir. Callers checking len() == child_count or files[0] as first child break – adds +1 per dir.
- Document breaking change in PR description / changelog, or gate with
include_selfparam (default false for v1 compat). read_dirorder is FS-dependent; doc example shows deterministic order. Consider sorting byabsolute_pathor documenting as unordered.- Glob edge:
file.list("/tmp/*")where matches are dirs now returns each matched dir (self) + its children – one-level recursive for wildcards. Confirm intentional.
File: implants/lib/eldritch/stdlib/eldritch-libfile/src/std/list_impl.rs:55-68
There was a problem hiding this comment.
i added dir_self as a flag, false by default, to show the directory itself. i also added sorting by absolute_path
| #[cfg(unix)] { | ||
| use std::os::unix::fs::MetadataExt; | ||
| times.insert(Value::String("changed".to_string()), Value::Int(metadata.ctime())); | ||
| } |
There was a problem hiding this comment.
get_times_dict mixes epoch sources + silent omission (76-107) at list_impl.rs:77-106:
changedviaMetadataExt::ctime()already epoch i64 (can be negative pre-1970). Others viaSystemTime::duration_since(UNIX_EPOCH)errors pre-1970 and gets silently dropped. A pre-1970 file could expose onlychanged. Normalize or document omission.cast_signed()on u64 truncates >i64::MAX;i64::try_frommore explicit.- Style:
return Value::Dictionary(...)+ stray;after}line 103 non-idiomatic – use tail exprValue::Dictionary(Arc::new(RwLock::new(times))). - Duplicate syscall:
create_dict_from_file:177-182callsmodified()for legacy string thenget_times_dictcalls it again. Cache once. - Typo
compatabilityline 177 →compatibility.
File: implants/lib/eldritch/stdlib/eldritch-libfile/src/std/list_impl.rs:77-106
There was a problem hiding this comment.
fixes applied, negative epochs supported for mtime/atime/crtime
| "absolute_path": "/some_directory", | ||
| "file_name": "some_directory", | ||
| "group": "root", | ||
| "modified": "2026-07-09 18:24:44 UTC", // modification time represented as a UTC string |
There was a problem hiding this comment.
Docs file.list example – style (735-791) at eldritch.md:747:
-
Caps:
CHANGED TIME IS ONLY SUPPORTED IN UNIX SYSTEMS!→changed is Unix-only (ctime). -
"type": "dir"/"file"lower-case matches code (good) but previous docs said Directory/File – call out normalization. -
Epoch values
1783621485≈ mid-2026 future – consider realistic1700...or note illustrative. -
Missing disclosure:
times.createdabsent on Linux ext4 (no birthtime),timesfields may be missing pre-1970 whenduration_sincefails. -
Grammar:
along with it's outputline 735 →its output. -
Document that
file.list("/some_directory")now returns directory itself as first entry – new vs upstream.
File: docs/_docs/user-guide/eldritch.md:735-791
There was a problem hiding this comment.
applied, lowercase dir and file are accurate, and all fields are accurate to the current version of file.list (including this PR)
|
didnt want to make a separate issue for this but during testing i noticed |
| `file.list(path: str, dir_self: Optional<bool> = False) -> List<Dict>` | ||
|
|
||
| The **file.list** method returns a list of files at the specified path. The path is relative to your current working directory and can be traversed with `../`. | ||
| If `path` is a directory, `dir_self` denotes whether to show the information of `path` itself in the `List<Dict>`. This field has no impact if `path` is not a directory. |
There was a problem hiding this comment.
Does dir_self only show the local dir or optionally include it?
If only itself - can you name it only_dir_self to be more clear.
If optionally include the local dir - i think we should include that by default similar to ls
There was a problem hiding this comment.
Okay seems like it optionally includes the local dir. I think we should always include this.
There was a problem hiding this comment.
yeah i feel dir_self is good for backwards compatability, it's optional because in tomes prior to this change (where only path was the parameter), they obviously won't include the param and thus it'll function basically the same to how it did prior.
i wanted to include it by default but felt it might be better this way to prevent potential annoyances.
There was a problem hiding this comment.
in hindsight, i'm not really sure what tomes are going to be hurt by it being included. i kind of originally made it so the dir was included by default but only added this after suggestions from @jabbate19 above.
🤷 idk, i suppose it doesn't hurt, i could include it by default/maybe remove the param for the sake of the function looking more polished tho
|
Thanks @AmeliaYeah ! |
What type of PR is this?
Feature
Eldritch-Function
What this PR does / why we need it:
While this does somewhat exist in the file.list API call (in regards to obtaining the stringified version of modified time), this is a bit of a hacky approach, and also doesn't work with directories. This API call is designed to be an easier way to obtain the time metadata of a file (aswell as beyond just the mtime).
My specific personal motivation was in regard to timestomping a file to it's metadata BEFORE writing to such file, but I'm sure there are multitudes of other use cases for this API call.