Close security audit low findings and migrate container builds to justfile #53

Merged
coding-agent-marvin8 merged 0 commits from refs/pull/53/head into main 2026-06-22 09:40:04 +00:00
coding-agent-marvin8 commented 2026-06-22 08:17:04 +00:00 (Migrated from codeberg.org)

Closes #52

Summary

  • L4 (accepted): Bootstrap API key is never surfaced via stdout — finding doesn't apply to current implementation; marked accepted/won't-fix in audit docs.
  • L1 (resolved): os.umask(0o027) added to entrypoint.sh before exec, so log files are created as 0o640 rather than world-readable. One test added.
  • L2 (resolved): logger.info added at six API mutation points (key generate/revoke, blocked user/hashtag add/delete), each including client IP. Six tests added.
  • Container builds: fenliu-docker-publish.yml and zhongli-container.yml deleted from Woodpecker; publish-container recipe added to each justfile using podman (amd64, Codeberg only).

Test plan

  • uv run --directory packages/fenliu tryke test passes with zero failures
  • uv run --directory packages/fenliu ruff check . clean
  • uv run --directory packages/fenliu ty check . clean
  • All audit items L1, L2, L4 marked done in Security-Audit.md
Closes #52 ## Summary - **L4 (accepted):** Bootstrap API key is never surfaced via stdout — finding doesn't apply to current implementation; marked accepted/won't-fix in audit docs. - **L1 (resolved):** `os.umask(0o027)` added to `entrypoint.sh` before exec, so log files are created as `0o640` rather than world-readable. One test added. - **L2 (resolved):** `logger.info` added at six API mutation points (key generate/revoke, blocked user/hashtag add/delete), each including client IP. Six tests added. - **Container builds:** `fenliu-docker-publish.yml` and `zhongli-container.yml` deleted from Woodpecker; `publish-container` recipe added to each justfile using `podman` (amd64, Codeberg only). ## Test plan - [ ] `uv run --directory packages/fenliu tryke test` passes with zero failures - [ ] `uv run --directory packages/fenliu ruff check .` clean - [ ] `uv run --directory packages/fenliu ty check .` clean - [ ] All audit items L1, L2, L4 marked done in `Security-Audit.md`
coding-agent-marvin8 commented 2026-06-22 09:31:03 +00:00 (Migrated from codeberg.org)

packages/fenliu/tests/test_l2_audit_logging.py line 63

Test asserts the log message contains 'generated' but does not verify that the client IP is included…

Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit ef69f49 — each of the six test functions now additionally asserts mock_logger.info.call_args[0][-1] == "10.0.0.1" (the last positional arg is always client_ip for every log call in the implementation).

[`packages/fenliu/tests/test_l2_audit_logging.py` line 63](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874194) > Test asserts the log message contains 'generated' but does not verify that the client IP is included… ✅ Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit `ef69f49` — each of the six test functions now additionally asserts `mock_logger.info.call_args[0][-1] == "10.0.0.1"` (the last positional arg is always `client_ip` for every log call in the implementation).
coding-agent-marvin8 commented 2026-06-22 09:31:10 +00:00 (Migrated from codeberg.org)

packages/fenliu/tests/test_l2_audit_logging.py line 79

Test asserts the log message contains 'revoked' but does not verify that the client IP is included…

Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit ef69f49 — each of the six test functions now additionally asserts mock_logger.info.call_args[0][-1] == "10.0.0.1" (the last positional arg is always client_ip for every log call in the implementation).

[`packages/fenliu/tests/test_l2_audit_logging.py` line 79](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874197) > Test asserts the log message contains 'revoked' but does not verify that the client IP is included… ✅ Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit `ef69f49` — each of the six test functions now additionally asserts `mock_logger.info.call_args[0][-1] == "10.0.0.1"` (the last positional arg is always `client_ip` for every log call in the implementation).
coding-agent-marvin8 commented 2026-06-22 09:31:16 +00:00 (Migrated from codeberg.org)

packages/fenliu/tests/test_l2_audit_logging.py line 106

Test asserts the log message contains 'blocked user added' but does not verify that the client IP is included…

Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit ef69f49 — each of the six test functions now additionally asserts mock_logger.info.call_args[0][-1] == "10.0.0.1" (the last positional arg is always client_ip for every log call in the implementation).

[`packages/fenliu/tests/test_l2_audit_logging.py` line 106](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874200) > Test asserts the log message contains 'blocked user added' but does not verify that the client IP is included… ✅ Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit `ef69f49` — each of the six test functions now additionally asserts `mock_logger.info.call_args[0][-1] == "10.0.0.1"` (the last positional arg is always `client_ip` for every log call in the implementation).
coding-agent-marvin8 commented 2026-06-22 09:31:23 +00:00 (Migrated from codeberg.org)

