ReviewerMeeting20090422

summary

logs

AMEU

Apr 22 10:02:24 <barry> #startmeeting
Apr 22 10:02:25 <MootBot>       Meeting started at 09:02. The chair is barry.
Apr 22 10:02:25 <MootBot>       Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]
Apr 22 10:02:30 *       jtv (n=jtv@125.25.141.156.adsl.dynamic.totbb.net) has joined #launchpad-meeting
Apr 22 10:02:37 <barry> hello everyone and welcome to this week's ameu reviewers meeting.  who's here today?
Apr 22 10:02:41 <jtv>   me
Apr 22 10:02:44 <mars>  me
Apr 22 10:02:52 <barry> jtv: hai!
Apr 22 10:03:00 <jtv>   barry: learning dutch, I see!
Apr 22 10:03:13 <abentley>      me
Apr 22 10:03:15 <EdwinGrubbs>   me
Apr 22 10:03:16 <gmb>   me
Apr 22 10:03:42 *       Ursinha (n=ursula@canonical/launchpad/ursinha) has joined #launchpad-meeting
Apr 22 10:03:52 <barry> jtv: D'r ken geen koekje meer bij
Apr 22 10:03:58 <jtv>   :-)
Apr 22 10:04:25 <barry> i know we have a bunch of folks at the tl sprint
Apr 22 10:04:44 <barry> allenap, danilo_ ping
Apr 22 10:04:56 <gary_poster>   me
Apr 22 10:04:57 <allenap>       me
Apr 22 10:05:04 <barry> bac, BjornT, cprov ping
Apr 22 10:05:17 <cprov> me
Apr 22 10:05:18 <mars>  salgado, gary_poster, ping
Apr 22 10:05:21 <bac>   me.  darn, forgot again.
Apr 22 10:05:23 <salgado>       me
Apr 22 10:05:24 <barry> intellectronica: ping
Apr 22 10:05:27 <mars>  gary_poster, oops, sorry
Apr 22 10:05:31 <gary_poster>   :-)
Apr 22 10:05:36 <barry> noodles: ping
Apr 22 10:05:46 <barry> rockstar: ping
Apr 22 10:05:54 <noodles>       me (sorry)
Apr 22 10:06:31 <barry> [TOPIC] agenda
Apr 22 10:06:32 <MootBot>       New Topic:  agenda
Apr 22 10:06:45 <barry>  * Roll call
Apr 22 10:06:45 <barry>  * Action items
Apr 22 10:06:45 <barry>  * Mentoring update
Apr 22 10:06:45 <barry>  * Peanut gallery (anything not on the agenda)
Apr 22 10:07:29 <barry> [TOPIC]  * Action items
Apr 22 10:07:30 <MootBot>       New Topic:   * Action items
Apr 22 10:07:37 <barry>  * allenap to look into storm/sqlobject result set compatibility
Apr 22 10:08:04 <allenap>       I'm going to try and do that today :(
Apr 22 10:08:05 <gary_poster>   barry: re agenda, didn't we have that conversation from a review...trying to recall...two points...
Apr 22 10:08:26 <gary_poster>   barry: what is appropriate to import in a view...
Apr 22 10:08:28 <barry> gary_poster: yes, it's on the list, sorry didn't move it up
Apr 22 10:08:33 <gary_poster>   barry: cool
Apr 22 10:09:04 <barry> allenap: np, thanks.  we'll leave it on the list
Apr 22 10:09:14 <barry>  * flacoste to work on API reviewer cheat sheet
Apr 22 10:09:20 <intellectronica>       me
Apr 22 10:09:24 <barry> he's not here today, but does anybody know anything about it?
Apr 22 10:09:38 <gary_poster>   I can guess :-)
Apr 22 10:10:06 <barry> gary_poster: yeah :)
Apr 22 10:10:16 <barry> we'll leave it on the agenda
Apr 22 10:10:23 <barry> [TOPIC] mentoring update
Apr 22 10:10:24 <MootBot>       New Topic:  mentoring update
Apr 22 10:10:41 <barry> noodles: how are things going?  any questions/concerns?
Apr 22 10:10:59 <noodles>       barry: I haven't started yet... not sure who my mentor is?
Apr 22 10:11:14 <noodles>       Should I find one myself? What's the normal process...?
Apr 22 10:11:28 <noodles>       (I was away last week)
Apr 22 10:11:50 <barry> noodles: ah, dang.  i will work on that and get back to you.  i remember now that henninge is also a mentat
Apr 22 10:12:02 <noodles>       Great! Thanks :)
Apr 22 10:12:13 <bac>   and is deryck an official mentat now?
Apr 22 10:12:29 <barry> [ACTION] barry to find a mentor for noodles
Apr 22 10:12:30 <MootBot>       ACTION received:  barry to find a mentor for noodles
Apr 22 10:12:48 <barry> bac: not quite.  he's going to do js reviews (mentored) for now
Apr 22 10:13:35 <barry> anything else on mentoring?
Apr 22 10:14:06 <barry> [TOPIC] peanut gallery
Apr 22 10:14:07 <MootBot>       New Topic:  peanut gallery
Apr 22 10:14:26 <barry> we have something gary_poster wants to bring up. let me paste the discussion notes first
Apr 22 10:14:39 <barry> gary writes:
Apr 22 10:14:39 <barry>  * What is the goal (are the goals?) of our use of security proxies and the import nazi?  My understanding is that they are "belt and suspenders" to try and keep us from revealing private information.
Apr 22 10:14:39 <barry>  * Do we still agree with those goals?
Apr 22 10:14:39 <barry>  * Do we feel that our use of security proxies helps us with these goals?
Apr 22 10:14:39 <barry>  * Do we feel that our use of the import nazi helps us with these goals?
Apr 22 10:14:39 <barry> As we know, security is very hard, and trying to answer these questions well should not be done lightly or too quickly.
Apr 22 10:14:39 <barry> I am hopeful that the small, specific question of whether we should import removeSecurityProxies in view code will fall out obviously, if not easily, from our answers to the discussion above.
Apr 22 10:14:49 <barry> take it away gary_poster the orchestra leader
Apr 22 10:14:52 <gary_poster>   :-)
Apr 22 10:14:55 <gary_poster>   OK so, context
Apr 22 10:15:10 <gary_poster>   barry and I were talking about what is appropriate in a view
Apr 22 10:15:23 <gary_poster>   we agreed on this:
Apr 22 10:15:26 <gary_poster>   "I think view methods should only return immutable Python basics like strings and ints, or security wrapped objects."
Apr 22 10:15:45 <gary_poster>   We disagreed on this:
Apr 22 10:15:46 <gary_poster>   Also, ideally view code would not import removeSecurityProxy.  AIUI, This is exactly the kind of import that import nazi is supposed to prohibit: tools that *allow* a view to return unproxied objects.  While arguing for the import nazi to be eliminated might be appropriate at a reviewers meeting, we should not make it more and more ineffective without recognizing what we are doing.
Apr 22 10:15:47 <intellectronica>       dicts?
Apr 22 10:16:20 <barry> intellectronica: dicts, lists, tuples, sets are all basic objects, and okay
Apr 22 10:16:26 <gary_poster>   intellectronica: dicts are problematic because they can contain things that need to be proxied.  You typically need them to be proxied so that the viral thing works.
Apr 22 10:16:26 <intellectronica>       ah ok
Apr 22 10:16:46 <barry> with gary_poster 's caveat about *what* those containers contain
Apr 22 10:17:10 <gary_poster>   right.  as a practical matter, disagree with barry's assertion.  as a theoretical matter, yeah, kinda sorta
Apr 22 10:17:55 <gary_poster>   so maybe we need to dig into that.  but I was just expecting to dig into the import question
Apr 22 10:18:04 <jtv>   What's going to keep us honest w.r.t. interfaces, if not the security proxies?  Or is the idea that we don't need to be?
Apr 22 10:18:09 <barry> gary_poster: so, i've had to deal with this quite a bit lately, with the change in permissions to private membership teams
Apr 22 10:18:44 <gary_poster>   jtv: security proxies don't necessarily keep us honest irt the interfaces.  You can open up other things in the zcml, and in fact sometimes you need to
Apr 22 10:18:49 <barry> there are a lot of places where i've had to unwrap objects to get to their .name attribute.  clearly we can't do that in the model.  istm that the view is the most natural, *best* place to do it
Apr 22 10:19:15 <barry> and at least it's clearly obvious!  when you see removeSecurityProxy() that's a big red flag that something special is going on
Apr 22 10:19:24 <gary_poster>   barry: and yet what you are implying is that this could in fact be a utility function elsewhere
Apr 22 10:19:36 <bac>   barry: in using the removeSecurityProxy to get at a private team's name you must first be sure that your doing so doesn't in fact leak data
Apr 22 10:19:52 <jtv>   ...which you ensure in the model, not in the view.
Apr 22 10:20:01 <barry> bac: yes agreed. but isn't the view the right place to ensure that?
Apr 22 10:20:07 <gary_poster>   bac: right.
Apr 22 10:20:18 <gary_poster>   barry: I'd like to step back to principles
Apr 22 10:20:32 <barry> jtv: i think the model is the wrong place in fact
Apr 22 10:20:37 <barry> gary_poster: sure
Apr 22 10:20:39 <jtv>   Well here's a counter-example:
Apr 22 10:20:45 <bac>   barry: yes.  but i don't want to see a pattern of "oops i can't get to the name, better remove the security proxy" when the proxy is doing it's job
Apr 22 10:21:01 <jtv>   we implement private kumquats.  How do we make sure all the pre-existing views respect kumquat privacy?
Apr 22 10:21:03 <gary_poster>   barry: as I said in the discussion notes, this is a question in my mind about our use of the import nazi, and our goals for our own protection in the view
Apr 22 10:21:09 <gary_poster>   views, I should say
Apr 22 10:21:27 <gary_poster>   so, the import nazi has been around a looong time.
Apr 22 10:21:31 <gary_poster>   why is it there?
Apr 22 10:21:40 <gary_poster>   do we still want it?
Apr 22 10:21:48 <gary_poster>   I don't think we're going to resolve those here
Apr 22 10:21:58 <gary_poster>   but I think we need to start the process of resolving them
Apr 22 10:22:20 <gary_poster>   I have guesses as to why they exist
Apr 22 10:22:31 <gary_poster>   sorry, why the import nazi exists
Apr 22 10:22:44 <gary_poster>   and I think they are for belt and suspenders of the views
Apr 22 10:22:47 <mars>  gary_poster, that may be a question for Francis
Apr 22 10:22:53 <gary_poster>   to try to prevent data leaking out
Apr 22 10:22:56 <gmb>   That's belt and braces for the brits in here.
Apr 22 10:22:57 <mars>  gary_poster, or Curtis
Apr 22 10:23:02 <barry> gary_poster: that's kind of separate from the rSP-in-views issue.  afaik, importnazi doesn't enforce that
Apr 22 10:23:03 *       henninge (n=henning@i577A8F9C.versanet.de) has joined #launchpad-meeting
Apr 22 10:23:15 <henninge>      sorry ... volume turned down ...
Apr 22 10:23:15 <gary_poster>   barry: disagree.
Apr 22 10:23:33 <gary_poster>   barry: I think that we have gradually taken the teeth out of the import nazi
Apr 22 10:23:55 <barry> gary_poster: just saying, i-n doesn't prevent rSP-in-view.  maybe it should, maybe it shouldn't
Apr 22 10:24:11 <gary_poster>   it is a tool that is supposed to help as developers and reviewers conform to code standards, supposed to help us...do something
Apr 22 10:24:23 <barry> francis has the strongest views about what i-n is for, i believe
Apr 22 10:24:39 <barry> gary_poster: yes, do something :)
Apr 22 10:24:52 <gary_poster>   if I understand its goal correctly (if!) then it is specifically designed to keep thigs *like* rSP from happening
Apr 22 10:25:11 <barry> (btw, it's importfascist, strictly speaking :)
Apr 22 10:25:34 <jtv>   barry: right, might be Italian instead of German
Apr 22 10:25:34 <gary_poster>   ah, thank you, that is less potentially offensive.
Apr 22 10:25:39 <abentley>      gary_poster: That presupposes that rSP-in-view is against policy.
Apr 22 10:25:46 <bac>   gary_poster: we don't yet have agreement on the policy regarding rSP in views.  if we agree it is forbidden then import socialist can be made to flag it
Apr 22 10:25:53 <gary_poster>   so, I think we, as reviewers, or as a team, or as francis :-) need to decide why the import nazi is there
Apr 22 10:26:09 <gary_poster>   abentley: absolutely.  that's the question I'm asking.  let me try to sum:
Apr 22 10:26:36 <salgado>       the import nazi is there to make sure browser code can't have access to unsecurity-proxied objects
Apr 22 10:26:43 <salgado>       without explicitly calling rSP
Apr 22 10:27:09 <gary_poster>   salgado: so rSP is in fact the blessed way around this?
Apr 22 10:27:29 <salgado>       yes
Apr 22 10:27:36 <gary_poster>   my concern is that this is not known, written clearly, etc.
Apr 22 10:27:41 <gary_poster>   salgado: ok cool
Apr 22 10:27:52 <barry> aside: it does other things too, like enforce __all__'s
Apr 22 10:28:11 <gary_poster>   barry: but I think that is still part of the same goal
Apr 22 10:28:16 <salgado>       gary_poster, yeah, it's very likely this is not written clearly anywhere
Apr 22 10:28:34 <barry> gary_poster: right, in that it's intended to enforce certain coding standards
Apr 22 10:29:09 <barry> salgado, gary_poster so yes, we definitely need to make it clear in our standards the official way of doing things
Apr 22 10:29:40 <gary_poster>   barry: so, I move that we add a rationale for our use of import fascist, and our use of security proxies, and what the proper way of "breaking glass" is, to our reviewing guidelines
Apr 22 10:30:20 <abentley>      gary_poster: That sounds more like general-purpose documentation to me.
Apr 22 10:30:37 <gary_poster>   the import fascist is just a tool for us to enforce our policies, and I'd rather we understand/follow/lead from the intent of our policies than the letter of our policies
Apr 22 10:30:51 <gary_poster>   abentley: not sure how we are disagreeing?
Apr 22 10:31:16 <abentley>      reviewing guidelines are for reviewers, not necessarily for developers.
Apr 22 10:31:30 <gary_poster>   abentley: oh I see.  yes, agree
Apr 22 10:31:48 <barry> gary_poster: agreed.  i'd like to give francis a chance to weigh in too.  gary_poster this sounds like a discussion for the ml.  would you like to start that there?  i will take an action to document any decisions in the wiki
Apr 22 10:32:06 <gary_poster>   barry: yes, I'll take that.
Apr 22 10:32:12 <barry> gary_poster: thanks
Apr 22 10:32:24 <barry> [ACTION] gary_poster to take importfascist discussion to the ml
Apr 22 10:32:25 <MootBot>       ACTION received:  gary_poster to take importfascist discussion to the ml
Apr 22 10:32:40 <barry> [ACTION] barry to document all importfascist decisions in the appropriate wiki page
Apr 22 10:32:41 <MootBot>       ACTION received:  barry to document all importfascist decisions in the appropriate wiki page
Apr 22 10:32:58 <gary_poster>   are we cool on the security-proxy-around container bit that we skimmed over?
Apr 22 10:33:10 <gary_poster>   or should we add that to an agenda for a later meeting?
Apr 22 10:33:23 <barry> gary_poster: do you mean the entire container (dict, set, list) should be security proxied, or just the items it contains?
Apr 22 10:33:50 *       adeuring (n=abel@p4FC579FB.dip.t-dialin.net) has joined #launchpad-meeting
Apr 22 10:34:03 <adeuring>      me -- sorry for being late...
Apr 22 10:34:06 <gary_poster>   typically you proxy the container for simplicity.  If you *really* want to proxy the items only, you could, I guess, but that's not lazy for the programmer or the computer, typically
Apr 22 10:34:37 <barry> gary_poster: wouldn't in most cases the items in the container already be proxied?
Apr 22 10:34:56 <barry> gary_poster: talking about how the view sees them, not necessarily what it returns
Apr 22 10:36:07 <gary_poster>   barry: ahhhh...that's too general for me.  not sure.  when you get a container from a storm result, I would expect the result set to be proxied, so that the items are proxied just by the "viral" story
Apr 22 10:36:29 <barry> gary_poster: that would be my expectation too
Apr 22 10:37:08 <gary_poster>   barry: but...yeah...maybe :-)  as a simple-to-follow rule (for which perhaps there are exceptions) I'd suggest that containers should be proxied
Apr 22 10:37:11 <abentley>      gary_poster: A result set is an object, though, not one of the items Barry mentioned.
Apr 22 10:37:12 <barry> gary_poster: maybe we should rename removeSecurityProxy() to yesReallyLeakPrivateData()
Apr 22 10:37:12 *       Ursinha has quit (Read error: 110 (Connection timed out))
Apr 22 10:37:19 <gary_poster>   abentley: agreed
Apr 22 10:37:23 <gary_poster>   barry: :-)
Apr 22 10:37:29 *       bac must duck out early.  sorry.
Apr 22 10:38:11 <abentley>      gary_poster: How would you define the security policy for a dict?
Apr 22 10:38:43 <gary_poster>   abentley: I was just trying to come up with a general case I could think of.  I'm not sure what our general cases of returning basic python containers in views really are.  I'm just suggesting that proxying them is a nice simple rule.  security policy for a dict: I Think this is predefined.  It's public for the standard dict API.
Apr 22 10:38:57 *       Ursinha (n=ursula@canonical/launchpad/ursinha) has joined #launchpad-meeting
Apr 22 10:38:59 <gary_poster>   mapping API I should say
Apr 22 10:39:39 <abentley>      gary_poster: So for dicts, we would have them wrapped with a security proxy that did nothing?
Apr 22 10:39:45 <barry> gary_poster: but that's just to ensure that any objects returned are themselves proxied right?
Apr 22 10:39:58 <gary_poster>   abentley, barry: right on both counts
Apr 22 10:40:23 <gary_poster>   as barry said, having a dict with the individual items wrapped would probably be fine, especially for our use cases
Apr 22 10:40:55 <barry> i /think/ that's an unnecessary in our case, as we're guaranteed to have only wrapped objects in result sets, but it's a good thing to bring up in the ml thread
Apr 22 10:40:58 <gary_poster>   My primary interest in this is "easy to remember, easy to follow"
Apr 22 10:41:14 <gary_poster>   maybe you are right that we are automatically protected
Apr 22 10:41:24 <gary_poster>   in which case that is easy to remember and easy to follow
Apr 22 10:41:24 <abentley>      gary_poster: I would want to profile before and after making such a change.
Apr 22 10:41:28 <barry> gary_poster: ideally, "just happens" and you only have to think about it when you need to violate the rules
Apr 22 10:42:00 <barry> and even then, it's made plainly obvious that you've thought about it and deliberately broken the rule
Apr 22 10:42:44 <barry> but of course, it will eventually happen that something will leak anyway ;)
Apr 22 10:42:53 <gary_poster>   abentley: sure.  It's all in C.  It's very fast.  If we get utilities or adapters or other objects and*they* return dicts, then this is already happening.  That's probably sufficient, as barry is saying.
Apr 22 10:43:19 <barry> gary_poster: i'm certain francis could immediately confirm or deny
Apr 22 10:44:07 <gary_poster>   barry: cool.
Apr 22 10:44:09 <barry> so, we have about 2 minutes left.  anything more to say on this topic here?
Apr 22 10:44:14 <gary_poster>   not me.
Apr 22 10:44:23 <gary_poster>   I'll put it in the ml post
Apr 22 10:44:45 <barry> cool, thanks!  this is an important discussion to have.
Apr 22 10:44:55 <barry> anybody else have anything for today?
Apr 22 10:45:10 <mars>  barry, intellectronica and I had a small debate about JS, but it can wait I guess
Apr 22 10:45:22 <intellectronica>       ?
Apr 22 10:45:23 <barry> mars: can we put it on the agenda for next week?
Apr 22 10:45:24 <mars>  barry, should we use: if (!some_js_var)
Apr 22 10:45:34 <mars>  barry, we can
Apr 22 10:45:35 <intellectronica>       ah, whether explicit is better than implicit in JS too
Apr 22 10:45:40 <mars>  yep
Apr 22 10:45:51 <barry> cool, thanks.  we're outta time, so let's break here for today
Apr 22 10:45:54 <barry> #endmeeting

AsiaPac

none today

ReviewerMeeting20090422 (last edited 2009-04-29 13:10:07 by barry)