search

yunginnanet / HellPot

HellPot is a cross-platform portal to endless suffering meant to punish unruly HTTP bots.
MIT License
686 stars 32 forks source link

Essentially Rewrite HellPot + Fun Bonus Features #162

Closed yunginnanet closed 3 months ago

yunginnanet commented 3 months ago

Overview

I accidentally a whole coca cola bottle HellPot

This really is essentially a rewrite.

The only thing that remains untouched more or less is the heffalump core.

This should resolve countless idiosyncrasies in configuration behavior.

Additionally, there are now unit tests.

New Features

New features by

are scheduled to hitch a ride, pending further review and testing.

Slimfast

Further slimming of dependencies

Issues resolved

yunginnanet commented 3 months ago

[!IMPORTANT]
~*Need to create the log file after we read the config and append it onto the output field of logger.Parameters and then assure this is properly handled upon calling `logger.New(logger.Parameters)`.**~

~A unit test is in order to verify the consistency of said behavior.~

✅ done

yunginnanet commented 3 months ago

@ShadowJonathan your commits are not signed, you'll probably need to rebase.. I will likely just make a separate PR until we get that sorted.

ShadowJonathan commented 3 months ago

I'm not sure what you mean, do you want to exclude my commits, or...?

137 is already merged, I'm unsure what you mean.

I haven't started signing my commits due to the hassle that I've experienced with it in the past, apologies that they're not indeed signed, though I did not know of this required when I was making #137.

yunginnanet commented 3 months ago

137 is already merged, I'm unsure what you mean.

it's merged to dev, we are trying to merge dev to main

kescherCode commented 3 months ago

@yunginnanet if signed commits are required, you should require them for all commits, on all branches and for all PRs. You can't expect someone to fix it after the fact.

You can fix it by doing an interactive rebase, and changing the committer (not the author!) to be yourself and doing a sign-and-amend.

kescherCode commented 3 months ago

And yes, that will require a force push. But that's less hassle than excluding those commits and requesting a PR with signed commits be made.

yunginnanet commented 3 months ago

@yunginnanet if signed commits are required, you should require them for all commits, on all branches and for all PRs. You can't expect someone to fix it after the fact.

You can fix it by doing an interactive rebase, and changing the committer (not the author!) to be yourself and doing a sign-and-amend.

While I agree that the inconsistency is annoying, branch protections on main vs branch protection on dev is using the system exactly as intended.

Additionally, I have had to fix my commits for many many PRs I've made to other projects and do force pushes myself.. For example, my PR/CL for https://github.com/golang/go [^1] I had to rebase against master several times and force push to my branch.

[^1]: got reverted unfortunately but this is in regard to the merge process as the repo is probably considered canonical amongst us gophers

yunginnanet commented 3 months ago

That said, since this has already been merged I am willing to put in the work on my end to help adjust the development branch once they make a new branch and do a rebase.

I can't sign their commits for them... I am not them.

If I hack apart the history in the way you describe and do a force push, I have now obliterated accountability for the changes made by the author.

If I merge without the signing requirements being met, I have now obliterated accountability for the changes made by the author.

ShadowJonathan commented 3 months ago

You can sign my commits, or remove them, at this point I don't care.

This should have been brought up before you merged the PR, and not now. I might've been partial to signing my commits in some way (by pushing them through the web interface), but now that has been more than 2 months ago, I assumed all development was done when you accepted the merge, and now I'm not interested in a retroactive problem, that I wasn't made aware of in any way, that should've been brought up in that PR in the first place.

Sorry.

yunginnanet commented 3 months ago

That's perfectly fine.

yunginnanet commented 3 months ago

Superseded by https://github.com/yunginnanet/HellPot/pull/163

kescherCode commented 3 months ago

@yunginnanet

While I agree that the inconsistency is annoying, branch protections on main vs branch protection on dev is using the system exactly as intended.

Well, it broke down here.

Also, yes, you can sign commits authored by others. Simply become the committer (≠ author!), and sign the commit.

Here's an example for me doing precisely that on another project, with a cherry-picked commit: https://github.com/CatCatNya/catstodon/commit/8e8cf3666615f24b05dcbcc3616db63bf5c5678a

yunginnanet commented 3 months ago

@yunginnanet

While I agree that the inconsistency is annoying, branch protections on main vs branch protection on dev is using the system exactly as intended.

Well, it broke down here.

Also, yes, you can sign commits authored by others. Simply become the committer (≠ author!), and sign the commit.

Here's an example for me doing precisely that on another project, with a cherry-picked commit: CatCatNya/catstodon@8e8cf36

Yes, which means I then adopt accountability for the commits.

HellPot is a honeypot, therefore it is security minded software. I need collaborators that have contributions that make it to the main branch to be prepared to take full responsibility for their contributions. If they are not, that is fine, but their code likely will need to stay in a development branch.

It is notable that the contributions in question modified the core heffalump package. This package, while still relatively minimal risk thanks to Go's memory safety, is by far the most sensitive and critical part of the entire project. I am okay with this outcome.

kescherCode commented 3 months ago

@yunginnanet Sure, but how do you ever expect a commit that is unsigned due to zero signage requirements on the dev branch to enter the main branch? That just discourages contributions. Make requirements for this consistent, or you will run into more such situations.

yunginnanet commented 3 months ago

@yunginnanet Sure, but how do you ever expect a commit that is unsigned due to zero signage requirements on the dev branch to enter the main branch? That just discourages contributions. Make requirements for this consistent, or you will run into more such situations.

I am also fine with making the requirements consistent, this was certainly not an intentional idiosyncrasy.