packages/fenliu/tests/test_l2_audit_logging.py line 127

Test asserts the log message contains 'blocked user deleted' but does not verify that the client IP is included…

Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit ef69f49 — each of the six test functions now additionally asserts mock_logger.info.call_args[0][-1] == "10.0.0.1" (the last positional arg is always client_ip for every log call in the implementation).

[`packages/fenliu/tests/test_l2_audit_logging.py` line 127](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874203) > Test asserts the log message contains 'blocked user deleted' but does not verify that the client IP is included… ✅ Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit `ef69f49` — each of the six test functions now additionally asserts `mock_logger.info.call_args[0][-1] == "10.0.0.1"` (the last positional arg is always `client_ip` for every log call in the implementation).
coding-agent-marvin8 commented 2026-06-22 09:31:30 +00:00 (Migrated from codeberg.org)

packages/fenliu/tests/test_l2_audit_logging.py line 154

Test asserts the log message contains 'blocked hashtag added' but does not verify that the client IP is included…

Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit ef69f49 — each of the six test functions now additionally asserts mock_logger.info.call_args[0][-1] == "10.0.0.1" (the last positional arg is always client_ip for every log call in the implementation).

[`packages/fenliu/tests/test_l2_audit_logging.py` line 154](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874206) > Test asserts the log message contains 'blocked hashtag added' but does not verify that the client IP is included… ✅ Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit `ef69f49` — each of the six test functions now additionally asserts `mock_logger.info.call_args[0][-1] == "10.0.0.1"` (the last positional arg is always `client_ip` for every log call in the implementation).
coding-agent-marvin8 commented 2026-06-22 09:31:36 +00:00 (Migrated from codeberg.org)

packages/fenliu/tests/test_l2_audit_logging.py line 175

Test asserts the log message contains 'blocked hashtag deleted' but does not verify that the client IP is included…

Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit ef69f49 — each of the six test functions now additionally asserts mock_logger.info.call_args[0][-1] == "10.0.0.1" (the last positional arg is always client_ip for every log call in the implementation).

[`packages/fenliu/tests/test_l2_audit_logging.py` line 175](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874209) > Test asserts the log message contains 'blocked hashtag deleted' but does not verify that the client IP is included… ✅ Valid observation. The original tests verified the format-string keyword but not the value of the client IP argument. Fixed in commit `ef69f49` — each of the six test functions now additionally asserts `mock_logger.info.call_args[0][-1] == "10.0.0.1"` (the last positional arg is always `client_ip` for every log call in the implementation).
coding-agent-marvin8 commented 2026-06-22 09:31:43 +00:00 (Migrated from codeberg.org)

packages/fenliu/tests/test_l2_audit_logging.py line 61

The test only asserts that logger.info is called once and that the format string contains a keyword… It does not verify that the client IP argument is correctly passed.

Fixed in commit ef69f49 — all six test functions now assert mock_logger.info.call_args[0][-1] == "10.0.0.1". The last positional arg is client_ip for every log call, so this one pattern covers all six handlers.

[`packages/fenliu/tests/test_l2_audit_logging.py` line 61](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17875649) > The test only asserts that logger.info is called once and that the format string contains a keyword… It does not verify that the client IP argument is correctly passed. ✅ Fixed in commit `ef69f49` — all six test functions now assert `mock_logger.info.call_args[0][-1] == "10.0.0.1"`. The last positional arg is `client_ip` for every log call, so this one pattern covers all six handlers.
coding-agent-marvin8 commented 2026-06-22 09:31:50 +00:00 (Migrated from codeberg.org)

packages/fenliu/Security-Audit-detail.md line 481

The 'Status' line for L1 appears twice with the same value.

Fixed in commit ef69f49 — the redundant **Status:** ✅ RESOLVED inside the Resolution section has been removed. The finding header already carries the status.

[`packages/fenliu/Security-Audit-detail.md` line 481](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874212) > The 'Status' line for L1 appears twice with the same value. ✅ Fixed in commit `ef69f49` — the redundant `**Status:** ✅ RESOLVED` inside the Resolution section has been removed. The finding header already carries the status.
coding-agent-marvin8 commented 2026-06-22 09:31:56 +00:00 (Migrated from codeberg.org)

packages/fenliu/Security-Audit-detail.md line 481

The 'Status: RESOLVED' line is a duplicate.

Fixed in commit ef69f49 — duplicate removed from the Resolution section.

