feat(gateway): add CI-triggered review dispatch #48

jwilger · 2026-05-02T23:03:54-07:00

jwilger commented

2026-05-02 23:03:54 -07:00

Summary

Add optional POST /reviews/ci gateway endpoint for Forgejo Actions to trigger semantic reviews after required CI checks pass.
Verify bearer action token, safe owner/repo path segments, current PR head SHA, and reviewable PR state before dispatching.
Document the CI-triggered workflow, token configuration, observability boundary, and threat-model changes.

Validation

nix develop -c cargo fmt --all -- --check
nix develop -c cargo nextest run -p ar-gateway
nix develop -c cargo clippy --workspace --all-targets -- -D warnings
env -u FORGEJO_BASE_URL -u FORGEJO_TOKEN -u LLM_BASE_URL -u LLM_API_KEY -u LLM_REASONING_MODEL -u LLM_CHEAP_MODEL -u LLM_EMBEDDING_MODEL nix develop -c cargo nextest run --workspace --no-tests=pass

Closes #42.

## Summary - Add optional `POST /reviews/ci` gateway endpoint for Forgejo Actions to trigger semantic reviews after required CI checks pass. - Verify bearer action token, safe owner/repo path segments, current PR head SHA, and reviewable PR state before dispatching. - Document the CI-triggered workflow, token configuration, observability boundary, and threat-model changes. ## Validation - `nix develop -c cargo fmt --all -- --check` - `nix develop -c cargo nextest run -p ar-gateway` - `nix develop -c cargo clippy --workspace --all-targets -- -D warnings` - `env -u FORGEJO_BASE_URL -u FORGEJO_TOKEN -u LLM_BASE_URL -u LLM_API_KEY -u LLM_REASONING_MODEL -u LLM_CHEAP_MODEL -u LLM_EMBEDDING_MODEL nix develop -c cargo nextest run --workspace --no-tests=pass` Closes #42.

jwilger added 1 commit

2026-05-02 23:03:54 -07:00

feat(gateway): add CI-triggered review dispatch

auto_review auto_review: 1 error, 1 warning

CI / Nix flake check (pull_request) Successful in 1m42s

Details

d4c82d9424

auto-review requested changes

2026-05-02 23:04:38 -07:00

Dismissed

auto-review left a comment

This PR introduces a new CI-triggered review dispatch endpoint in the ar-gateway crate, allowing Forgejo Actions to request semantic reviews after CI checks pass. The implementation includes token validation and PR state verification to ensure secure and accurate dispatching. Ensure that sensitive tokens are securely stored and not exposed in logs or error messages.

Walkthrough

New CI-triggered Review Endpoint:
- Added a new POST /reviews/ci endpoint in ar-gateway to allow Forgejo Actions to trigger reviews.
- The endpoint requires a bearer token (AR_CI_REVIEW_TOKEN) and verifies the current PR head SHA before dispatching.
- Includes validation for safe owner/repo path segments and reviewable PR state.
Security Considerations:
- Ensure that the AR_CI_REVIEW_TOKEN is securely stored and not exposed in logs or error messages.
- Consider adding logging for unauthorized access attempts for auditing purposes.
- Ensure that sensitive information is not logged or exposed in error messages.
Documentation Updates:
- Updated QUICKSTART.md, README.md, and other documentation files to include information about the new endpoint and its configuration.
- Added examples for configuring Forgejo Actions to use the new endpoint.

Pre-merge checks

CHANGELOG updated — CHANGELOG.md is in the diff
Tests touched — test changes are in the diff
No new TODO/FIXME comments — no new TODO/FIXME markers

Linters

gitleaks — ok, 8 findings
semgrep — ok, 0 findings
trivy — ok, 0 findings
osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
ast-grep — ok, 0 findings
typos — ok, 25 findings

