=== ursula_ is now known as Ursinha === ursula_ is now known as Ursinha [15:02] #startmeeting [15:02] Meeting started at 09:02. The chair is barry. [15:02] Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [15:02] hello everyone and welcome to this week's ameu reviewers meeting. who's here today? [15:02] me [15:02] me [15:02] jtv: hai! [15:03] barry: learning dutch, I see! [15:03] me [15:03] me [15:03] me [15:03] jtv: D'r ken geen koekje meer bij [15:03] :-) [15:04] i know we have a bunch of folks at the tl sprint [15:04] allenap, danilo_ ping [15:04] me [15:04] me [15:05] bac, BjornT, cprov ping [15:05] me [15:05] salgado, gary_poster, ping [15:05] me. darn, forgot again. [15:05] me [15:05] intellectronica: ping [15:05] gary_poster, oops, sorry [15:05] :-) [15:05] noodles: ping [15:05] rockstar: ping [15:05] me (sorry) [15:06] [TOPIC] agenda [15:06] New Topic: agenda [15:06] * Roll call [15:06] * Action items [15:06] * Mentoring update [15:06] * Peanut gallery (anything not on the agenda) [15:07] [TOPIC] * Action items [15:07] New Topic: * Action items [15:07] * allenap to look into storm/sqlobject result set compatibility [15:08] I'm going to try and do that today :( [15:08] barry: re agenda, didn't we have that conversation from a review...trying to recall...two points... [15:08] barry: what is appropriate to import in a view... [15:08] gary_poster: yes, it's on the list, sorry didn't move it up [15:08] barry: cool [15:09] allenap: np, thanks. we'll leave it on the list [15:09] * flacoste to work on API reviewer cheat sheet [15:09] me [15:09] he's not here today, but does anybody know anything about it? [15:09] I can guess :-) [15:10] gary_poster: yeah :) [15:10] we'll leave it on the agenda [15:10] [TOPIC] mentoring update [15:10] New Topic: mentoring update [15:10] noodles: how are things going? any questions/concerns? [15:10] barry: I haven't started yet... not sure who my mentor is? [15:11] Should I find one myself? What's the normal process...? [15:11] (I was away last week) [15:11] noodles: ah, dang. i will work on that and get back to you. i remember now that henninge is also a mentat [15:12] Great! Thanks :) [15:12] and is deryck an official mentat now? [15:12] [ACTION] barry to find a mentor for noodles [15:12] ACTION received: barry to find a mentor for noodles [15:12] bac: not quite. he's going to do js reviews (mentored) for now [15:13] anything else on mentoring? [15:14] [TOPIC] peanut gallery [15:14] New Topic: peanut gallery [15:14] we have something gary_poster wants to bring up. let me paste the discussion notes first [15:14] gary writes: [15:14] * 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. [15:14] * Do we still agree with those goals? [15:14] * Do we feel that our use of security proxies helps us with these goals? [15:14] * Do we feel that our use of the import nazi helps us with these goals? [15:14] As we know, security is very hard, and trying to answer these questions well should not be done lightly or too quickly. [15:14] 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. [15:14] take it away gary_poster the orchestra leader [15:14] :-) [15:14] OK so, context [15:15] barry and I were talking about what is appropriate in a view [15:15] we agreed on this: [15:15] "I think view methods should only return immutable Python basics like strings and ints, or security wrapped objects." [15:15] We disagreed on this: [15:15] 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. [15:15] dicts? [15:16] intellectronica: dicts, lists, tuples, sets are all basic objects, and okay [15:16] 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. [15:16] ah ok [15:16] with gary_poster 's caveat about *what* those containers contain [15:17] right. as a practical matter, disagree with barry's assertion. as a theoretical matter, yeah, kinda sorta [15:17] so maybe we need to dig into that. but I was just expecting to dig into the import question [15:18] 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? [15:18] gary_poster: so, i've had to deal with this quite a bit lately, with the change in permissions to private membership teams [15:18] 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 [15:18] 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 [15:19] and at least it's clearly obvious! when you see removeSecurityProxy() that's a big red flag that something special is going on [15:19] barry: and yet what you are implying is that this could in fact be a utility function elsewhere [15:19] 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 [15:19] ...which you ensure in the model, not in the view. [15:20] bac: yes agreed. but isn't the view the right place to ensure that? [15:20] bac: right. [15:20] barry: I'd like to step back to principles [15:20] jtv: i think the model is the wrong place in fact [15:20] gary_poster: sure [15:20] Well here's a counter-example: [15:20] 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 [15:21] we implement private kumquats. How do we make sure all the pre-existing views respect kumquat privacy? [15:21] 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 [15:21] views, I should say [15:21] so, the import nazi has been around a looong time. [15:21] why is it there? [15:21] do we still want it? [15:21] I don't think we're going to resolve those here [15:21] but I think we need to start the process of resolving them [15:22] I have guesses as to why they exist [15:22] sorry, why the import nazi exists [15:22] and I think they are for belt and suspenders of the views [15:22] gary_poster, that may be a question for Francis [15:22] to try to prevent data leaking out [15:22] That's belt and braces for the brits in here. [15:22] gary_poster, or Curtis [15:23] gary_poster: that's kind of separate from the rSP-in-views issue. afaik, importnazi doesn't enforce that [15:23] sorry ... volume turned down ... [15:23] barry: disagree. [15:23] barry: I think that we have gradually taken the teeth out of the import nazi [15:23] gary_poster: just saying, i-n doesn't prevent rSP-in-view. maybe it should, maybe it shouldn't [15:24] it is a tool that is supposed to help as developers and reviewers conform to code standards, supposed to help us...do something [15:24] francis has the strongest views about what i-n is for, i believe [15:24] gary_poster: yes, do something :) [15:24] if I understand its goal correctly (if!) then it is specifically designed to keep thigs *like* rSP from happening [15:25] (btw, it's importfascist, strictly speaking :) [15:25] barry: right, might be Italian instead of German [15:25] ah, thank you, that is less potentially offensive. [15:25] gary_poster: That presupposes that rSP-in-view is against policy. [15:25] 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 [15:25] so, I think we, as reviewers, or as a team, or as francis :-) need to decide why the import nazi is there [15:26] abentley: absolutely. that's the question I'm asking. let me try to sum: [15:26] the import nazi is there to make sure browser code can't have access to unsecurity-proxied objects [15:26] without explicitly calling rSP [15:27] salgado: so rSP is in fact the blessed way around this? [15:27] yes [15:27] my concern is that this is not known, written clearly, etc. [15:27] salgado: ok cool [15:27] aside: it does other things too, like enforce __all__'s [15:28] barry: but I think that is still part of the same goal [15:28] gary_poster, yeah, it's very likely this is not written clearly anywhere [15:28] gary_poster: right, in that it's intended to enforce certain coding standards [15:29] salgado, gary_poster so yes, we definitely need to make it clear in our standards the official way of doing things [15:29] 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 [15:30] gary_poster: That sounds more like general-purpose documentation to me. [15:30] 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 [15:30] abentley: not sure how we are disagreeing? [15:31] reviewing guidelines are for reviewers, not necessarily for developers. [15:31] abentley: oh I see. yes, agree [15:31] 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 [15:32] barry: yes, I'll take that. [15:32] gary_poster: thanks [15:32] [ACTION] gary_poster to take importfascist discussion to the ml [15:32] ACTION received: gary_poster to take importfascist discussion to the ml [15:32] [ACTION] barry to document all importfascist decisions in the appropriate wiki page [15:32] ACTION received: barry to document all importfascist decisions in the appropriate wiki page [15:32] are we cool on the security-proxy-around container bit that we skimmed over? [15:33] or should we add that to an agenda for a later meeting? [15:33] gary_poster: do you mean the entire container (dict, set, list) should be security proxied, or just the items it contains? [15:34] me -- sorry for being late... [15:34] 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 [15:34] gary_poster: wouldn't in most cases the items in the container already be proxied? [15:34] gary_poster: talking about how the view sees them, not necessarily what it returns [15:36] 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 [15:36] gary_poster: that would be my expectation too [15:37] barry: but...yeah...maybe :-) as a simple-to-follow rule (for which perhaps there are exceptions) I'd suggest that containers should be proxied [15:37] gary_poster: A result set is an object, though, not one of the items Barry mentioned. [15:37] gary_poster: maybe we should rename removeSecurityProxy() to yesReallyLeakPrivateData() [15:37] abentley: agreed [15:37] barry: :-) [15:37] * bac must duck out early. sorry. [15:38] gary_poster: How would you define the security policy for a dict? [15:38] 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. [15:38] mapping API I should say [15:39] gary_poster: So for dicts, we would have them wrapped with a security proxy that did nothing? [15:39] gary_poster: but that's just to ensure that any objects returned are themselves proxied right? [15:39] abentley, barry: right on both counts [15:40] as barry said, having a dict with the individual items wrapped would probably be fine, especially for our use cases [15:40] 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 [15:40] My primary interest in this is "easy to remember, easy to follow" [15:41] maybe you are right that we are automatically protected [15:41] in which case that is easy to remember and easy to follow [15:41] gary_poster: I would want to profile before and after making such a change. [15:41] gary_poster: ideally, "just happens" and you only have to think about it when you need to violate the rules [15:42] and even then, it's made plainly obvious that you've thought about it and deliberately broken the rule [15:42] but of course, it will eventually happen that something will leak anyway ;) [15:42] 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. [15:43] gary_poster: i'm certain francis could immediately confirm or deny [15:44] barry: cool. [15:44] so, we have about 2 minutes left. anything more to say on this topic here? [15:44] not me. [15:44] I'll put it in the ml post [15:44] cool, thanks! this is an important discussion to have. [15:44] anybody else have anything for today? [15:45] barry, intellectronica and I had a small debate about JS, but it can wait I guess [15:45] ? [15:45] mars: can we put it on the agenda for next week? [15:45] barry, should we use: if (!some_js_var) [15:45] barry, we can [15:45] ah, whether explicit is better than implicit in JS too [15:45] yep [15:45] cool, thanks. we're outta time, so let's break here for today [15:45] #endmeeting [15:45] Meeting finished at 09:45. [15:45] barry: Also, ampersands-in-urls. [15:46] thanks everyone! [15:46] abentley: agenda for next week, or ml? [15:46] thanks barry [15:46] barry: for next week [15:46] thanks. [15:46] barry: dankjewel en tot volgende keer :) [15:46] barry: Sorry for missing out, thanks for calling me! [15:46] abentley: if you are curious, this is one of the default checkers in zope/security/checkers.py: [15:46] dict: NamesChecker(['__getitem__', '__len__', '__iter__', [15:46] 'get', 'has_key', 'copy', '__str__', 'keys', [15:46] 'values', 'items', 'iterkeys', 'iteritems', [15:46] 'itervalues', '__contains__']) [15:47] jtv: :) [15:47] _default_checkers has a bunch of them [15:48] so that lets you get, but not set the dict === salgado is now known as salgado-lunch === salgado-lunch is now known as salgado === salgado is now known as salgado-afk