[`packages/fenliu/Security-Audit-detail.md` line 481](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17875652) > The '**Status:** ✅ RESOLVED' line is a duplicate. ✅ Fixed in commit `ef69f49` — duplicate removed from the Resolution section.
coding-agent-marvin8 commented 2026-06-22 09:32:03 +00:00 (Migrated from codeberg.org)

packages/fenliu/Security-Audit-detail.md line 539

Inconsistent status phrasing: 'Accepted' versus 'Accepted / Won't Fix'…

Fixed in commit ef69f49 — L3 header status unified to Accepted / Won't Fix to match L4.

[`packages/fenliu/Security-Audit-detail.md` line 539](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874215) > Inconsistent status phrasing: 'Accepted' versus 'Accepted / Won't Fix'… ✅ Fixed in commit `ef69f49` — L3 header status unified to `Accepted / Won't Fix` to match L4.
coding-agent-marvin8 commented 2026-06-22 09:32:10 +00:00 (Migrated from codeberg.org)

packages/fenliu/Security-Audit-detail.md line 556

Inconsistent status format: line 591 uses 'Accepted / Won't Fix' while line 574 uses 'Accepted'…

Fixed in commit ef69f49 — L3 header unified to Accepted / Won't Fix.

[`packages/fenliu/Security-Audit-detail.md` line 556](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17875655) > Inconsistent status format: line 591 uses 'Accepted / Won't Fix' while line 574 uses 'Accepted'… ✅ Fixed in commit `ef69f49` — L3 header unified to `Accepted / Won't Fix`.
coding-agent-marvin8 commented 2026-06-22 09:32:16 +00:00 (Migrated from codeberg.org)

packages/fenliu/Security-Audit-detail.md line 561

The resolution states 'No code change required' but the status says 'Accepted / Won't Fix', which might imply a risk acceptance decision was made…

Text clarified in commit ef69f49. The resolution now states explicitly that the finding does not apply — the described vulnerability (API key printed to stdout) was never present in the implementation. There is no risk to accept. Accepted / Won't Fix is kept as the standard audit disposition for a finding closed without a code change, but the body now makes clear the reason is inapplicability rather than risk tolerance.

[`packages/fenliu/Security-Audit-detail.md` line 561](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874218) > The resolution states 'No code change required' but the status says 'Accepted / Won't Fix', which might imply a risk acceptance decision was made… ✅ Text clarified in commit `ef69f49`. The resolution now states explicitly that the finding **does not apply** — the described vulnerability (API key printed to stdout) was never present in the implementation. There is no risk to accept. `Accepted / Won't Fix` is kept as the standard audit disposition for a finding closed without a code change, but the body now makes clear the reason is inapplicability rather than risk tolerance.
coding-agent-marvin8 commented 2026-06-22 09:32:23 +00:00 (Migrated from codeberg.org)

packages/fenliu/Security-Audit-detail.md line 543

The message format for blocked hashtag add uses '%s' for a single value, while other entries show multiple arguments.

🔴 Not actioned — the format is correct and intentional. add_blocked_hashtag logs two arguments: the hashtag name and the client IP ("Blocked hashtag added: %s; client=%s"). add_blocked_user logs three (identifier, pattern_type, client IP) because user blocks carry a pattern_type field that hashtag blocks do not have. The argument count differs because the handler signatures differ, not because of an inconsistency.

[`packages/fenliu/Security-Audit-detail.md` line 543](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17874221) > The message format for blocked hashtag add uses '%s' for a single value, while other entries show multiple arguments. 🔴 Not actioned — the format is correct and intentional. `add_blocked_hashtag` logs two arguments: the hashtag name and the client IP (`"Blocked hashtag added: %s; client=%s"`). `add_blocked_user` logs three (identifier, pattern_type, client IP) because user blocks carry a `pattern_type` field that hashtag blocks do not have. The argument count differs because the handler signatures differ, not because of an inconsistency.
coding-agent-marvin8 commented 2026-06-22 09:32:30 +00:00 (Migrated from codeberg.org)

packages/fenliu/entrypoint.sh line 31

A05: Security Misconfiguration - The umask is set to 0o027… no critical vulnerability here.

Confirmed. The 0o027 umask is exactly the intended security control for audit finding L1 (log file permissions). Good to have independent validation that the choice is sound.

[`packages/fenliu/entrypoint.sh` line 31](https://codeberg.org/marvinsmastodontools/dujiangyan/pulls/53#issuecomment-17875658) > A05: Security Misconfiguration - The umask is set to 0o027… no critical vulnerability here. ✅ Confirmed. The `0o027` umask is exactly the intended security control for audit finding L1 (log file permissions). Good to have independent validation that the choice is sound.
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
marvin8/dujiangyan!53
No description provided.