Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Remote ISF server for Linux testcases #1316

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

gcmoreira
Copy link
Contributor

This PR introduces changes to enhance our testing capabilities by allowing Linux test cases to use the @Abyss-W4tcher ISF repository on GitHub.
This not only provides greater flexibility for updating the ISF database and keeping it in sync with the latest dwarf2json version, but also establishes a fully open-source workflow.
This will enable us to easily add more images later on, such as different Linux kernel versions and distributions, which will be immediately ready for testing our code.

If it's successful and accepted, we can also migrate the macOS test cases to use this repository.

@gcmoreira
Copy link
Contributor Author

Bloody brilliant! It’s working like a kangaroo on a trampoline! :)

Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'm roughly happy with the concept, but I'd like to run it past @npetroni who I believe made the first testing harness, and also get his views on downloading additional files from a server we don't control, which my security hat is just making me wonder about. Another option would be to create our own mini repo with symbols just for the test cases we've got but I can't tell if that's prudent or paranoid. I don't think a bad JSON file could do anything bad to us, it shouldn't be able to execute anything so the worst case is it causes our tests to fail, and we do already download the ISF files from a server (albeit a server we own). So probably ok, but I'd like @npetroni 's opinion on it first please...

.github/workflows/test.yaml Outdated Show resolved Hide resolved
test/test_volatility.py Show resolved Hide resolved
test/test_volatility.py Show resolved Hide resolved
@ikelos ikelos requested a review from npetroni October 22, 2024 23:36
@Abyss-W4tcher
Copy link
Contributor

Hi, just passing by as I was mentioned. Thanks for wanting to integrate my repo as a symbol source, it's much appreciated !

However, if I take a neutral point of view, this increases the risk of supply chain bugs. Even if I put efforts to prevent that from happening, I think each testcase should come with its own ISF attached, hosted by volatility foundation ?

I am not trying to sabotage your work, but I am not sure that the remote isf feature fits this use case ?

Thanks again 👍

@ikelos
Copy link
Member

ikelos commented Oct 23, 2024

Yeah, that was my thinking, it's just a text file on a webserver that points the banner to a downloadable file, so we could definitely look after that and keep it updated with our testcases, and then that saves @Abyss-W4tcher from become an integral service to out testcases. 5;P It will mean that we'll have to get better/quicker at updating those JSON files and where they're stored, but that's definitely a solvable "us" problem rather than something insurmountable. 5:)

@npetroni would you be happy with that (and/or, could you help us get it all in place please?) 5:)

@ikelos
Copy link
Member

ikelos commented Nov 7, 2024

Hiya, could we please get the remote URL turned into a variable so it's only defined once clearly at the top of the test cases? We're looking at making our own little ISF github repo just for the test materials to start off with. We'll let you know what the URL will change to once we've started making it...

@gcmoreira
Copy link
Contributor Author

You mean to move the URL from being a parameter in the YAML/pytest to hard-coding it in test_volatility.py.
I can make that change, but it seems less flexible overall. Right now, it's already defined once in this implementation, and keeping it as a pytest parameter allows for easy command-line changes during development, without modifying the code itself. I could be overlooking something, but the suggested approach seems like a step backward to when it was a hard-coded variable in the source.

@ikelos
Copy link
Member

ikelos commented Nov 8, 2024

No, I just mean as a variable in the YAML/pytest file, rather than in the middle of the command statement. I'm pretty sure the yaml allows parameterization?

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#defining-environment-variables-for-a-single-workflow

So setting an env, (ideally pulling out $IMAGES_URL or something too, so that the duplicated parts of the image URL are in a single place) and turning it into a $REMOTE_ISF_URL env variable, so it's easier to see and the command used in the test is easier to read.

@gcmoreira
Copy link
Contributor Author

@ikelos Cool, all done

@ikelos
Copy link
Member

ikelos commented Nov 15, 2024

Awesome, now we just need a little local volatilityfoundation repo to keep the test ISFs in, and we should be good to go. I believe @npetroni is working on that, but I might be able to get that moving next week since I've got a week off so hopefully I'll do a bunch of volatility housekeeping stuff... 5:D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants