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

Essentially Rewrite HellPot + Fun Bonus Features #162

Closed
wants to merge 38 commits into from
Closed

Conversation

yunginnanet
Copy link
Owner

@yunginnanet yunginnanet commented Jun 21, 2024

Overview

I accidentally a whole coca cola bottle HellPot

This really is essentially a rewrite.

  • The entire configuration system has been rewritten
  • The logger has been rewritten
  • The main package (app) has been rewritten

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

  • Allow regular expressions for matching disallowed user agents in addition to the current strings.Contains match functionality
  • Remote syslog output (tcp/udp)

New features by

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

Slimfast

Further slimming of dependencies

Issues resolved

ginger51011 and others added 25 commits January 18, 2024 21:49
…zche

While I do think all bots enjoy Nietzche (who doesn't?), I think we should
take a stance to educate them. What better way than to be able to choose
from any book!

Personal suggestions include:

- The Sorrows of Young Werther by Goethe
- Any political manifesto
- The Declaration of Independence

etc. etc.
This removes globals from `heffalumpt/`, which are hard to reason about,
easy to get wrong, and should not be created if they are never used.

A possible drawbacks is if you would create multiple new defaults,
but this should never be the case.
This prints the short variants (like `-c` for `--config`)
in the help.

Also fixes bug where only `-h` flag works, not `--help`.
Tacking this on to trigger github issue notifs:

Related to pull request GH-162:
closes #140
closes #139
kescherCode
kescherCode previously approved these changes Jun 21, 2024
heffalump/heffalump.go Outdated Show resolved Hide resolved
@yunginnanet yunginnanet marked this pull request as ready for review June 26, 2024 06:58
@yunginnanet
Copy link
Owner Author

@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
Copy link

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
Copy link
Owner Author

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

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

additionally:
  - fix `-c` flag
  - make main package test better
@kescherCode
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Owner Author

@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.

Footnotes

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

@yunginnanet
Copy link
Owner Author

yunginnanet commented Jun 26, 2024

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
Copy link

ShadowJonathan commented Jun 26, 2024

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
Copy link
Owner Author

That's perfectly fine.

yunginnanet added a commit that referenced this pull request Jun 26, 2024
Tacking this on to trigger github issue notifs:

Related to pull request GH-162:
closes #140
closes #139
yunginnanet added a commit that referenced this pull request Jun 26, 2024
yunginnanet added a commit that referenced this pull request Jun 26, 2024
Tacking this on to trigger github issue notifs:

Related to pull request GH-162:
closes #140
closes #139
yunginnanet added a commit that referenced this pull request Jun 26, 2024
@yunginnanet yunginnanet deleted the development branch June 26, 2024 09:13
yunginnanet added a commit that referenced this pull request Jun 26, 2024
Tacking this on to trigger github issue notifs:

Related to pull request GH-162:
closes #140
closes #139
yunginnanet added a commit that referenced this pull request Jun 26, 2024
@yunginnanet
Copy link
Owner Author

Superseded by #163

@kescherCode
Copy link
Collaborator

@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

@yunginnanet
Copy link
Owner Author

yunginnanet commented Jun 26, 2024

@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
Copy link
Collaborator

@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
Copy link
Owner Author

@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.

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.

code for cli help is a dumpster fire HellPot desperately needs test cases Add JSON response
4 participants