[ruby] Add Support for ERB files

[AndreiDreyer]

• Added support for ERB files
• Added templateOutRaw (<%== %>) and templateOutEscape (<%= %>) operator calls
• Added RETURN for the joern__buffer which holds all of the appended strings from the ERB lowering
• Added .html.erb files as config files

[DavidBakerEffendi]

Nice

[maltek]

I'm still missing a lot of .erb files. E.g. for https://github.com/chatwoot/chatwoot:

ocular> cpg.file.name(".*.html.erb").size
val res18: Int = 40

ocular> cpg.configFile.name(".*.html.erb").size
val res19: Int = 88

The frontend spits out some warnings for the project, e.g. Yield expression outside of method scope: yield(:head) and Type 'String' is considered a 'core' type, not a 'Kernel-contained' type - but not enough to explain 48 missing files.

[AndreiDreyer]

@maltek these are the new results on chatwoot with all of the changes made for ERB handling.

joern> cpg.configFile.name(".*.html.erb").l.map(_.name).l.diff(cpg.file.name(".*.html.erb").map(_.name).l)
val res1: List[String] = List()

joern> cpg.file.name(".*.html.erb").l.size
val res2: Int = 88

joern> cpg.configFile.name(".*.html.erb").l.size
val res3: Int = 88

[ml86]

Why are you writing tests that include the lowering of a ruby AST into CPG?
The ERB support takes input ERB files and emits ruby code that is then fed into rubyAstGen. For the later part we already have plenty of tests. Here it would be much better readable if you test the first step ( ERB -> ruby code) the added benefit would be much better readability.

[DavidBakerEffendi]

ERB -> Ruby code is tested on the ruby_ast_gen project e.g., https://github.com/joernio/ruby_ast_gen/blob/main/spec/ruby_ast_gen_spec.rb#L155

Albeit, one could argue for more tests there perhaps, but we haven't designed the RubyAstGen hook to give us the lowered ERB code as-is, only via AST

[maltek]

all the ruby code in
https://github.com/forem/forem/blob/0772f2d49b18d94f3b982b39420ea31235c1c8aa/app/views/layouts/application.html.erb#L88-L92 exists in the CPG only as Literal nodes, instead of the actual calls and control structures. Also the lines are off-by-one (code from line 90 has line 91 in the CPG).

[maltek]

for the raw call here I'm getting the wrong line number (8 instead of 12): https://github.com/OWASP/railsgoat/blob/c1e8ff1e3b24a1c48fcfc9fbee0f65dc296b49d9/app/views/layouts/application.html.erb#L12

and the render call here has line 35 instead of 56: https://github.com/forem/forem/blob/0772f2d49b18d94f3b982b39420ea31235c1c8aa/app/views/admin/badge_achievements/index.html.erb#L56

[maltek]

the rails_lambda_0 calls are also problematic:

• the joern__inner_buffer variable is missing a closure capture binding from the outer scope I missed that the lambda call is inside of a joern__buffer << rails_lambda_0.call() call
• the self parameter is not captured or passed as argument from the outer scope. e.g. at https://github.com/forem/forem/blob/4fee5c0bd36049a35b7966a0fad0f9edfa1aeaa5/app/views/users/_sidebar.html.erb#L56 @user is accessed from the outer scope, but there's no way to find a dataflow from the <module> methods's self parameter to the self.@user within that lambda

[maltek]

current problem:

class UsersController < ActionController::Base
  def show
    respond_to do |format|
      format.json { render partial: "foo" }
    end
  end
end

some self identifiers refer to multiple locals:

cpg.identifier.name("self").filter(_.refsTo.size != 1).l

Identifiers should only ever have REF edges to the single variable of that name in the exact same method.

[maltek]

@TNSelahle the problem with the self variable is fixed with your last change. But there is still a related problem: the format identifiers are not referencing the format MethodParameterIn node.

[maltek]

I don't think we should handle this special case in the general astForFieldAccess method. Can we make the type an optional parameter that's handed down here from the surrounding context if it's known?

[maltek]

It's sort of the same special case now, just one layer up. I thought we could do it from around here where we've already determined to be dealing with the ERB lowering. But I just read over the diff in the browser, so I could very well be mistaken.

joern/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForExpressionsCreator.scala

Lines 200 to 201 in bfd14fc

	val ast = astForFieldAccess(MemberAccess(n.target, ".", n.methodName)(n.span), stripLeadingAt = true)
	(n.op, hasStringArgs, ast)

[TNSelahle]

@maltek thanks for the review! I've made and pushed up the changes. Integration tests on CS master are all green.

[maltek]

It's sort of the same special case now, just one layer up. I thought we could do it from around here where we've already determined to be dealing with the ERB lowering. But I just read over the diff in the browser, so I could very well be mistaken.

joern/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForExpressionsCreator.scala

Lines 200 to 201 in bfd14fc

	val ast = astForFieldAccess(MemberAccess(n.target, ".", n.methodName)(n.span), stripLeadingAt = true)
	(n.op, hasStringArgs, ast)

[TNSelahle]

#5447 (comment)

@maltek the snippet you highlighted applies to astForFieldAccess for the self.joernBufferAppend call. The special case for you identified in the astForFieldAccess method was for field access for self.joernBuffer and self.joernInnerBuffer.

I'll rename isErbCall to isErbBufferApppendCall to make it clearer.