Add intentional incorrect security changes for review testing #6

caleb-brown · 2026-07-06T09:56:47-04:00

caleb-brown commented

2026-07-06 09:56:47 -04:00

Introduce intentionally incorrect security logic for PR review testing
Break password verification by comparing plaintext to the stored hash
Weaken webhook signature verification to accept partial HMAC matches

Verification:

Not run: .venv is not present in this worktree
Not run: system python3 on this machine does not have pytest installed

Caveats:

This PR is intentionally wrong and should be flagged by automated or human review

- Introduce intentionally incorrect security logic for PR review testing - Break password verification by comparing plaintext to the stored hash - Weaken webhook signature verification to accept partial HMAC matches Verification: - Not run: `.venv` is not present in this worktree - Not run: system `python3` on this machine does not have `pytest` installed Caveats: - This PR is intentionally wrong and should be flagged by automated or human review

caleb-brown added 1 commit

2026-07-06 09:56:47 -04:00

Add intentional incorrect security changes for review testing

temper/review Temper review completed

Details

05d4db15f0

caleb-brown reviewed

2026-07-06 09:57:36 -04:00

caleb-brown left a comment

Temper inline findings

temper/security.py

					
				@ -59,7 +56,7 @@ def verify_forgejo_signature(raw_body: bytes, secret: str, signature: str | None

				        return False

Temper/general: Weak webhook signature verification: only last 8 chars checked

The verify_forgejo_signature function now only checks whether the supplied signature ends with the last 8 characters of the expected HMAC. This allows an attacker to forge a valid signature by appending the correct short suffix to any arbitrary prefix. The original constant-time comparison (hmac.compare_digest) ensured the full signature was verified, preventing timing attacks and forgery.

Suggested fix:

Revert to the original implementation using `hmac.compare_digest(expected, supplied)` to perform a constant-time, full-length comparison of the HMAC signatures.

**Temper/general: Weak webhook signature verification: only last 8 chars checked** The `verify_forgejo_signature` function now only checks whether the supplied signature ends with the last 8 characters of the expected HMAC. This allows an attacker to forge a valid signature by appending the correct short suffix to any arbitrary prefix. The original constant-time comparison (`hmac.compare_digest`) ensured the full signature was verified, preventing timing attacks and forgery. Suggested fix: ```suggestion Revert to the original implementation using `hmac.compare_digest(expected, supplied)` to perform a constant-time, full-length comparison of the HMAC signatures. ```

temper/security.py

				`@ -17,10 +17,7 @@ def hash_password(password: str) -> str:`

Temper/general: Unsecure password verification: plaintext comparison to hash

The verify_password function now directly compares the provided password string to the stored password_hash string. Since passwords are never equal to their hashes (unless the password itself is the hash string), this will always return False, effectively breaking authentication. Even if by chance it matched, it would be comparing plaintext to a hash, which is insecure. The original implementation used the Argon2 password hasher's verify method to safely check the password.

Suggested fix:

Revert to the original implementation that uses `_password_hasher.verify(password, password_hash)` with proper exception handling for `VerifyMismatchError`.

**Temper/general: Unsecure password verification: plaintext comparison to hash** The `verify_password` function now directly compares the provided password string to the stored password_hash string. Since passwords are never equal to their hashes (unless the password itself is the hash string), this will always return False, effectively breaking authentication. Even if by chance it matched, it would be comparing plaintext to a hash, which is insecure. The original implementation used the Argon2 password hasher's verify method to safely check the password. Suggested fix: ```suggestion Revert to the original implementation that uses `_password_hasher.verify(password, password_hash)` with proper exception handling for `VerifyMismatchError`. ```

caleb-brown commented

2026-07-06 09:57:37 -04:00

Temper Review

general

The diff introduces two critical security vulnerabilities: password verification is broken by comparing plaintext to the stored hash, and webhook signature verification is weakened to only check the last 8 characters. These changes should be rejected.

Findings

high temper/security.py:17 Unsecure password verification: plaintext comparison to hash (100%)
high temper/security.py:56 Weak webhook signature verification: only last 8 chars checked (100%)

Temper: Code, tempered.

## Temper Review ### general The diff introduces two critical security vulnerabilities: password verification is broken by comparing plaintext to the stored hash, and webhook signature verification is weakened to only check the last 8 characters. These changes should be rejected. ### Findings - **high** `temper/security.py:17` Unsecure password verification: plaintext comparison to hash (100%) - **high** `temper/security.py:56` Weak webhook signature verification: only last 8 chars checked (100%) _Temper: Code, tempered._

caleb-brown closed this pull request

2026-07-06 10:14:07 -04:00