fix(apps): check for auth when executing as publisher

[uael]

Important

Adds authorization check for Publisher execution mode in execute_component and updates ExecutionMode to derive Copy.

• Authorization Check:
  • In execute_component, added check for ExecutionMode::Publisher to ensure user is authenticated and authorized.
  • Returns Error::NotAuthorized if unauthorized.
• Enum Update:
  • ExecutionMode now derives Copy trait.
• Build Configuration:
  • Updates to flake.nix and flake.lock for dependencies and build configurations.

^{This description was created by for 9515514. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to d60ca29 in 11 seconds

More details

• Looked at 45 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. backend/windmill-api/src/apps.rs:193

• Draft comment:
  Deriving Copy for ExecutionMode is safe here, but be cautious if adding non-unit variants in the future.
• Reason this comment was not posted:
  Confidence changes required: 33%
  The Copy trait is not suitable for enums with non-unit variants. The ExecutionMode enum has no non-unit variants, so it is safe to derive Copy. However, if in the future, non-unit variants are added, this could lead to issues.

Workflow ID: wflow_zuRN0dmbnxcaHLZJ

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	ada4e11
Status:	✅  Deploy successful!
Preview URL:	https://55ca328e.windmill.pages.dev
Branch Preview URL:	https://uael-apps-exec-auth.windmill.pages.dev

View logs

[ellipsis-dev]

👍 Looks good to me! Incremental review on b3073c9 in 54 seconds

More details

• Looked at 75 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. backend/windmill-api/src/apps.rs:1377

• Draft comment:
  The authorization check for ExecutionMode::Publisher should handle cases where opt_authed is None to prevent unauthorized access.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment suggests adding a check that already exists in the code. The get_on_behalf_details_from_policy_and_authed function will return an error if opt_authed is None in Publisher mode (line 1214-1221). So this comment is suggesting something that is already handled correctly.
  Could I be missing some edge case where the authorization check could be bypassed? Is there a race condition possible between the checks?
  No, the checks are sequential and the get_on_behalf_details_from_policy_and_authed function will always be called before any execution can proceed. There is no way to bypass this check.
  The comment should be deleted because it suggests adding an authorization check that already exists in the code through the get_on_behalf_details_from_policy_and_authed function.

Workflow ID: wflow_aI2n8JXvkbhf74Py

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on e4715fc in 58 seconds

More details

• Looked at 105 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. backend/windmill-api/src/apps.rs:1381

• Draft comment:
  Consider using once_cell::sync::Lazy or tokio::sync::OnceCell for PERMIT_CACHE to avoid potential blocking in async contexts.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment suggests alternatives to lazy_static but doesn't explain why they would be better. lazy_static is a widely used and stable library. The comment speculates about "potential blocking" but doesn't demonstrate any actual issues. The cache implementation itself looks reasonable and is using async functions properly.
  The comment could be right that once_cell or OnceCell might be marginally better choices, but without evidence of actual problems, this seems like premature optimization.
  The current implementation works correctly and there's no evidence of blocking issues. Making this change would be a refactor without clear benefits.
  The comment should be deleted as it suggests a speculative change without demonstrating clear benefits or actual problems with the current approach.

2. backend/windmill-api/src/apps.rs:1395

• Draft comment:
  Consider using let permit = PERMIT_CACHE.get_or_insert_with(&permit_key, || async { ... }).await; for better readability and efficiency.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment is incorrect because: 1. get_or_insert_with is for synchronous operations while this code requires async database queries 2. get_or_insert_async is the correct choice here since we need to await the database permission check 3. The suggestion would actually break the code since you can't use a sync function for async operations
  Could there be some performance benefit to using a sync function that I'm missing? Maybe the commenter knows something about the cache implementation that I don't?
  No - in this case we definitively need async since we're doing database queries. There's no way to make this synchronous without blocking, which would be worse for performance.
  The comment should be deleted because it suggests using a synchronous cache method when async is required for the database queries. The current implementation using get_or_insert_async is correct.

3. backend/windmill-api/src/apps.rs:1390

• Draft comment:
  Consider simplifying the permit_key calculation by directly using Sha256::digest on the concatenated bytes.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The current implementation is actually more memory efficient since it avoids concatenating the byte slices into a single allocation. Using fold() and chain_update() allows streaming the bytes directly into the hasher. While Sha256::digest would be slightly simpler code, it would require allocating and concatenating the bytes first.
  The comment may have a point about code readability - the fold() pattern is less immediately obvious than a simple digest() call. The performance impact of the extra allocation would likely be negligible in practice.
  However, the current implementation follows best practices for hashing by avoiding unnecessary allocations. The slight readability tradeoff is worth it for better memory usage.
  The current implementation is actually preferable as it is more memory efficient, even if slightly more complex. The suggested change would not be an improvement.

Workflow ID: wflow_N9pxmOQelFB2c7KC

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 9515514 in 3 minutes and 5 seconds

More details

• Looked at 106 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. backend/windmill-api/src/apps.rs:1381

• Draft comment:
  Consider using once_cell::sync::Lazy instead of lazy_static! for PERMIT_CACHE to improve code readability and maintainability.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The use of lazy_static! for PERMIT_CACHE is not ideal. In modern Rust, once_cell::sync::Lazy is preferred for thread-safe, lazy-initialized static variables. This change would improve code readability and maintainability.

2. backend/windmill-api/src/apps.rs:1390

• Draft comment:
  Consider using Sha256::digest for calculating permit_key to improve readability.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The permit_key calculation uses a fold with Sha256::new(), which is correct but could be simplified using Sha256::digest for better readability.

3. backend/windmill-api/src/apps.rs:1395

• Draft comment:
  Ensure proper error handling for database operations in permit_fut to improve robustness.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The permit_fut logic checks for authorization but does not handle potential database errors explicitly. Adding error handling would improve robustness.

Workflow ID: wflow_Z6nAxcFHX5GAdy0R

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.