didericis/bot-bottle
1
0
Fork 0
Code Issues 7 Pull Requests 15 Actions Packages Projects Releases Wiki Activity

Second audit: smolmachines parity and Git HTTP hardening #134

New Issue
Closed
opened 2026-06-02 04:27:12 -04:00 by didericis-codex · 1 comment
didericis-codex commented 2026-06-02 04:27:12 -04:00
Collaborator
Copy Link
Copy Source

Summary

Second complexity/security audit after closing #117. The codebase is in a stronger state overall, but the next set of issues clusters around smolmachines backend parity, backend-aware dashboard/remediation flows, and one Git HTTP robustness gap.

Codebase rating

7.4/10

The architecture and test posture are solid: PRD discipline is strong, unit coverage is broad, and the recent hotspot work reduced risk in manifest, sidecar supervision, supervise waits, Codex auth redaction, and pipelock rendering. The main reason this is not an 8+ is backend parity. Docker is mature; smolmachines still has Docker assumptions and env-handling differences in important operator paths.

Unit status at audit time:

  • python3 -m unittest discover -s tests/unit passed: 826 tests.

Revised hotspot ranking

Rank Area Risk Reason Tracking Status
1 backend/smolmachines/prepare.py + smolvm.py + bottle.py env flow High Manifest env entries bypass resolve_env(); secret/interpolated env handling diverges from Docker and values are passed on smolvm argv. PRD 0038 ✅ Merged (#141)
2 smolmachines supervise/capability remediation High smolmachines agents get supervise tools, but capability-block approval calls Docker-only teardown/snapshot/apply code. PRD 0039 ✅ Merged (#142)
3 backend-aware resume/dashboard attach Medium metadata does not persist backend, resume defaults through backend selection, and dashboard reattach synthesizes Docker bottles for external slugs. PRD 0040 ✅ Merged (#143)
4 git_http_backend.py request handling Medium Smart HTTP wrapper reads arbitrary Content-Length into memory and does not handle malformed lengths. PRD 0041 ✅ Merged (#144)
5 smolmachines parity tests Medium No cross-backend test coverage to prove secrets, forwarded env, resume, and remediation behave the same way across Docker and smolmachines. Fixes to ranks 1–3 are unverifiable without this. #139 ✅ Merged (#145)
6 sidecar_init.py pipe lifecycle Low Unit suite passes but emits ResourceWarning for unclosed child stdout pipes on restart paths. #140 ✅ Merged (#146)

Follow-up rating (2026-06-02)

8.1/10

All six findings are resolved and merged. Test count is up to 855 (from 826). The backend parity gap that held the previous rating below 8 is closed: smolmachines env now goes through resolve_env(), capability-block remediation is backend-aware, resume and dashboard reattach use persisted backend metadata, and cross-backend parity tests enforce the contract.

The remaining known gap — smolmachines env values reaching the guest via -e K=V on smolvm host argv — is explicitly documented in prepare.py as a limitation of smolvm 0.8.0 (no env-file or stdin injection path). It is a lower-severity exposure than the previous high-risk findings since values are on the host process table, not inside the agent sandbox, and are short-lived.

Unit status:

  • python3 -m unittest discover -s tests/unit passed: 855 tests.

New hotspot ranking

Rank Area Risk Reason Suggested tracking
1 smolmachines guest env on host argv Low-Medium smolvm machine create -e K=V exposes resolved env values (including secrets) on the host process table. Documented known gap pending smolvm env-file support. New PRD when smolvm gains env-file or stdin injection
2 BottlePlan.print rendering divergence Low git gate lines and egress auth annotation differ between backends. Tracked. PRD 0044 (#147) ✅
3 git_http_backend.py access hook argv Low client_address[0] is passed as both the remote-addr and peer-addr argument to the access hook — likely a copy-paste, functionally harmless but worth verifying intent. #148 ✅
## Summary Second complexity/security audit after closing #117. The codebase is in a stronger state overall, but the next set of issues clusters around smolmachines backend parity, backend-aware dashboard/remediation flows, and one Git HTTP robustness gap. ## Codebase rating **7.4/10** The architecture and test posture are solid: PRD discipline is strong, unit coverage is broad, and the recent hotspot work reduced risk in manifest, sidecar supervision, supervise waits, Codex auth redaction, and pipelock rendering. The main reason this is not an 8+ is backend parity. Docker is mature; smolmachines still has Docker assumptions and env-handling differences in important operator paths. Unit status at audit time: - `python3 -m unittest discover -s tests/unit` passed: 826 tests. ## Revised hotspot ranking | Rank | Area | Risk | Reason | Tracking | Status | |---|---|---|---|---|---| | 1 | `backend/smolmachines/prepare.py` + `smolvm.py` + `bottle.py` env flow | High | Manifest env entries bypass `resolve_env()`; secret/interpolated env handling diverges from Docker and values are passed on smolvm argv. | PRD 0038 | ✅ Merged (#141) | | 2 | smolmachines supervise/capability remediation | High | smolmachines agents get supervise tools, but capability-block approval calls Docker-only teardown/snapshot/apply code. | PRD 0039 | ✅ Merged (#142) | | 3 | backend-aware resume/dashboard attach | Medium | metadata does not persist backend, `resume` defaults through backend selection, and dashboard reattach synthesizes Docker bottles for external slugs. | PRD 0040 | ✅ Merged (#143) | | 4 | `git_http_backend.py` request handling | Medium | Smart HTTP wrapper reads arbitrary Content-Length into memory and does not handle malformed lengths. | PRD 0041 | ✅ Merged (#144) | | 5 | smolmachines parity tests | Medium | No cross-backend test coverage to prove secrets, forwarded env, resume, and remediation behave the same way across Docker and smolmachines. Fixes to ranks 1–3 are unverifiable without this. | #139 | ✅ Merged (#145) | | 6 | `sidecar_init.py` pipe lifecycle | Low | Unit suite passes but emits ResourceWarning for unclosed child stdout pipes on restart paths. | #140 | ✅ Merged (#146) | --- ## Follow-up rating (2026-06-02) **8.1/10** All six findings are resolved and merged. Test count is up to 855 (from 826). The backend parity gap that held the previous rating below 8 is closed: smolmachines env now goes through `resolve_env()`, capability-block remediation is backend-aware, resume and dashboard reattach use persisted backend metadata, and cross-backend parity tests enforce the contract. The remaining known gap — smolmachines env values reaching the guest via `-e K=V` on `smolvm` host argv — is explicitly documented in `prepare.py` as a limitation of smolvm 0.8.0 (no env-file or stdin injection path). It is a lower-severity exposure than the previous high-risk findings since values are on the host process table, not inside the agent sandbox, and are short-lived. Unit status: - `python3 -m unittest discover -s tests/unit` passed: 855 tests. ## New hotspot ranking | Rank | Area | Risk | Reason | Suggested tracking | |---|---|---|---|---| | 1 | smolmachines guest env on host argv | Low-Medium | `smolvm machine create -e K=V` exposes resolved env values (including secrets) on the host process table. Documented known gap pending smolvm env-file support. | New PRD when smolvm gains env-file or stdin injection | | 2 | `BottlePlan.print` rendering divergence | Low | git gate lines and egress auth annotation differ between backends. Tracked. | PRD 0044 (#147) ✅ | | 3 | `git_http_backend.py` access hook argv | Low | `client_address[0]` is passed as both the remote-addr and peer-addr argument to the access hook — likely a copy-paste, functionally harmless but worth verifying intent. | #148 ✅ |
didericis-codex added the Kind/EnhancementKind/Security labels 2026-06-02 04:27:12 -04:00
didericis-claude was assigned by didericis 2026-06-02 11:22:30 -04:00
didericis-claude commented 2026-06-02 12:18:21 -04:00
Collaborator
Copy Link
Copy Source

Follow-up rating (2026-06-02)

8.6/10

PRD 0044 (#147) merged: BottlePlan.print rendering divergence is resolved. Both backends now inherit a single concrete print implementation from the base class; git gate lines render as name → upstream_host:upstream_port and egress lines carry [auth:scheme] annotations consistently. Seven new cross-backend unit tests enforce the contract.

Test count is up to 862 (from 855).

Of the three items in the new hotspot ranking, two are now closed:

Rank Area Risk Status
1 smolmachines guest env on host argv Low-Medium Open — pending smolvm env-file support
2 BottlePlan.print rendering divergence Low ✅ PRD 0044 (#147)
3 git_http_backend.py access hook argv Low ✅ #148

The remaining gap is a documented smolvm 0.8.0 limitation — resolved env values briefly appear on the host process table via -e K=V argv. The codebase has called this out in prepare.py and cannot fix it until smolvm gains env-file or stdin injection support. It is an external dependency ceiling, not an open codebase risk. The deduction from a higher score reflects the exposure being real even if unactionable here.

## Follow-up rating (2026-06-02) **8.6/10** PRD 0044 (#147) merged: `BottlePlan.print` rendering divergence is resolved. Both backends now inherit a single concrete `print` implementation from the base class; git gate lines render as `name → upstream_host:upstream_port` and egress lines carry `[auth:scheme]` annotations consistently. Seven new cross-backend unit tests enforce the contract. Test count is up to 862 (from 855). Of the three items in the new hotspot ranking, two are now closed: | Rank | Area | Risk | Status | |---|---|---|---| | 1 | smolmachines guest env on host argv | Low-Medium | Open — pending smolvm env-file support | | 2 | `BottlePlan.print` rendering divergence | Low | ✅ PRD 0044 (#147) | | 3 | `git_http_backend.py` access hook argv | Low | ✅ #148 | The remaining gap is a documented smolvm 0.8.0 limitation — resolved env values briefly appear on the host process table via `-e K=V` argv. The codebase has called this out in `prepare.py` and cannot fix it until smolvm gains env-file or stdin injection support. It is an external dependency ceiling, not an open codebase risk. The deduction from a higher score reflects the exposure being real even if unactionable here.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label Kind/Enhancement Kind/Security
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
didericis-claude (didericis (claude)) didericis-codex (didericis (codex)) didericis-gemma (gemma) didericis
No Assignees didericis-claude
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: didericis/bot-bottle#134
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?