Skip to content

enforce policyserver policy if exists#1304

Open
alexcos20 wants to merge 1 commit intomainfrom
fix/policy_server_enforce
Open

enforce policyserver policy if exists#1304
alexcos20 wants to merge 1 commit intomainfrom
fix/policy_server_enforce

Conversation

@alexcos20
Copy link
Copy Markdown
Member

@alexcos20 alexcos20 commented Mar 27, 2026

Summary

  • Enforce policy-server if exists (if check fails, reject the action)
  • Fix policy-server integration to correctly interpret denial responses via response.success (instead of truthiness checks).
  • Improve Policy Server client to:
    • Support optional API key header (POLICY_SERVER_API_KEY via X-API-Key).
    • Return a meaningful error message when the request fails (e.g., network/DNS/connection issues).

@alexcos20 alexcos20 marked this pull request as ready for review March 27, 2026 14:22
@alexcos20
Copy link
Copy Markdown
Member Author

/run-security-scan

Copy link
Copy Markdown
Member Author

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This pull request introduces support for an API key for the policy server and significantly improves its error handling. It refactors the PolicyServer.askServer method to include an X-API-Key header if POLICY_SERVER_API_KEY is set in the environment. Crucially, it corrects the error reporting when the fetch call to the policy server fails, ensuring that success: false is returned along with a meaningful error message instead of incorrectly reporting success: true with an empty message.

Downstream handlers (ddoHandler, encryptHandler) have been updated to correctly interpret the PolicyServerResult object by checking response.success instead of the boolean response directly, aligning with the improved policy server response structure.

Comments:
• [INFO][other] This change correctly checks the success property of the PolicyServerResult object, aligning with the new interface and improved error handling in the PolicyServer.
• [INFO][other] Similar to ddoHandler, this updates the check to correctly use response.success from the PolicyServerResult object.
• [INFO][other] Consistent update to use response.success for the EncryptFileHandler.
• [INFO][security] Adding private apikey: string and fetching it from process.env.POLICY_SERVER_API_KEY is a good practice for securing policy server communication when an API key is required.
• [INFO][security] Conditionally adding the X-API-Key header if the apikey is present is a robust way to handle both secured and unsecured policy server configurations. Good job on ensuring Content-Type is always set.
• [ERROR][bug] This is a critical bug fix. Previously, a failed fetch operation (e.g., network error, DNS resolution failure) would incorrectly return { success: true, message: '', httpStatus: 400 }. Changing success: true to success: false and providing a detailed error message (errorText || 'Policy server request failed') significantly improves the reliability and debuggability of the policy server integration.

While httpStatus: 400 for a network-level fetch exception might be debatable (as 4xx usually implies a server response), it's a reasonable fallback for a client-side error that prevents a proper request/response cycle. The main point is success: false and a helpful message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant