Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Commits on Mar 15, 2009
  1. David Cato Eric Kidd

    Fix tainted string error when updating comment

    davec authored emk committed
    When updating a comment, a tainted string error was being thrown due to
    the lack of a h() escape on the article title in the comment partial.
    A deprecation warning from truncate() in the comment partial has also
    been resolved.
    
    A functional test (test_should_update_comment) for the admin comments
    controller is also included.
  2. David Cato Eric Kidd

    Expire cache on theme change from admin/settings

    davec authored emk committed
    Force cache expiration when changing the theme from the Admin::Settings
    controller as is done when changing the theme from the Admin::Themes
    controller so that the behavior after a change of theme is consistent,
    regardless of where the change is made.
  3. David Cato Eric Kidd

    Don't depend on database ordering for asset tests

    davec authored emk committed
    test_should_edit_asset and test_should_update_asset were depending on
    database-specific ordering of the newly added assets. Although these
    tests worked with MySQL, they failed with PostgreSQL because an
    unordered find does not necessarily return the first of the three
    uploaded assets.
    
    Instead, Asset.find was changed to Asset.find_by_filename to
    specifically return the desired asset file. Asset.find_by_filename is
    used instead of Asset.find(:order => :id) so that there is no dependence
    on the order in which the three versions of the asset file are added.
Commits on Jan 9, 2009
  1. Eric Kidd

    Changed user login to send user to admin section on succesful login i…

    Chris Cummer authored emk committed
    …nstead of the blog homepage since users have the ability to post to the blog
Commits on Dec 27, 2008
  1. Eric Kidd

    Upgrade to interim release of Webrat from github

    emk authored
    This should let us experiment with Webrat::Selenium support.
  2. Eric Kidd

    Add integration test for "reset password"

    emk authored
    Note that we actually extract the activation URL from the e-mail and
    pass it directly to 'visit'.
  3. Eric Kidd

    Write login integration tests using Webrat

    emk authored
    Why do we need integration tests? We've been suffering a lot of
    regressions in the Mephisto UI, because our functional tests don't reach
    high enough up towards the browser, and whole classes of bugs manage to
    slip through.
    
    What is Webrat? Webrat is a "browser simulator" written in Ruby.  It
    generates a DOM and allows us to fill in forms as though an actual user
    were interacting with the site.
    
    Why Webrat, and not Selenium, Watir, etc?  Webrat is recommended by the
    Cucumber project as the default way to write user stories; it's very
    fast; and it has a reasonable API.  Plus, Webrat actively maintained,
    and very recent versions of Webrat can be used as a front-end to
    Selenium.
    
    Why Rails integration tests, and not Cucumber stories?  Since the people
    contributing to Mephisto will largely be programmers, I decided to write
    integration tests using a Ruby-based DSL.  Cucumber stories look really
    interesting, but with no actual clients in the loop, the text-based format
    is slightly less useful and has a steeper learning curve for programmers.
    
    Since we're switching to a new integration testing tool, I moved a bunch of
    code out of test_helper.rb and put it into our only existing integration
    test, caching_test.rb.  I also switched blueprints.rb to set up user
    passwords using 'password' and 'password_confirm' (instead of crypted and
    salted values) to make it easier for tests to override
Commits on Dec 24, 2008
  1. Eric Kidd

    Modernize rjs: admin/articles

    emk authored
    I'm going to try to rename all the *.rjs files to *.js.rjs.  This is
    trickier than the other renamings, because our unit test coverage isn't
    perfect, and I'm trying to test everything by hand when possible.  So
    I'm going to do this one directory at a time.
    
    Other changes:
      - The live_preview and _preview stuff wasn't being used.
      - We didn't have a test case for the 'label' action.
Commits on Dec 22, 2008
  1. Eric Kidd

    Fix theme controller bugs

    emk authored
    J.C. Zhu reported an InvalidAuthenticityToken error when trying to
    change themes.  This was one of several bugs in the theme switching
    code left over from the Rails 2.2 porting process and the security
    audit.
    
    We needed to convince Ajax.Request to use a GET request when displaying
    the theme tools, and we needed to properly escape some text in the
    _tools template.
    
    I've also added a test case to make sure we actually render the _tools
    view successfully.
