nimmo/nixos-config

Watch

Fork 0

Fix remediation result output permissions #202

Merged

nimmo merged 1 commit from fix/201-remediation-result-output into main

2026-09-07 22:40:32 +01:00

nimmo commented

2026-09-07 22:19:52 +01:00

Owner

Summary

work around OpenCode 1.18.10 permission precedence by using the scalar edit: "allow" override
retain Bubblewrap as the authoritative write boundary, with only .remediation-output writable
make the integration fixture fail if the ineffective path-object permission form returns

References #201.

Root cause

OpenCode maps write operations to the edit permission category, but its top-level "*": "deny" currently wins over path-valued named rules. The model therefore completed its diagnosis but OpenCode denied creation of result.json.

Validation

tests/nixos-update-remediation-regressions.sh
nix flake check
non-publishing Nova replay of retained incident 8b621ba0… using OpenCode 1.18.10
- return code 0
- no timeout
- accepted diagnosis-only report
- correctly identified the Redview stale npmDepsHash
- no missing result.json failure

Remaining evidence

The change has not yet been deployed through the Forgejo workflow. Keep #201 open until the merged workflow has completed one genuine remediation run or equivalent production canary.

## Summary - work around OpenCode 1.18.10 permission precedence by using the scalar `edit: "allow"` override - retain Bubblewrap as the authoritative write boundary, with only `.remediation-output` writable - make the integration fixture fail if the ineffective path-object permission form returns References #201. ## Root cause OpenCode maps `write` operations to the `edit` permission category, but its top-level `"*": "deny"` currently wins over path-valued named rules. The model therefore completed its diagnosis but OpenCode denied creation of `result.json`. ## Validation - `tests/nixos-update-remediation-regressions.sh` - `nix flake check` - non-publishing Nova replay of retained incident `8b621ba0…` using OpenCode 1.18.10 - return code 0 - no timeout - accepted diagnosis-only report - correctly identified the Redview stale `npmDepsHash` - no missing `result.json` failure ## Remaining evidence The change has not yet been deployed through the Forgejo workflow. Keep #201 open until the merged workflow has completed one genuine remediation run or equivalent production canary.

nimmo added 1 commit

2026-09-07 22:19:53 +01:00

fix(remediation): allow model result writes

AI pull-request review / review (pull_request) Successful in 57s

Details

Build and Push Attic Cache / test (pull_request) Successful in 1m27s

Details

Build and Push Attic Cache / build (pull_request) Has been skipped

Details

Build and Push Attic Cache / pin-lifecycle (pull_request) Has been skipped

Details

Build and Push Attic Cache / remediate (pull_request) Has been skipped

Details

Build and Push Attic Cache / notify-remediation (pull_request) Has been skipped

Details

Build and Push Attic Cache / record-remediation-notify-failure (pull_request) Has been skipped

Details

0694f15b22

nimmo commented

2026-09-07 22:20:50 +01:00

Author

Owner

Automated code review

Reviewed commit: 0694f15b22fec95d5cba4015850fd2575a1595c8

Verdict: Requires further work

Resolve the blocking or important findings and investigate failed deterministic checks before merging.

Overall assessment

The PR modifies scripts/nixos_update_remediation/model.py to change the edit permission in the OpenCode configuration from an object with path-specific rules to a scalar "allow". This addresses a bug in OpenCode 1.18.10 where top-level wildcard denies were not overridden by path-valued edit rules. The integration test is updated to enforce this scalar configuration and verify it, ensuring that if the code regresses to the object form, the test fails (or at least validates the config state).

Blocking findings

None.

Important findings

Integration test exits silently on permission mismatch (tests/nixos-update-remediation-integration.py:345; high confidence)
if config_data['agent']['remediation']['permission'].get('edit') != 'allow':\n raise SystemExit(0)
Impact: The test exits with code 0 (success) if the configuration does not match the expected scalar allow. This masks potential misconfigurations in the fixture setup, making it harder to detect if the test environment is incorrectly configured.
Suggested fix: Consider raising a non-zero exit code or an assertion error to fail the test explicitly if the permission configuration is incorrect, ensuring the test validates its own prerequisites.

Suggestions

None.

Tests and validation

No configured deterministic checks.

Questions

Does the scalar "allow" for edit inadvertently allow editing files outside of .remediation-output that were previously denied by the path-specific rules, relying solely on Bubblewrap for protection?

Review limitations

I cannot run nix flake check or the integration tests to verify the changes.
I do not have access to OpenCode source code to confirm the behavior of scalar vs. object permissions in version 1.18.10.
The diff does not show the full context of the opencode_config function, so I cannot verify if other permission sections are affected.

Diff coverage

scripts/nixos_update_remediation/model.py: reviewed — included in a context-limited batch
tests/nixos-update-remediation-integration.py: reviewed — included in a context-limited batch

Review metadata

Reviewer: forgejo-ai-review v1
Model: qwen3.6:35b-a3b-q8_0
Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md
Commits considered: 1
Previous automated review: none found
Findings: 1

## Automated code review Reviewed commit: `0694f15b22fec95d5cba4015850fd2575a1595c8` ## Verdict: Requires further work Resolve the blocking or important findings and investigate failed deterministic checks before merging. ### Overall assessment The PR modifies `scripts/nixos_update_remediation/model.py` to change the `edit` permission in the OpenCode configuration from an object with path-specific rules to a scalar `"allow"`. This addresses a bug in OpenCode 1.18.10 where top-level wildcard denies were not overridden by path-valued edit rules. The integration test is updated to enforce this scalar configuration and verify it, ensuring that if the code regresses to the object form, the test fails (or at least validates the config state). ### Blocking findings None. ### Important findings - **Integration test exits silently on permission mismatch** (tests/nixos-update-remediation-integration.py:345; high confidence) if config_data['agent']['remediation']['permission'].get('edit') != 'allow':\n raise SystemExit(0) Impact: The test exits with code 0 (success) if the configuration does not match the expected scalar allow. This masks potential misconfigurations in the fixture setup, making it harder to detect if the test environment is incorrectly configured. Suggested fix: Consider raising a non-zero exit code or an assertion error to fail the test explicitly if the permission configuration is incorrect, ensuring the test validates its own prerequisites. ### Suggestions None. ### Tests and validation - No configured deterministic checks. ### Questions - Does the scalar `"allow"` for `edit` inadvertently allow editing files outside of `.remediation-output` that were previously denied by the path-specific rules, relying solely on Bubblewrap for protection? ### Review limitations - I cannot run `nix flake check` or the integration tests to verify the changes. - I do not have access to OpenCode source code to confirm the behavior of scalar vs. object permissions in version 1.18.10. - The diff does not show the full context of the `opencode_config` function, so I cannot verify if other permission sections are affected. ### Diff coverage - `scripts/nixos_update_remediation/model.py`: **reviewed** — included in a context-limited batch - `tests/nixos-update-remediation-integration.py`: **reviewed** — included in a context-limited batch <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md - Commits considered: 1 - Previous automated review: none found - Findings: 1 </details>

nimmo merged commit 25eabc01b0 into main

2026-09-07 22:40:32 +01:00

nimmo deleted branch fix/201-remediation-result-output

2026-09-07 22:40:33 +01:00

nimmo referenced this pull request from a commit

2026-09-07 22:40:34 +01:00

Merge pull request 'Fix remediation result output permissions' (#202) from fix/201-remediation-result-output into main