codewars / codewars-runner-cli

Old CodeRunner project. See https://github.com/codewars/runner instead.
GNU Affero General Public License v3.0
400 stars 141 forks source link

Turn off term rewriting macros for Nim #835

Closed metagn closed 4 years ago

metagn commented 4 years ago

Term rewriting macros make it easy to cheat Nim tests. There are no katas that require their use and they're an experimental feature in the first place.

kazk commented 4 years ago

Thanks for trying to contribute. Unfortunately, this repository only exists for the issue tracker and the code is no longer used.

image


But I can apply this change to the current runner. Can you elaborate on the effect of this change? Any example of a cheat this prevents?

g964 commented 4 years ago

@hlaaftana posted an issue at https://www.codewars.com/kata/5b1cd19fcd206af728000056/discuss#5f59fdd701e9c60029deae07 Maybe it can help. Edit: I have to change the place of my custom testing function too.

metagn commented 4 years ago

It's not for every test, but it's very easy to write a test that's susceptible to tampering with. g964's katas that check floats have a check that basically does abs(actual - expected) <= epsilon, but there's nowhere else in the test where <= should return false, so you can override all <= calls to make all the tests pass like so:

template replaceLe*{`<=`(a, b)}(a, b: float64): bool = true

Something I just found out is that there is a much more effective solution at breaking katas. You can define your own check overload and the tests can call it, because the check macro in Nim's unittest has an underspecified type for the first argument (untyped). You can change it to bool like so and it will work:

template check*(a: bool) = discard

The solution is either every Nim test calling unittest.check every time, or simply changing import solution in the test preload to import solution except test, suite, check. The change would be in this line:

https://github.com/codewars/codewars-runner-cli/blob/d1c1d8bfb9919ee61d520370484894be27f48739/lib/runners/nim.js#L24

kazk commented 4 years ago

Thanks. I tested the changes against all Nim kata and it didn't break any of the reference solutions. I'll let you know when it's deployed.

kazk commented 3 years ago

Forgot to update, but this was deployed. Thanks for contributing!