Elevate cleanup tasks to high priority in TODO
Mark A2, A5, Q1, Q3 as DO FIRST before bug fixes -- consolidating duplicate functions and the service catalog ensures each bug only needs to be fixed in one place. Update order of attack accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -155,12 +155,13 @@ Generated by codebase audit (2026-06-10). Ranked by severity.
|
|||||||
- Path A chosen. Delete: `setup`, `privileged-setup`, `user-operations`,
|
- Path A chosen. Delete: `setup`, `privileged-setup`, `user-operations`,
|
||||||
`services-improved`.
|
`services-improved`.
|
||||||
|
|
||||||
### A2 -- Three sources of truth for the service catalog [HIGH]
|
### A2 -- Three sources of truth for the service catalog [HIGH] -- DO FIRST
|
||||||
- `services`: `get_service_ports()` + inline image strings (CANONICAL)
|
- `services`: `get_service_ports()` + inline image strings (CANONICAL)
|
||||||
- `services-improved`: scheduled for deletion (Path B)
|
- `services-improved`: scheduled for deletion (Path B)
|
||||||
- `lib/docker.sh`: `HOPS_SERVICES` array -- reconcile or remove duplicates
|
- `lib/docker.sh`: `HOPS_SERVICES` array -- reconcile or remove duplicates
|
||||||
- Fix: `lib/docker.sh` service maps must match `services`; remove anything
|
- Fix: `lib/docker.sh` service maps must match `services`; remove anything
|
||||||
only used by Path B.
|
only used by Path B.
|
||||||
|
- Must be done before port collision fix (B7) so there is one authoritative map.
|
||||||
|
|
||||||
### A3 -- `lib/privileges.sh` is dead code [MED] -- RESOLVED: delete
|
### A3 -- `lib/privileges.sh` is dead code [MED] -- RESOLVED: delete
|
||||||
- Path B artifact. Delete it.
|
- Path B artifact. Delete it.
|
||||||
@@ -171,11 +172,12 @@ Generated by codebase audit (2026-06-10). Ranked by severity.
|
|||||||
- Fix passphrase-on-command-line exposure (S1, S2).
|
- Fix passphrase-on-command-line exposure (S1, S2).
|
||||||
- Wire encrypt/decrypt calls into `install` flow.
|
- Wire encrypt/decrypt calls into `install` flow.
|
||||||
|
|
||||||
### A5 -- `hops` duplicates functions from `lib/common.sh` [MED]
|
### A5 -- `hops` duplicates functions from `lib/common.sh` [HIGH] -- DO FIRST
|
||||||
- `log`, `error_exit`, `warning`, `success`, `info`, `validate_timezone`,
|
- `log`, `error_exit`, `warning`, `success`, `info`, `validate_timezone`,
|
||||||
`validate_password`, `generate_secure_password`, `create_docker_networks`,
|
`validate_password`, `generate_secure_password`, `create_docker_networks`,
|
||||||
`get_service_port/image` are all defined twice (or three times).
|
`get_service_port/image` are all defined twice (or three times).
|
||||||
- Fix: source `lib/common.sh` from `hops` and remove local duplicates.
|
- Fix: source `lib/common.sh` from `hops` and remove local duplicates.
|
||||||
|
- Must be done before bug fixes to avoid patching the same logic in multiple places.
|
||||||
|
|
||||||
### A6 -- Caddy is unreachable via the menu [LOW]
|
### A6 -- Caddy is unreachable via the menu [LOW]
|
||||||
- `services` defines `generate_caddy` but the `select_services` menu in
|
- `services` defines `generate_caddy` but the `select_services` menu in
|
||||||
@@ -223,19 +225,20 @@ Generated by codebase audit (2026-06-10). Ranked by severity.
|
|||||||
|
|
||||||
## CODE QUALITY
|
## CODE QUALITY
|
||||||
|
|
||||||
### Q1 -- Three separate error-handling implementations [MED]
|
### Q1 -- Three separate error-handling implementations [HIGH] -- DO FIRST
|
||||||
- `hops`, `uninstall`, and `lib/common.sh` each define their own `error_exit`
|
- `hops`, `uninstall`, and `lib/common.sh` each define their own `error_exit`
|
||||||
and `log` with different formats. Consolidate in `lib/common.sh`.
|
and `log` with different formats. Consolidate in `lib/common.sh`.
|
||||||
|
- Covered by A5; tracked here for completeness.
|
||||||
|
|
||||||
### Q2 -- `set -e` + intentional non-zero returns is a minefield [MED]
|
### Q2 -- `set -e` + intentional non-zero returns is a minefield [MED]
|
||||||
- `validate_password` returns 1/2/3, `check_port` returns 1 -- these work only
|
- `validate_password` returns 1/2/3, `check_port` returns 1 -- these work only
|
||||||
because they happen to be in conditionals. Combined with B5 this is fragile.
|
because they happen to be in conditionals. Combined with B5 this is fragile.
|
||||||
Consider `set -euo pipefail` with explicit `|| true` where non-zero is intended.
|
Consider `set -euo pipefail` with explicit `|| true` where non-zero is intended.
|
||||||
|
|
||||||
### Q3 -- Debug `echo` statements left in production code [LOW]
|
### Q3 -- Debug `echo` statements left in production code [MED] -- DO FIRST
|
||||||
- Files: `lib/system.sh:605,823,1043,1046,1084,1089,1149-1156`,
|
- Files: `lib/system.sh:605,823,1043,1046,1084,1089,1149-1156`
|
||||||
`privileged-setup:72`
|
|
||||||
- `DEBUG:` prefixed echo lines should be removed or gated behind a `$DEBUG` flag.
|
- `DEBUG:` prefixed echo lines should be removed or gated behind a `$DEBUG` flag.
|
||||||
|
- Clean these before bug fixes so signal isn't buried in debug noise.
|
||||||
|
|
||||||
### Q4 -- `services-improved` leaks `set -e` when sourced [LOW]
|
### Q4 -- `services-improved` leaks `set -e` when sourced [LOW]
|
||||||
- File: `services-improved` top of file
|
- File: `services-improved` top of file
|
||||||
@@ -246,18 +249,25 @@ Generated by codebase audit (2026-06-10). Ranked by severity.
|
|||||||
|
|
||||||
## SUGGESTED ORDER OF ATTACK
|
## SUGGESTED ORDER OF ATTACK
|
||||||
|
|
||||||
1. Delete Path B files: `setup`, `privileged-setup`, `user-operations`,
|
### Cleanup first (do before any bug fixes)
|
||||||
`services-improved`, `lib/privileges.sh` (A1/A3 -- resolved)
|
1. [DONE] Delete Path B files (A1/A3)
|
||||||
2. Fix B1 (infinite recursion in `services`) -- unblocks all Linux installs
|
2. Consolidate duplicate functions into `lib/common.sh` (A5, Q1) -- one copy to fix
|
||||||
3. Fix B5 (`((x++))` under `set -e`) -- prevents silent aborts
|
3. Reconcile `lib/docker.sh` service maps with `services` (A2) -- one catalog to fix
|
||||||
4. Fix B3 (glob directory detection) -- fixes multi-user and uninstall
|
4. Remove debug echo statements from `lib/system.sh` (Q3) -- reduce noise
|
||||||
5. Fix B4 (wrong filename in firewall setup)
|
|
||||||
6. Reconcile `lib/docker.sh` service maps with `services` (A2)
|
### Bug fixes
|
||||||
7. Security pass: S3 (default Authelia cred), S2/S6 (passphrase on cmdline,
|
5. Fix B1 (infinite recursion in `services`) -- unblocks all Linux installs
|
||||||
mktemp), S5 (eval on env var), S7 (string-built commands)
|
6. Fix B5 (`((x++))` under `set -e`) -- prevents silent aborts
|
||||||
8. Fix and wire in `lib/secrets.sh`: replace broken crypto, hook into install
|
7. Fix B3 (glob directory detection) -- fixes multi-user and uninstall
|
||||||
|
8. Fix B4 (wrong filename in firewall setup)
|
||||||
|
9. Fix B7 (intra-selection port collision detection)
|
||||||
|
|
||||||
|
### Security pass
|
||||||
|
10. S3 (default Authelia cred), S5 (eval on env var), S6 (mktemp),
|
||||||
|
S7 (string-built commands), S2 (passphrase on cmdline)
|
||||||
|
11. Fix and wire in `lib/secrets.sh`: replace broken crypto, hook into install
|
||||||
flow to encrypt `.env` at rest (A4/S1/S2)
|
flow to encrypt `.env` at rest (A4/S1/S2)
|
||||||
9. Fix B12 empty-password regex, B8 watchtower port, B9 backup self-copy
|
|
||||||
10. Consolidate duplicate functions into `lib/common.sh` (A5, Q1)
|
### Remaining
|
||||||
11. Remove debug echo statements (Q3)
|
12. Fix B12 empty-password regex, B8 watchtower port, B9 backup self-copy
|
||||||
12. macOS / WSL2 support (B6, P4) -- future roadmap
|
13. macOS / WSL2 support (B6, P4) -- future roadmap
|
||||||
|
|||||||
Reference in New Issue
Block a user