Commits on Dec 20, 2008
  1. risk danger olson
  2. risk danger olson
  3. Eric Kidd

    Add unit test for img filtering

    emk authored
    Test case for a644733.
  4. Eric Kidd

    Merge branch 'experimental'

    emk authored
    Conflicts:
    
    	app/drops/comment_drop.rb
  5. Eric Kidd

    Security: Fix many broken filter regexps

    emk authored
    In Ruby, "foo\nbar" =~ /^bar/ will result in a match, because ^ matches
    at the start of any line, not at the start of the string.  In general,
    we want to use \A and \z in place of ^ and $, respectively.
    
    We rely heavily on regular expressions to filter untrusted data.  And
    many of these regular expressions can be fooled easily because they rely
    on ^ and $ when they shouldn't.  See comment_drop_test for a
    user-exploitable example.
    
    This patch does a bulk search-and-replace of the offending patterns.  It
    may easily have missed something somewhere, but it's a good start.
Commits on Dec 19, 2008
  1. Eric Kidd

    Security: Fix XSS attack against new comment form

    emk authored
    WARNING: If you're one of the first people testing this commit, please
    use a backup database.
    
    How to reproduce: Create a new comment, and set all fields to
    <script>alert("Pwned")</script>.  Submit it.  You will see a JavaScript
    alert dialog, which is bad.
    
    What's happening: Untrusted fields in Comment objects are sanitized
    immediately before they're written to the database for the first time.
    But if validation fails, it leaves the application with an unsanitized
    comment object.  When the "can't submit comment" error is displayed,
    this unsanitized comment object can be passed straight throught to
    Liquid, which assumes that all HTML tags have been escaped.
    
    (This may look like "self XSS" attack only, but hostile pages can
    trigger it by tricking you into submitting a comment form back to your
    own site, preloaded with malicious data.)
    
    How we fix it: We make HTML escaping the responsibility of CommentDrop,
    not the Comment model.  This means that we need to unescape several
    existing fields in the database.
    
    Possible issues: This means that we're storing dangerous, untrusted data
    in our database, and that we need to rely on the proper use of 'h' and
    'CGI.escapeHTML'.  In the case of 'h', we're already using SafeERB, so
    insecure admin templates will be caught automatically, and dangerous
    data should never be sent to the user.  In the case of Liquid, we need
    to carefully examine our CommentDrop class to make sure that we're not
    passing any unescaped data through to the Liquid templates.  But this is
    a pretty manageable "proof obligation"--and remember that the old
    "sanitize on create" code actually suffered from XSS attacks, because it
    was too easy to do the sanitization in the wrong place.
Commits on Dec 17, 2008
  1. Eric Kidd

    Test whether database adapter supports safe_erb

    emk authored
    We need to make sure that whatever data we receive from the database is
    properly marked as tainted.
Commits on Dec 12, 2008
  1. Eric Kidd

    Security: Replace white_list with Rails 2.2 sanitizer

    emk authored
    The Rails 2.2 santizer is an enhanced version of Rick's original
    white_list plugin, so let's upgrade and get the latest fixes.
    
    Note that Mephisto had separate rules for sanitizing comments and
    non-comments in Atom feeds.  This difference was introduced in commit
    88df87e.  Unfortunately, I'm not able
    to track down any information on the problem being fixed here.  Since we
    already add half of the tags in question to the whitelist, I've decided
    to just treat all sanitized Atom feed content the same.  Please let me
    know if this breaks anything.
Commits on Dec 11, 2008
  1. Eric Kidd

    Security: Protect the account controller from forgery

    emk authored
    I tried to test all the code paths by hand.  Note that the account
    controller is now generally restricted to POST requests.
    
    This prevents hostile sites from you out of your Mephisto account, and
    from sending out "reset password" e-mails.
    
    It also would theoretically prevent "login CSRF" attacks, but this
    attack model doesn't make much sense against Mephisto:
    
      http://hublog.hubmed.org/archives/001755.html
