Closed
Conversation
Core Changes: - **Terminal node enforcement**: Prevent child rulesets from being added under terminal nodes for proper security boundaries - **Symlink security**: Reject symlinked ACL files in Exists() and LoadFromFile() to prevent security vulnerabilities - **Rule removal fix**: RemoveRuleSet now only clears rules from target node while preserving child nodes, preventing accidental data loss - **Rule clearing**: SetRules properly clears existing rules when passed nil/empty ruleset Technical Details: - tree.go: Add ErrTerminalNodeExists, validate terminal nodes in AddRuleSet, rewrite RemoveRuleSet to preserve children - node.go: Fix SetRules to clear rules when nil/empty input provided - aclspec.go: Replace os.Stat with os.Lstat and add symlink rejection in Exists() - ruleset.go: Add symlink validation in LoadFromFile() with descriptive error messages Tests added to internal/server/acl and internal/aclspec packages to increase coverage and validate new security behaviors.
Reverts changes to terminal node behavior to match the original design: **Core Reversion:** - **Terminal nodes allow children**: Child rulesets can be added under terminal nodes for performance (avoids tree rebuilds) - **Terminal controls lookup/inheritance**: Terminal flag stops traversal during GetNode/GetNearestNodeWithRules, not during AddRuleSet - **Original RemoveRuleSet**: Restores original behavior that removes entire subtrees **Technical Changes:** - tree.go: Remove ErrTerminalNodeExists, restore original AddRuleSet logic, restore original RemoveRuleSet behavior - tree_test.go: Update tests to verify children can be added but lookups stop at terminal boundaries **Design Rationale:** Tree stores all ACL files for performance optimization while terminal flag controls rule inheritance during lookups. Child ACL files exist in tree structure but are ignored when under terminal nodes.
yashgorana
changed the title
Contributor
|
holding off on this pr - it has some major conflicts with #12 - which also changes the behavior of acl caching. |
yashgorana
added a commit
that referenced
this pull request
Jun 18, 2025
yashgorana
added a commit
that referenced
this pull request
Jun 18, 2025
- Docker server/client with MinIO integration - Auth bypass via SYFTBOX_AUTH_ENABLED=0 for local development - Multi-client support with per-email config persistence - Just commands for easy Docker orchestration - Smart entrypoint handling local vs production servers
* fix(server/explorer): serve empty dirs * refactor(server/acl): rename acl->ACL + update perms + simplified caching + fixes * feat(server/acl): add acl endpoints * feat(server): enforce perms + standardized api error * feat(client): read ignore list from file
madhavajay
changed the title
* feat: go to prod * chore: retire syftbox running openmined.org domain * fix: update error message
* feat(server): add HTTP message handling and send endpoint - Implement HTTP message processing with /send/msg endpoint and HttpMsg types * feat(send): add polling endpoint for HTTP responses - Add /send/poll endpoint with configurable timeout and structured responses using PollObjectRequest and SendAcknowledgment * refactor(sync): clean up and rename HTTP message functions - Remove unused code and rename handleHttp to processHttpMessage with .http.request/.http.response extensions * refactor(cors): centralize CORS middleware * fix(server): resolve CORS and send timeout issues * refactor: improve RPC message handling - Remove base64 decoding, add FromSyftURL parser and enhanced JSON marshaling * feat: add automatic request/response cleanup * feat: standardize API responses with 202 status for timeouts * feat: comprehensive URL handling system (#14) - Implement SyftBoxURL with parsing, validation, header management, and flexible routing * feat(routes): embed HTML templates * fix: normalize file path separators * refactor: remove user agent check from polling API * refactor(send): add interfaces and comprehensive tests (#27) * test(middlewares): add guest access to JWTAuth for RPC routes (#28) --------- Co-authored-by: Yash Gorana <yash.gorana@hotmail.com>
* fix(server/handlers/send): replace json.Marshal with custom MarshalJSON for message storage * sort query params in syfturl to main consistent ordering
* cd pipeline to deploy syftbox to dev, stage, prod * add just commands to bump and release package * fix bug in show versions in justfile * integrate version release in deployment action * auto release version on deployment to prod * update deploy workflow to set remote URL with token and push tags * refactor deployment workflows to separate production and non-production processes
* fix: go releaser installation in workflow * use cask arg in homebrew * pass remote username without remote= var
Collaborator
Author
|
closing in favour of this #37 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Core Changes:
Technical Details:
Tests added to internal/server/acl and internal/aclspec packages to increase coverage and validate new security behaviors.
Description
Please include a summary of the change, the motivation, and any additional context that will help others understand your PR. If it closes one or more open issues, please tag them as described here.
Affected Dependencies
List any dependencies that are required for this change.
How has this been tested?
Checklist