This PR introduces a new CI-triggered review dispatch endpoint in the `ar-gateway` crate, allowing Forgejo Actions to request semantic reviews after CI checks pass. The implementation includes token validation and PR state verification to ensure secure and accurate dispatching. Ensure that sensitive tokens are securely stored and not exposed in logs or error messages. ## Walkthrough - **New CI-triggered Review Endpoint**: - Added a new `POST /reviews/ci` endpoint in `ar-gateway` to allow Forgejo Actions to trigger reviews. - The endpoint requires a bearer token (`AR_CI_REVIEW_TOKEN`) and verifies the current PR head SHA before dispatching. - Includes validation for safe owner/repo path segments and reviewable PR state. - **Security Considerations**: - Ensure that the `AR_CI_REVIEW_TOKEN` is securely stored and not exposed in logs or error messages. - Consider adding logging for unauthorized access attempts for auditing purposes. - Ensure that sensitive information is not logged or exposed in error messages. - **Documentation Updates**: - Updated `QUICKSTART.md`, `README.md`, and other documentation files to include information about the new endpoint and its configuration. - Added examples for configuring Forgejo Actions to use the new endpoint. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>

crates/ar-gateway/src/main.rs Outdated

					
				@ -682,3 +706,44 @@ fn autodetect_oci_runtime() -> String {

				    }

🔴 Error: The validate_ci_review_token function should ensure that the token is securely handled and not logged or exposed in error messages. Consider using secure storage mechanisms for sensitive tokens.

🔴 **Error:** The `validate_ci_review_token` function should ensure that the token is securely handled and not logged or exposed in error messages. Consider using secure storage mechanisms for sensitive tokens.

Addressed in bf18cba by adding a regression assertion that the validation error names AR_CI_REVIEW_TOKEN and the strength requirement without echoing the rejected token value.

Reflection/classification: one-off coverage gap. The implementation already avoided logging or returning the token, but the security property was implicit rather than pinned by a focused test.

Addressed in bf18cba by adding a regression assertion that the validation error names `AR_CI_REVIEW_TOKEN` and the strength requirement without echoing the rejected token value. Reflection/classification: one-off coverage gap. The implementation already avoided logging or returning the token, but the security property was implicit rather than pinned by a focused test.

jwilger marked this conversation as resolved