Commits on Dec 8, 2008
  1. Eric Kidd

    Store themes in different paths depending on RAILS_ENV

    emk authored
    Until now, our themes were stored in the following places:
    
      production:  themes/site-N
      development: themes/site-N
      test:        themes/site-N or tmp/themes/site-N
    
    Notice the fun complications here: Production and development themes are
    stored in the same place, despite conflicting values of N, and test
    themes are stored in one of two different locations depending on various
    factors.  So there's lots of ways to accidentally lose data by confusing
    themes associated with different databases.
    
    This patch regularizes things a bit:
    
      production:  themes/site-N
      development: themes/development/site-N
      test:        themes/test/site-N
    
    Every non-production enviornment gets its own themes directory.  We do
    this by centralizing the logic in lib/mephisto/theme_root.rb.
    
    Note that this change will break existing development sites, which won't
    be able to find their themes.  You can either fix this by hand, or run:
    
      rake db:bootstrap:copy_default_theme
    
    The appropriate error message has been updated to explain this.
  2. isaackearse

    cleanup after tests that leave themes in the themes directory after c…

    isaackearse authored
    …reating sites => please review possibly naive implementation
Commits on Dec 7, 2008
  1. Eric Kidd

    Change test-spec contexts into regular unit tests

    emk authored
    This completely removes our depency on the test-spec library, which is
    no longer maintained.
  2. Eric Kidd

    Start replacing test-spec syntatic sugar with Rails 2.2 "it"

    emk authored
    The "specify" feature in test-spec is now provided by "it" in Rails 2.2.
  3. isaackearse Eric Kidd

    Fix bugs in the themes and admin/base controllers

    isaackearse authored emk committed
    admin/themes controller:
    
      change_to and export changed from get to post
      import changed from post to get
      added test verifying that trying to get admin/themes/change_to will
        fail and be redirected
    
    admin/base controller:
    
      changed protect_action method to be more generic and to give a helpful
        flash[:error]
    
    [Notes from Eric: I squashed three patches together and made some
    tweaks.  See the patch changelog below.]
    
    v2:
      Squashed a patch series into one patch
      Fix themes_controller_spec to route :post request, not :get
      Added comment on ThemesController#import about GET handling
Commits on Dec 6, 2008
  1. isaackearse

    only 2 failures in test:functionals now => changed more params to poi…

    isaackearse authored
    …nt to :controller => '/account'
  2. isaackearse
  3. isaackearse
  4. Eric Kidd

    Fix nil.controller_name? bug by removing extra requires

    emk authored
    We had a huge number of controller errors of the form:
    
      You have a nil object when you didn't expect it!
      The error occurred while evaluating nil.cached_pages
      .../mephisto/app/cachers/article_sweeper.rb:12:in `after_save'
    
    These were caused by duplicate calls to our sweepers' 'after' methods,
    which clear the controller values stored in the sweepers:
    
      self.controller = nil
    
    Calling into the sweeper a second time tripped over this line of code:
    
      controller_callback_method_name =
        "#{timing}_#{controller.controller_name.underscore}"
    
    This problem took me quite a while to track down.  The key insight came
    from the following blog post:
    
      http://blog.pastie.org/2008/12/passenger-wtf-1-nilcontroller_name-in-sweepers-module.html
    
    Our controller definitions were getting loaded twice!  This, in turn,
    registered each sweeper twice, causing it to get called twice.
    
    Thanks go to Isaac Kearse, who figured out how to fix this, as seen in
    commit 8fef75a.  Basically, all we need
    to do is remove lots of 'require' statements.  (As I understand it, we
    could also just replace them with 'require_dependency' statements, which
    check for duplicate loads.)
  5. isaackearse

    added another explicit route for sections passes another integration …

    isaackearse authored
    …test and section controller routing test
  6. isaackearse

    oooo I love deleting code

    isaackearse authored
Commits on Dec 5, 2008
  1. isaackearse
  2. Eric Kidd

    Make sure that all action/* routes are working

    emk authored
    What can I say?  I'm feeling paranoid today.
  3. isaackearse
  4. isaackearse
  5. Eric Kidd
  6. Eric Kidd
Something went wrong with that request. Please try again.