Project

General

Profile

Actions

Feature #21871

open

Add Module#undef_const

Feature #21871: Add Module#undef_const

Added by jeremyevans0 (Jeremy Evans) 14 days ago. Updated 11 days ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:124726]

Description

I propose to add Module#undef_const, which would operate for constants similarly to how undef_method (and the undef keyword) works for methods. When an undefed constant is found during constant lookup, lookup stops, and results in const_missing being called (similar to how undef_method stops method lookup and results in method_missing being called).

The expected use for this is for stopping direct access to global constants inside namespaces. For example:

class DangerousClass; end

class SafeClass
  undef_const :DangerousClass

  DangerousClass   # NameError (const_missing behavior)
end

DangerousClass # no error

You cannot implement this behavior without introducing the equivalent of Module#undef_const. The best you can do is define a constant in the namespace that raises later when there is some access to the returned object. However, that approach has significant limitations, since you cannot raise on the lookup, only later when something operates on the returned object:

class DangerousClass; end

class SafeClass
  DangerousClass = BasicObject.new

  DangerousClass.foo       # NoMethodError
  DangerousClass::Constant # TypeError

  def m
    # doesn't raise, NoMethodError/TypeError occurs potentially much later in unrelated code
    [DangerousClass]
  end
end

As to why it is good to stop direct access to a particular constants inside a namespace, it helps avoid IDOR (insecure direct object reference) security issues, since it pushes you to use a style like:

# Find a bar related to this foo
bar = foo.bars.find { it.id == some_id }

As opposed to:

bar = Bar.find { it.id == some_id && it.foo.id == foo.id }

Assuming these are equivalent, the first approach is still better, because it avoids the possibility that a developer can write the following, missing the necessary check that the bar being accessed is related to the foo:

bar = Bar.find { it.id == some_id }

By using undef_const :Bar in the namespace, the Bar.find approach will not work, and developers will be pushed to use the foo.bars.find approach, which enforces safer behavior.

I've submitted a pull request that implements this proposal: https://github.com/ruby/ruby/pull/16116

Updated by Eregon (Benoit Daloze) 14 days ago ยท Edited Actions #1 [ruby-core:124732]

"undefined constants" is what I would call the old autoload behavior which left autoload constants in some limbo state when the loading failed.
I'm not keen on basically re-adding that or something similar to constants, the semantic model is much simpler without them.
It also means an extra check whenever looking up a constant, which is not great.
The sentinel value could leak and that would cause chaos (including potentially non-undefined constants being partially treated as undefined if that sentinel is assigned to another constant).

Most importantly, I think it would add a lot of confusion for users for cases like:

A = 42
p A # OK
module Foo
  # ...

  p A # fails
end

We would be breaking a pretty fundamental properly of constant scopes here, which seems clearly not worth it.

I think the DangerousClass = BasicObject.new approach is good enough, I doubt many codebases would want to use a style like bar = foo.bars.find { it.id == some_id }.
Preventing access to Bar in a whole namespace seems too restrictive, the code could also be bar = foo.find_bar(some_id), and there can be legitimate accesses to Bar under the namespace.
People might also just workaround with bar = ::Bar.find { it.id == some_id } or bar = SomeHigherNamespace::Bar.find { it.id == some_id } and then undefining the constant doesn't achieve much besides confusion.

TLDR: I'm against this, there are many downsides and the use case seems pretty niche and not that convincing (doesn't actually help IDOR much).

Updated by matz (Yukihiro Matsumoto) 11 days ago Actions #2 [ruby-core:124786]

First, it doesn't seem directly related to the example and the functionality being proposed. Second, it doesn't directly explain the pros and cons of introducing undefining constants. At this point, I don't see the need for it.

Matz.

Actions

Also available in: PDF Atom