crates/ar-gateway/src/webhook.rs Outdated

					
				@ -193,6 +195,85 @@ fn is_bot_self(sender_login: &str, bot_login: &str) -> bool {

				    sender_login.eq_ignore_ascii_case(bot_login)

🟡 Warning: The handle_ci_review function should ensure that sensitive information is not logged or exposed in error messages. Consider sanitizing inputs and outputs to prevent information leakage.

🟡 **Warning:** The `handle_ci_review` function should ensure that sensitive information is not logged or exposed in error messages. Consider sanitizing inputs and outputs to prevent information leakage.

Addressed in bf18cba by extending the unauthorized /reviews/ci test to assert neither the configured action token nor the rejected bearer token appears in the response body, while still proving no Forgejo fetch or dispatch occurs.

Reflection/classification: one-off coverage gap. The handler already returned a generic unauthorized response and hid upstream Forgejo bodies, but the no-token-leak guarantee needed explicit regression coverage.

Addressed in bf18cba by extending the unauthorized `/reviews/ci` test to assert neither the configured action token nor the rejected bearer token appears in the response body, while still proving no Forgejo fetch or dispatch occurs. Reflection/classification: one-off coverage gap. The handler already returned a generic `unauthorized` response and hid upstream Forgejo bodies, but the no-token-leak guarantee needed explicit regression coverage.

jwilger marked this conversation as resolved

jwilger added 1 commit

2026-05-02 23:09:30 -07:00

test(gateway): cover CI token leak guards

CI / Nix flake check (pull_request) Successful in 1m45s

Details

auto_review auto_review: 2 warnings

bf18cbaa6c

auto-review reviewed

2026-05-02 23:10:13 -07:00

auto-review left a comment

This PR introduces a new CI-triggered review dispatch endpoint in the ar-gateway crate, allowing Forgejo Actions to request semantic reviews after CI checks pass. The implementation appears robust, with appropriate validation and tests, but could benefit from additional documentation and logging for better maintainability and debugging.

Walkthrough

CHANGELOG.md: Documents the addition of the CI-triggered review endpoint. Consider adding details about token requirements.
QUICKSTART.md: Introduces AR_CI_REVIEW_TOKEN but lacks details on token strength requirements.
crates/ar-gateway/src/lib.rs: Adds ci_review_endpoint to AppState and a method to configure it. Documentation is sparse.
crates/ar-gateway/src/main.rs: Implements token validation with a minimum length check. Consider logging when the token is missing or invalid.
crates/ar-gateway/src/webhook.rs: Implements the handle_ci_review function with constant-time token comparison. Logging for authorization failures could be improved.
Tests: Comprehensive tests for the new endpoint, but additional logging and edge case tests could enhance coverage.

Pre-merge checks

CHANGELOG updated — CHANGELOG.md is in the diff
Tests touched — test changes are in the diff
No new TODO/FIXME comments — no new TODO/FIXME markers

Linters

gitleaks — ok, 8 findings
semgrep — ok, 0 findings
trivy — ok, 0 findings
osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
ast-grep — ok, 0 findings
typos — ok, 25 findings

This PR introduces a new CI-triggered review dispatch endpoint in the `ar-gateway` crate, allowing Forgejo Actions to request semantic reviews after CI checks pass. The implementation appears robust, with appropriate validation and tests, but could benefit from additional documentation and logging for better maintainability and debugging. ## Walkthrough - **CHANGELOG.md**: Documents the addition of the CI-triggered review endpoint. Consider adding details about token requirements. - **QUICKSTART.md**: Introduces `AR_CI_REVIEW_TOKEN` but lacks details on token strength requirements. - **crates/ar-gateway/src/lib.rs**: Adds `ci_review_endpoint` to `AppState` and a method to configure it. Documentation is sparse. - **crates/ar-gateway/src/main.rs**: Implements token validation with a minimum length check. Consider logging when the token is missing or invalid. - **crates/ar-gateway/src/webhook.rs**: Implements the `handle_ci_review` function with constant-time token comparison. Logging for authorization failures could be improved. - **Tests**: Comprehensive tests for the new endpoint, but additional logging and edge case tests could enhance coverage. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>

jwilger requested review from auto-review

2026-05-02 23:21:12 -07:00

jwilger commented

2026-05-03 08:29:56 -07:00

@auto-review re-review

jwilger referenced this pull request

2026-05-03 08:29:58 -07:00

Handle review requests from Forgejo #49

jwilger commented

2026-05-03 08:32:41 -07:00

@auto_review re-review

auto-review commented

2026-05-03 08:32:42 -07:00

Queued a fresh review at bf18cbaa6c. Watch the commit-status badge for progress.

Queued a fresh review at bf18cbaa6c60edf7a73cf3542b66e84ec96ab85c. Watch the commit-status badge for progress.

auto-review commented

2026-05-03 08:32:43 -07:00

Queued a fresh review at bf18cbaa6c. Watch the commit-status badge for progress.

Queued a fresh review at bf18cbaa6c60edf7a73cf3542b66e84ec96ab85c. Watch the commit-status badge for progress.

auto-review requested changes

2026-05-03 08:33:23 -07:00

Dismissed

auto-review left a comment

This PR introduces a new CI-triggered review dispatch endpoint in the ar-gateway crate, allowing Forgejo Actions to trigger semantic reviews after CI checks pass. The implementation includes security measures such as token validation and PR state verification. Ensure that sensitive tokens are securely managed and not exposed in logs.

Walkthrough

New CI-triggered Review Endpoint
- Added a new POST /reviews/ci endpoint in ar-gateway to allow Forgejo Actions to trigger reviews.
- The endpoint requires a bearer token (AR_CI_REVIEW_TOKEN) and verifies the current PR head SHA before dispatching a review job.
- The implementation includes tests to ensure correct behavior, such as handling of stale head SHAs and unauthorized requests.
Security Considerations
- The AR_CI_REVIEW_TOKEN is a critical security token and should be handled with care to prevent exposure.
- The implementation uses constant-time comparison to prevent timing attacks on token validation.
Documentation Updates
- Updated various documentation files to reflect the new endpoint and its configuration, including threat model and operations guidelines.

Pre-merge checks

CHANGELOG updated — CHANGELOG.md is in the diff
Tests touched — source changed but no test file appears in the diff
No new TODO/FIXME comments — no new TODO/FIXME markers

Linters

gitleaks — ok, 8 findings
semgrep — ok, 0 findings
trivy — ok, 0 findings
osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
ast-grep — ok, 0 findings
typos — ok, 25 findings

This PR introduces a new CI-triggered review dispatch endpoint in the `ar-gateway` crate, allowing Forgejo Actions to trigger semantic reviews after CI checks pass. The implementation includes security measures such as token validation and PR state verification. Ensure that sensitive tokens are securely managed and not exposed in logs. ## Walkthrough - **New CI-triggered Review Endpoint** - Added a new `POST /reviews/ci` endpoint in `ar-gateway` to allow Forgejo Actions to trigger reviews. - The endpoint requires a bearer token (`AR_CI_REVIEW_TOKEN`) and verifies the current PR head SHA before dispatching a review job. - The implementation includes tests to ensure correct behavior, such as handling of stale head SHAs and unauthorized requests. - **Security Considerations** - The `AR_CI_REVIEW_TOKEN` is a critical security token and should be handled with care to prevent exposure. - The implementation uses constant-time comparison to prevent timing attacks on token validation. - **Documentation Updates** - Updated various documentation files to reflect the new endpoint and its configuration, including threat model and operations guidelines. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [ ] Tests touched — source changed but no test file appears in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>

crates/ar-gateway/src/main.rs Outdated

					
				@ -682,3 +706,49 @@ fn autodetect_oci_runtime() -> String {

				    }

🔴 Error: The validate_ci_review_token function should ensure that the token is not logged or exposed in error messages to prevent accidental leakage.

🔴 **Error:** The `validate_ci_review_token` function should ensure that the token is not logged or exposed in error messages to prevent accidental leakage.

Addressed in 217f55f by adding external regression coverage in crates/ar-gateway/tests/ci_review_secrets.rs for CI review secret hygiene. The new test asserts unauthorized /reviews/ci responses and captured logs do not contain either the configured action token or the rejected bearer token, and still stop before Forgejo fetch/dispatch.

Reflection/classification: one-off coverage/presentation gap. The implementation and existing unit test already avoided echoing the token, but the no-response/no-log property needed visible integration-level coverage for the review thread; no durable guardrail change is warranted.

Addressed in 217f55f by adding external regression coverage in `crates/ar-gateway/tests/ci_review_secrets.rs` for CI review secret hygiene. The new test asserts unauthorized `/reviews/ci` responses and captured logs do not contain either the configured action token or the rejected bearer token, and still stop before Forgejo fetch/dispatch. Reflection/classification: one-off coverage/presentation gap. The implementation and existing unit test already avoided echoing the token, but the no-response/no-log property needed visible integration-level coverage for the review thread; no durable guardrail change is warranted.

jwilger marked this conversation as resolved

crates/ar-gateway/src/main.rs

					
				@ -685,0 +725,4 @@

				    #[test]

				    fn ci_review_token_accepts_strong_random_value() {

				        let token = "0123456789abcdef0123456789abcdef".to_string();

🔴 Error: Ensure that any sensitive information, such as API keys or tokens, is not logged or exposed in error messages to prevent accidental leakage.

🔴 **Error:** Ensure that any sensitive information, such as API keys or tokens, is not logged or exposed in error messages to prevent accidental leakage.

Addressed in 217f55f with a real integration test file covering secret hygiene around the CI review endpoint. The token-like values in tests are synthetic fixtures, and the assertions now prove they are not returned or logged during authorization failures.

Reflection/classification: one-off coverage gap rather than a guardrail gap. The missing piece was explicit external regression coverage, not a missing always-loaded rule.

Addressed in 217f55f with a real integration test file covering secret hygiene around the CI review endpoint. The token-like values in tests are synthetic fixtures, and the assertions now prove they are not returned or logged during authorization failures. Reflection/classification: one-off coverage gap rather than a guardrail gap. The missing piece was explicit external regression coverage, not a missing always-loaded rule.

jwilger marked this conversation as resolved

auto-review requested changes

2026-05-03 08:33:28 -07:00

Dismissed

auto-review left a comment

This PR introduces a new CI-triggered review endpoint in the ar-gateway crate, allowing Forgejo Actions to request semantic reviews after CI checks pass. The implementation appears robust, with appropriate validation and security measures, but ensure that error handling and logging do not expose sensitive information.

Walkthrough

New CI-triggered Review Endpoint:
- Added a new POST /reviews/ci endpoint in ar-gateway.
- Requires a bearer token (AR_CI_REVIEW_TOKEN) for authentication.
- Validates the PR's current head SHA before dispatching a review job.
- Includes tests to verify correct behavior and edge cases.
Security Considerations:
- Uses constant-time comparison for token validation to prevent timing attacks.
- Ensures that error messages do not leak sensitive information.
- Requires a minimum token length of 32 characters for security.
Documentation Updates:
- Updated various documentation files to reflect the new endpoint and its configuration.
- Provided examples for setting up CI workflows to use the new endpoint.

Pre-merge checks

CHANGELOG updated — CHANGELOG.md is in the diff
Tests touched — source changed but no test file appears in the diff
No new TODO/FIXME comments — no new TODO/FIXME markers

Linters

gitleaks — ok, 8 findings
semgrep — ok, 0 findings
trivy — ok, 0 findings
osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
ast-grep — ok, 0 findings
typos — ok, 25 findings

This PR introduces a new CI-triggered review endpoint in the `ar-gateway` crate, allowing Forgejo Actions to request semantic reviews after CI checks pass. The implementation appears robust, with appropriate validation and security measures, but ensure that error handling and logging do not expose sensitive information. ## Walkthrough - **New CI-triggered Review Endpoint**: - Added a new `POST /reviews/ci` endpoint in `ar-gateway`. - Requires a bearer token (`AR_CI_REVIEW_TOKEN`) for authentication. - Validates the PR's current head SHA before dispatching a review job. - Includes tests to verify correct behavior and edge cases. - **Security Considerations**: - Uses constant-time comparison for token validation to prevent timing attacks. - Ensures that error messages do not leak sensitive information. - Requires a minimum token length of 32 characters for security. - **Documentation Updates**: - Updated various documentation files to reflect the new endpoint and its configuration. - Provided examples for setting up CI workflows to use the new endpoint. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [ ] Tests touched — source changed but no test file appears in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>

crates/ar-gateway/src/main.rs Outdated

🟡 Warning: Lines 682–706: Ensure that the error message for a short AR_CI_REVIEW_TOKEN does not expose any part of the token value, as this could be a security risk.

🟡 **Warning:** **Lines 682–706:** Ensure that the error message for a short `AR_CI_REVIEW_TOKEN` does not expose any part of the token value, as this could be a security risk.

Covered by the existing ci_review_token_rejects_short_non_empty_value unit test and reinforced in 217f55f with external CI review secret-hygiene tests. The short AR_CI_REVIEW_TOKEN validation error names the env var and strength requirement without echoing the rejected token value.

Reflection/classification: one-off coverage/presentation gap. The behavior was correct, but the security property was not obvious enough to the reviewer/tooling until pinned with explicit regression assertions.

Covered by the existing `ci_review_token_rejects_short_non_empty_value` unit test and reinforced in 217f55f with external CI review secret-hygiene tests. The short `AR_CI_REVIEW_TOKEN` validation error names the env var and strength requirement without echoing the rejected token value. Reflection/classification: one-off coverage/presentation gap. The behavior was correct, but the security property was not obvious enough to the reviewer/tooling until pinned with explicit regression assertions.

jwilger marked this conversation as resolved

crates/ar-gateway/src/webhook.rs Outdated

🟡 Warning: Lines 492–573: Ensure that the handle_ci_review function's error responses do not leak sensitive information, especially in the case of authorization failures or upstream errors.

🟡 **Warning:** **Lines 492–573:** Ensure that the `handle_ci_review` function's error responses do not leak sensitive information, especially in the case of authorization failures or upstream errors.

Addressed in 217f55f by adding crates/ar-gateway/tests/ci_review_secrets.rs. It captures the gateway logs while exercising unauthorized and upstream-failure /reviews/ci paths, then asserts neither action tokens, Forgejo API tokens, nor upstream sensitive bodies appear in responses or logs, and that no dispatch occurs.

Reflection/classification: one-off coverage gap. The handler already returned generic errors, but log hygiene for these paths needed explicit integration-level regression coverage.

Addressed in 217f55f by adding `crates/ar-gateway/tests/ci_review_secrets.rs`. It captures the gateway logs while exercising unauthorized and upstream-failure `/reviews/ci` paths, then asserts neither action tokens, Forgejo API tokens, nor upstream sensitive bodies appear in responses or logs, and that no dispatch occurs. Reflection/classification: one-off coverage gap. The handler already returned generic errors, but log hygiene for these paths needed explicit integration-level regression coverage.

jwilger marked this conversation as resolved

jwilger referenced this pull request

2026-05-03 08:35:52 -07:00

Deduplicate chat mentions handled by webhook and poller #50

jwilger added 1 commit

2026-05-03 08:54:21 -07:00

test(gateway): pin CI review secret hygiene

auto_review auto_review: 1 warning

CI / Nix flake check (pull_request) Has been cancelled

Details

217f55f4b1

auto-review reviewed

2026-05-03 08:55:02 -07:00

auto-review left a comment

This PR introduces a new CI-triggered review dispatch endpoint in the gateway, allowing Forgejo Actions to trigger semantic reviews after CI checks pass. The implementation appears robust with proper validation and error handling, but ensure no sensitive information is exposed in the codebase.

Walkthrough

New CI-triggered Review Endpoint:
- Added a new POST /reviews/ci endpoint to allow Forgejo Actions to trigger reviews.
- Validates the action token and ensures the PR head SHA is current before dispatching.
- Includes tests to verify the endpoint's behavior under various conditions.
Security Considerations:
- The AR_CI_REVIEW_TOKEN must be a strong token, and the code includes checks to ensure it is not too short.
- Static analysis flagged a potential generic API key exposure; ensure no sensitive data is committed.
Documentation Updates:
- Updated documentation to reflect the new endpoint and its configuration.
- Added details on the threat model and observability changes related to the new feature.

Pre-merge checks

CHANGELOG updated — CHANGELOG.md is in the diff
Tests touched — test changes are in the diff
No new TODO/FIXME comments — no new TODO/FIXME markers

Linters

gitleaks — ok, 8 findings
semgrep — ok, 0 findings
trivy — ok, 0 findings
osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
ast-grep — ok, 0 findings
typos — ok, 25 findings

This PR introduces a new CI-triggered review dispatch endpoint in the gateway, allowing Forgejo Actions to trigger semantic reviews after CI checks pass. The implementation appears robust with proper validation and error handling, but ensure no sensitive information is exposed in the codebase. ## Walkthrough - **New CI-triggered Review Endpoint**: - Added a new `POST /reviews/ci` endpoint to allow Forgejo Actions to trigger reviews. - Validates the action token and ensures the PR head SHA is current before dispatching. - Includes tests to verify the endpoint's behavior under various conditions. - **Security Considerations**: - The `AR_CI_REVIEW_TOKEN` must be a strong token, and the code includes checks to ensure it is not too short. - Static analysis flagged a potential generic API key exposure; ensure no sensitive data is committed. - **Documentation Updates**: - Updated documentation to reflect the new endpoint and its configuration. - Added details on the threat model and observability changes related to the new feature. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>

crates/ar-gateway/src/main.rs Outdated

🟡 Warning: The AR_CI_REVIEW_TOKEN should be a strong token of at least 32 characters to ensure security. Consider adding a validation check for this in the documentation or setup scripts.

🟡 **Warning:** The `AR_CI_REVIEW_TOKEN` should be a strong token of at least 32 characters to ensure security. Consider adding a validation check for this in the documentation or setup scripts.

Addressed in 869e367 by clarifying the operator-facing docs: QUICKSTART.md and crates/ar-gateway/README.md now state that AR_CI_REVIEW_TOKEN must be generated independently from WEBHOOK_SECRET with at least 32 random bytes/chars. The startup validation for short values was already present and covered by ci_review_token_rejects_short_non_empty_value.

Reflection/classification: one-off documentation drift. The runtime check existed, but two docs surfaces did not state the exact strength requirement; no new guardrail is warranted because the existing operator docs/examples already include the setup pattern.

Addressed in 869e367 by clarifying the operator-facing docs: `QUICKSTART.md` and `crates/ar-gateway/README.md` now state that `AR_CI_REVIEW_TOKEN` must be generated independently from `WEBHOOK_SECRET` with at least 32 random bytes/chars. The startup validation for short values was already present and covered by `ci_review_token_rejects_short_non_empty_value`. Reflection/classification: one-off documentation drift. The runtime check existed, but two docs surfaces did not state the exact strength requirement; no new guardrail is warranted because the existing operator docs/examples already include the setup pattern.

jwilger marked this conversation as resolved

jwilger added 1 commit

2026-05-03 08:56:14 -07:00

docs(gateway): clarify CI review token strength

auto_review auto_review: no findings

CI / Nix flake check (pull_request) Successful in 1m58s

Details

869e3677e9

auto-review reviewed

2026-05-03 08:56:54 -07:00

auto-review left a comment

This PR introduces a new CI-triggered review dispatch endpoint in the ar-gateway crate, allowing Forgejo Actions to request semantic reviews after CI checks pass. The implementation appears robust, with proper validation and security measures, and includes comprehensive documentation and tests. However, ensure no sensitive information is exposed in logs or code.

Walkthrough

CHANGELOG.md: Updated to include details about the new CI-triggered review feature.
QUICKSTART.md: Added documentation for the new AR_CI_REVIEW_TOKEN environment variable.
crates/ar-gateway/src/lib.rs: Introduced CiReviewEndpointDeps and updated AppState to include the new endpoint dependencies.
crates/ar-gateway/src/main.rs: Added logic to enable the CI review endpoint based on the presence of AR_CI_REVIEW_TOKEN.
crates/ar-gateway/src/webhook.rs: Implemented the handle_ci_review function to process CI-triggered review requests, including validation and dispatch logic.
crates/ar-gateway/tests/ci_review_secrets.rs: Added tests to ensure the CI review endpoint handles authorization and error cases correctly.
Documentation: Updated various documentation files to reflect the new feature and its configuration.

Pre-merge checks

CHANGELOG updated — CHANGELOG.md is in the diff
Tests touched — test changes are in the diff
No new TODO/FIXME comments — no new TODO/FIXME markers

Linters

gitleaks — ok, 8 findings
semgrep — ok, 0 findings
trivy — ok, 0 findings
osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
ast-grep — ok, 0 findings
typos — ok, 25 findings

This PR introduces a new CI-triggered review dispatch endpoint in the `ar-gateway` crate, allowing Forgejo Actions to request semantic reviews after CI checks pass. The implementation appears robust, with proper validation and security measures, and includes comprehensive documentation and tests. However, ensure no sensitive information is exposed in logs or code. ## Walkthrough - **CHANGELOG.md**: Updated to include details about the new CI-triggered review feature. - **QUICKSTART.md**: Added documentation for the new `AR_CI_REVIEW_TOKEN` environment variable. - **crates/ar-gateway/src/lib.rs**: Introduced `CiReviewEndpointDeps` and updated `AppState` to include the new endpoint dependencies. - **crates/ar-gateway/src/main.rs**: Added logic to enable the CI review endpoint based on the presence of `AR_CI_REVIEW_TOKEN`. - **crates/ar-gateway/src/webhook.rs**: Implemented the `handle_ci_review` function to process CI-triggered review requests, including validation and dispatch logic. - **crates/ar-gateway/tests/ci_review_secrets.rs**: Added tests to ensure the CI review endpoint handles authorization and error cases correctly. - **Documentation**: Updated various documentation files to reflect the new feature and its configuration. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>

jwilger referenced this pull request

2026-05-03 09:07:20 -07:00

fix(review): clear stale AR change requests after clean re-review #52

jwilger dismissed auto-review's review

2026-05-03 09:07:50 -07:00

Reason:

AR not yet submitting APPROVE status even though no new findings. Ticket #52 will fix this.

jwilger merged commit 83d3dc67cd into main

2026-05-03 09:07:57 -07:00

jwilger deleted branch feat/ci-triggered-reviews

2026-05-03 09:07:57 -07:00

jwilger referenced this pull request from a commit