Add verify and pull functionality #106
Conversation
reteps
force-pushed
the
verify-and-pull
branch
from
d4937d0 to
46f4cda
Compare
reteps
changed the title
|
I have now successfully used this to sync my challenge state on github + CTFd, so I can confirm it works! @ColdHeat |
|
Bump on this @ColdHeat :) |
ColdHeat
force-pushed
the
verify-and-pull
branch
from
6140196 to
9879681
Compare
|
I'm not entirely sure what's up with this branch but I'm having difficulty pulling from upstream without bringing in your other changes. Please fix up the lints on this code. |
reteps
force-pushed
the
verify-and-pull
branch
from
9879681 to
1c7c4d4
Compare
|
@ColdHeat how about now? |
ColdHeat
left a comment
ColdHeat
left a comment
There was a problem hiding this comment.
Looks roughly correct but needs some changes before it can be merged in
| # Get base file name | ||
| f_base = f.split("/")[-1].split('?token=')[0] | ||
| if f_base not in local_files and _verify_new_files: | ||
| raise Exception(f"Remote challenge has file {f_base} that is not present locally") |
There was a problem hiding this comment.
These exceptions should be some kind of custom exception or some other higher level exception instead of the base Exception
| req = s.get(f) | ||
| req.raise_for_status() | ||
| remote_file = req.content | ||
| local_file = Path(challenge.directory, local_files[f_base]).read_bytes() | ||
| if remote_file != local_file: | ||
| raise Exception(f"Remote challenge file {f_base} does not match local file") |
There was a problem hiding this comment.
Maybe it makes more sense to use filecmp and write the downloaded file to a tempfile?
There was a problem hiding this comment.
Just so we don't read the local file into memory?
| updated_challenge[key] = remote_details_updates[key] | ||
|
|
||
| # Hack: remove tabs in multiline strings | ||
| updated_challenge['description'] = updated_challenge['description'].replace('\t', '') |
There was a problem hiding this comment.
Are you referring to the removal of tabs?
| If, at any point, changes may have been made to a challenge through the CTFd UI by an admin. To verify that your challenge.yml file matches remote contents, you can use: | ||
|
|
||
| ``` | ||
| ❯ ctf challenge verify [challenge.yml | DIRECTORY] [--verify-files] [--verify-defaults] |
There was a problem hiding this comment.
I think that this should be --files and --ignore-defaults. I think the default checking behavior (which I'm not totally clear about what it does) should be implicit. As for the files, users should be warned when they run the command that files are not verified unless you provide the --files switch.
| If you want to pull down the latest version of the challenge, and its challenge files, you can use: | ||
|
|
||
| ``` | ||
| ❯ ctf challenge verify [challenge.yml | DIRECTORY] [--update_files] [--create-files] [--create_defaults] |
There was a problem hiding this comment.
Yes, sorry that is a typo
| If the `--update_files` flag is set, the latest version of every file will be redownloaded from CTFd. | ||
|
|
||
| If the `--create-files` flag is set, any new files added to through the CTFd UI will be downloaded to the same directory as the `challenge.yml` file. | ||
|
|
||
| If the `--create-defaults` flag is set, any optional default values will be added to the `challenge.yml`. | ||
|
|
||
| **This is a destructive action! It will overwrite the local version of `challenge.yml` with the version on CTFd!** |
There was a problem hiding this comment.
I'm not a fan of listing out all the switches here. The README should show just basic concepts of the idea and the help text should show how to use the switches.
There was a problem hiding this comment.
Sounds good, I will remove them here
| If the `--verify-files` flag is set, challenge files will be downloaded and the binary files will be compared. | ||
|
|
||
| If the `--verify-defaults` flag is set, challenge files will be verified to make sure they include default optional keys present on CTFd. | ||
| If you want to pull down the latest version of the challenge, and its challenge files, you can use: |
There was a problem hiding this comment.
See below regarding just showing basic concepts of the idea
…lity Added feature from CTFd#106
|
upvoting this. |
Adds `ctf challenge mirror <challenge>` and `ctf challenge verify <challenge>` adapted from #106 Originally, this functionality was called `pull` and `verify` - however, `push` is already used to push challenge changes to the git repository. I think `mirror` is a better name, as ctfcli will attempt to mirror / copy the remote state from ctfd. This way `pull` stays in its current git-like form, for git-related operations. More additions: - I've removed update / create / verify files - this can be achieved by just using --ignore=files. - I've added `files_directory_name` (defaulting to `dist`) to specify where ctfcli should download the files, relative to challenge.yml - I've added a warning when there are additional challenges on the remote, that are not registered locally - `ctf challenge verify` will exit with status code 2 if the verification was successful, but some challenges are out of sync. - I've fixed some typos Thanks to @reteps for the initial contribution! Closes: #101 #106
|
@ColdHeat This can be closed now |
|
Thanks for the contribution @reteps, sorry this didn't end up getting merged. |
4 participants
This PR fixes issue #101, as well as a personal need to pull the challenge state back down from CTFd.
The main idea of these pieces of functionality is to ensure that CTFd and repo state never get out of sync.