Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to check symbol namespaces in :allow and :deny #934

Closed
manicolosi opened this issue Oct 3, 2024 · 3 comments
Closed

Ability to check symbol namespaces in :allow and :deny #934

manicolosi opened this issue Oct 3, 2024 · 3 comments

Comments

@manicolosi
Copy link

Is your feature request related to a problem? Please describe.

I'm trying to create a sandbox environment where only specific vars are exposed in the user namespace. I want to exclude clojure.core vars through the use of :allow. However I have some macros that need access to clojure.core. My approach is to alias some clojure.core vars in an impl namespace. For example, I don't want to expose mapv but I need in for my macro implementations.

Describe the solution you'd like

The ability to namespace qualify symbols in :allow and :deny:

(def my-each ^:sci/macro
  (fn [_&form _&env coll body]
    `(impl/mapv (impl/fn [item#]
                  (impl/let [~'%item item#]
                    ~body))
                ~coll)))

(sci/eval-string (pr-str '(each %foo (+ %item %item)))
                 {:namespaces {'user {'each my-each}
                               'impl {'mapv mapv
                                     'fn (with-meta @#'fn {:sci/macro true})
                                     'let (with-meta @#'let {:sci/macro true})}}
                  :bindings {'%foo [1 2 3]}
                  :allow ['each 'impl/mapv 'impl/fn 'impl/let]})

This results in an exception: impl/mapv is not allowed!. However, this example works if I leave out :allow.

Describe alternatives you've considered

Ideally, macros could use clojure.core without exposing it to the programs that get eval'd.

Additional context

None

@borkdude
Copy link
Collaborator

borkdude commented Oct 3, 2024

Hi! I think this PR fixes it more properly: #936
I don't think there should have ever been a reason why a non-namespaced symbol would be not allowed, probably just an oversight. If you can test this PR locally and it works, then I'll merge it.

@manicolosi
Copy link
Author

Hi! I think this PR fixes it more properly: #936 I don't think there should have ever been a reason why a non-namespaced symbol would be not allowed, probably just an oversight. If you can test this PR locally and it works, then I'll merge it.

PR #936 works fine. Thanks!

@borkdude
Copy link
Collaborator

borkdude commented Oct 4, 2024

Merged

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

No branches or pull requests

2 participants